> On 2012-01-28 15:17:28, Stanton Sievers wrote: > > My biggest concern with this change is that it breaks backwards > > compatibility. There is no longer "iframeurl" on the metadata responses. > > This isn't a big deal if everyone is using the common container, since > > you've updated that portion, but if they aren't, then this will cause > > problems. Maybe not a big deal, as the metadata request/response is not > > spec'd. Something to consider and get more feedback on from the rest of > > the dev list. > > > > Also, have you tried this out with requestNavigateTo functionality or the > > view switching functionality (as in the sample common container)? With > > this patch the CC shouldn't have to re-issue any metadata requests because > > it has everything it needs. I want to make sure that is indeed the case.
RE backwards compatibility I realize this. Since this is the first release of Shindig with the common container I think it is fine to break the "API". However if someone have a need for that functionality I can add it back in. RE requestNavigateTo Even before this change we only ever made 1 request to get the metadata for the gadget. When switching views we used the cached metadata. This was part of the problem. The cache is indexed by gadget URL so switching views and / or subsequent calls to requestNavigateTo always resulted in using the cached metadata. > On 2012-01-28 15:17:28, Stanton Sievers wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js, > > line 169 > > <https://reviews.apache.org/r/3670/diff/3/?file=71190#file71190line169> > > > > With your changes, uri is now the uri with render params and other > > query params set on it. Is that necessary for the patch? I'd like to not > > change this behavior if you don't have to. It was not necessary, but it seemed odd to me that we were not using the actual URL in the iFrame when setting up relay uri. If getIframeUrl_ changed the URL in some way that would effect setting up RPC then we might be in trouble. I figured it would be best to use the same URL that the iFrame will contain. - Ryan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3670/#review4672 ----------------------------------------------------------- On 2012-01-30 15:29:59, Ryan Baxter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3670/ > ----------------------------------------------------------- > > (Updated 2012-01-30 15:29:59) > > > Review request for shindig. > > > Summary > ------- > > The request for gadgets.metadata fails for a gadget spec that doesn't specify > a "default" view. > > > This addresses bug SHINDIG-1549. > https://issues.apache.org/jira/browse/SHINDIG-1549 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js > 1236884 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js > 1236884 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js > 1236884 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js > 1236884 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java > 1236884 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java > 1236884 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java > 1236884 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java > 1236884 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/IframeUriManager.java > 1236884 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/FakeIframeUriManager.java > 1236884 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java > 1236884 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java > 1236884 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java > 1236884 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriManagerTestBase.java > 1236884 > > Diff: https://reviews.apache.org/r/3670/diff > > > Testing > ------- > > Updated unit tests and tested in the various containers > > > Thanks, > > Ryan > >
