> On 2011-12-07 17:08:05, Dan Dumont wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js, > > line 922 > > <https://reviews.apache.org/r/3037/diff/2/?file=62529#file62529line922> > > > > Disclaimer: I don't fully understand the impact of case in the jsdoc, > > however this url: > > http://code.google.com/closure/compiler/docs/js-for-compiler.html never > > uses 'String' only 'string'
Not sure why i did that in the first place :) > On 2011-12-07 17:08:05, Dan Dumont wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js, > > line 924 > > <https://reviews.apache.org/r/3037/diff/2/?file=62529#file62529line924> > > > > Please describe the args for the function param: > > http://code.google.com/closure/compiler/docs/js-for-compiler.html > > > > @param {function(Object)=} does that look right? LGTM > On 2011-12-07 17:08:05, Dan Dumont wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/open-views/viewenhancements-container.js, > > line 221 > > <https://reviews.apache.org/r/3037/diff/2/?file=62531#file62531line221> > > > > Your past review had something like this in it... I thought you > > changed it to: if(!result[gadgetUrl] || result[gadgetUrl].error) > > ... > > navigateCallback([null, result[gadgetUrl] || {error:result}); > > > > Is that not needed anymore? Good catch. Different code but needs to have similar error handling :) - Ryan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3037/#review3702 ----------------------------------------------------------- On 2011-12-06 22:36:38, Ryan Baxter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3037/ > ----------------------------------------------------------- > > (Updated 2011-12-06 22:36:38) > > > Review request for shindig, Dan Dumont and Stanton Sievers. > > > Summary > ------- > > 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 addresses bug SHINDIG-1669. > https://issues.apache.org/jira/browse/SHINDIG-1669 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/open-views/viewenhancements-container.js > 1211103 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js > 1211103 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js > 1211103 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js > 1211103 > > http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/embeddedexperiences/PhotoList.xml > 1211103 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container.url/container_url_test.js > 1211103 > > Diff: https://reviews.apache.org/r/3037/diff > > > Testing > ------- > > Tested in container as well as updating unit tests. > > > Thanks, > > Ryan > >
