[
https://issues.apache.org/jira/browse/SHINDIG-1669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13166199#comment-13166199
]
[email protected] commented on SHINDIG-1669:
--------------------------------------------------------
bq. On 2011-12-08 15:24:35, Jesse Ciancetta wrote:
bq. > Sorry for being late to the review -- this is the first chance I've had
to really look this over. I have a few concerns with this approach that make
me wonder if an alternate approach might be more appropriate -- although the
alternate I have in mind has some issues of its own too... I'll list my
concerns first and then offer the potential alternate solution.
bq. >
bq. > -- Performance (of course, as others have pointed out) on both the
client side and server side. On the client side -- if the server takes a while
to process the request (say two seconds) then that means the client has to wait
four seconds before they actually see the content. On the server side the
server will end up seeing twice the load.
bq. >
bq. > -- Side effects of two calls. I know an HTTP GET for an iframe url
should be idempotent, but what if its not? We should at least document that
the call gets made twice to be sure callers are aware.
bq. >
bq. > -- Authentication. What if the iframe url requires some sort of
authentication? Maybe the iframe url returns a a 401 with a basic auth
challenge. Or maybe the url is protected with SSO and the clients browser has
a valid SSO cookie to send, but shindig would not. In either of these cases
we'd say that the navigate failed (since shindig would not be able to auth) but
the client may have been able to auth just fine.
bq. >
bq. > -- There's really no *guarantee* that just because the head request
succeeds that the subsequent get will succeed too. It's extremely likely to
succeed, but not guaranteed.
bq. >
bq. > -- If we go with this approach I think we should offer some way for the
caller to opt-out of the head request in case they need to avoid any of the
issues I've raised above.
bq. >
bq. > As an alternative -- it seems to me that an onload listener on the
iframe with a caller-configurable-timeout might actually be a more useful way
to approach this. I think more often than not we have some well known set
amount of time that we're willing to wait for the content of the iframe to load
before we just give up and call it a failure -- so I'm wondering if we could
just register an onload listener on the iframe and if the onload event hasn't
fired within the designated amount of time we consider the navigate as a
failure. The issues I see with this approach are:
bq. >
bq. > -- It needs to be tested and proven that onload on the iframe works the
way I think it does in all major browsers (which is that it only fires if the
iframe loaded successfully).
bq. >
bq. > -- If the iframe fails immediately the user would see the error
displayed in the frame until the timeout expired (assuming on timeout we put a
nice error message in place of the iframe). Maybe the iframe could be hidden
until successful? That could degrade from the experience though too... Not
sure what the best approach is here.
bq. >
bq. > If the consensus is that the head method is best I'm fine with it (as
long as we add the ability for the caller to opt-out) -- I just wanted to be
sure to point out all the issues I see and offer this potential alternative for
consideration.
bq.
bq. Ryan Baxter wrote:
bq. All great points Jesse.
bq.
bq. I thought of the iframe onload event as well, but when I was thinking
about it I was hoping there was an event passed into the onload function which
would tell me whether the iframe loaded properly or not, but there wasn't (At
least I couldn't find any information on it.). I didn't think about doing it
the way you described. I did a quick test and FF does not fire the onload
event when the URL cannot load the page, but Chrom and IE8 do, so the approach
of a timeout would not work.
bq.
bq. I can certainly make the head request an opt in approach and only make
the head request when the called supplies a callback into the navigateUrl call,
if not, there is no point in making the head request anyways.
Ah -- ok... I'd seen some people saying onload wouldn't get fired for iframes
when the content failed to load but hadn't had a chance to test it out -- too
bad it isn't consistent across browsers. With the option to opt-out by not
passing the callback I think this looks good.
- Jesse
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3037/#review3739
-----------------------------------------------------------
On 2011-12-08 17:58:33, Ryan Baxter wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/3037/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-12-08 17:58:33)
bq.
bq.
bq. Review request for shindig, Dan Dumont and Stanton Sievers.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. When you call commoncontainer.navigateUrl if the URL cannot be reached the
caller has no way of knowing if the URL was navigated successfully or not. To
solve this we make a head request to the URL we are navigating to and add a
callback to the API.
bq.
bq. It is important to note that we will not be caching the response of the
head request. While this could possibly give us better performance we have no
way of guaranteeing the server will still be up next time and everything may
fail. This is different from the gadget case where we have the gadget XML
cached on the server.
bq.
bq.
bq. This addresses bug SHINDIG-1669.
bq. https://issues.apache.org/jira/browse/SHINDIG-1669
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/embeddedexperiences/PhotoList.xml
1211913
bq.
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js
1211913
bq.
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js
1211913
bq.
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js
1211913
bq.
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/open-views/viewenhancements-container.js
1211913
bq.
http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container.url/container_url_test.js
1211913
bq.
bq. Diff: https://reviews.apache.org/r/3037/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Tested in container as well as updating unit tests.
bq.
bq.
bq. Thanks,
bq.
bq. Ryan
bq.
bq.
> Navigating to URLs in the common container has no indication of success or
> failure
> ----------------------------------------------------------------------------------
>
> Key: SHINDIG-1669
> URL: https://issues.apache.org/jira/browse/SHINDIG-1669
> Project: Shindig
> Issue Type: Improvement
> Affects Versions: 3.0.0
> Reporter: Ryan Baxter
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> When you call commoncontainer.navigateUrl if the URL cannot be reached the
> caller has no way of knowing if the URL was navigated successfully or not.
> To solve this we make a head request to the URL we are navigating to and add
> a callback to the API.
> It is important to note that we will not be caching the response of the head
> request. While this could possibly give us better performance we have no way
> of guaranteeing the server will still be up next time and everything may
> fail. This is different from the gadget case where we have the gadget XML
> cached on the server.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
