You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Dan Dumont <dd...@us.ibm.com> on 2012/04/12 15:34:54 UTC
Review Request: Path To JS Servlet Not Passed As Param For Gadgets Of Type
URL
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4710/
-----------------------------------------------------------
Review request for shindig.
Summary
-------
Fix shindig to implement the stated spec, that the libs param should be a url to download feature impl, not just a list of features.
Additional conversation on the spec list here: https://groups.google.com/d/topic/opensocial-and-gadgets-spec/7lGzhk67nY8/discussion
This addresses bug SHINDIG-1707.
https://issues.apache.org/jira/browse/SHINDIG-1707
Diffs
-----
http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.html PRE-CREATION
http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.xml PRE-CREATION
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1311965
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1311965
Diff: https://reviews.apache.org/r/4710/diff
Testing
-------
Updated unit tests to expect the new url.
All tests pass.
Added a sample url gadget and html page to show the feature working.
Thanks,
Dan
Re: Review Request: Path To JS Servlet Not Passed As Param For Gadgets Of
Type URL
Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4710/#review6899
-----------------------------------------------------------
Ship it!
Committed r1325432
- Dan
On 2012-04-12 13:34:54, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4710/
> -----------------------------------------------------------
>
> (Updated 2012-04-12 13:34:54)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> Fix shindig to implement the stated spec, that the libs param should be a url to download feature impl, not just a list of features.
>
> Additional conversation on the spec list here: https://groups.google.com/d/topic/opensocial-and-gadgets-spec/7lGzhk67nY8/discussion
>
>
> This addresses bug SHINDIG-1707.
> https://issues.apache.org/jira/browse/SHINDIG-1707
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.html PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.xml PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1311965
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1311965
>
> Diff: https://reviews.apache.org/r/4710/diff
>
>
> Testing
> -------
>
> Updated unit tests to expect the new url.
> All tests pass.
>
> Added a sample url gadget and html page to show the feature working.
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Path To JS Servlet Not Passed As Param For Gadgets Of
Type URL
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4710/#review6878
-----------------------------------------------------------
Ship it!
LGTM
- Stanton
On 2012-04-12 13:34:54, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4710/
> -----------------------------------------------------------
>
> (Updated 2012-04-12 13:34:54)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> Fix shindig to implement the stated spec, that the libs param should be a url to download feature impl, not just a list of features.
>
> Additional conversation on the spec list here: https://groups.google.com/d/topic/opensocial-and-gadgets-spec/7lGzhk67nY8/discussion
>
>
> This addresses bug SHINDIG-1707.
> https://issues.apache.org/jira/browse/SHINDIG-1707
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.html PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.xml PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1311965
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1311965
>
> Diff: https://reviews.apache.org/r/4710/diff
>
>
> Testing
> -------
>
> Updated unit tests to expect the new url.
> All tests pass.
>
> Added a sample url gadget and html page to show the feature working.
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Path To JS Servlet Not Passed As Param For Gadgets Of
Type URL
Posted by Dan Dumont <dd...@us.ibm.com>.
> On 2012-04-12 17:42:54, Henry Saputra wrote:
> > Could you take a look at the RenderingGadgetRewriter.injectFeatureLibraries() method? Looks like its trying to get "libs" from context. For HTML based gadget the libs in the URL could just be colon separated feature names I believe.
>
> Dan Dumont wrote:
> I'm not sure I want to change the behavior of non url type gadgets. The arguments in an html gadget aren't specced, for the most part, as shindig implements all the feature code that uses them. The url type gadgets though require us to identify key ones that must be there and useful. I am curious now to see if the feature detection libraries need to be updated for url gadgets. I'll check in to that.
>
> Did I miss your point? I can hop on gchat if you think that would be easier.
I tested loading and using the open-views feature and it works, but there are some apis that malfunction.
gadgets.util.hasFeature('open-views') is a call that does not work correctly in url gadgets. The reason for this is not due to my change, but because we don't have a way to do the config injection for url type gadgets. Tossed some ideas back and forth with Stanton for a few minutes, there may be something we can do if libs= param were a multi-url param. Then we might send a link to the config, I'm not super crazy about that approach and I'd like to see if we couldn't let the gadgets obtain config from the container page over rpc instead... anyway... this isn't the time or place for these talks, I'll open a JIRA for the config issue and that can be assigned as we think is appropriate.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4710/#review6893
-----------------------------------------------------------
On 2012-04-12 13:34:54, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4710/
> -----------------------------------------------------------
>
> (Updated 2012-04-12 13:34:54)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> Fix shindig to implement the stated spec, that the libs param should be a url to download feature impl, not just a list of features.
>
> Additional conversation on the spec list here: https://groups.google.com/d/topic/opensocial-and-gadgets-spec/7lGzhk67nY8/discussion
>
>
> This addresses bug SHINDIG-1707.
> https://issues.apache.org/jira/browse/SHINDIG-1707
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.html PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.xml PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1311965
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1311965
>
> Diff: https://reviews.apache.org/r/4710/diff
>
>
> Testing
> -------
>
> Updated unit tests to expect the new url.
> All tests pass.
>
> Added a sample url gadget and html page to show the feature working.
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Path To JS Servlet Not Passed As Param For Gadgets Of
Type URL
Posted by Dan Dumont <dd...@us.ibm.com>.
> On 2012-04-12 17:42:54, Henry Saputra wrote:
> > Could you take a look at the RenderingGadgetRewriter.injectFeatureLibraries() method? Looks like its trying to get "libs" from context. For HTML based gadget the libs in the URL could just be colon separated feature names I believe.
I'm not sure I want to change the behavior of non url type gadgets. The arguments in an html gadget aren't specced, for the most part, as shindig implements all the feature code that uses them. The url type gadgets though require us to identify key ones that must be there and useful. I am curious now to see if the feature detection libraries need to be updated for url gadgets. I'll check in to that.
Did I miss your point? I can hop on gchat if you think that would be easier.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4710/#review6893
-----------------------------------------------------------
On 2012-04-12 13:34:54, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4710/
> -----------------------------------------------------------
>
> (Updated 2012-04-12 13:34:54)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> Fix shindig to implement the stated spec, that the libs param should be a url to download feature impl, not just a list of features.
>
> Additional conversation on the spec list here: https://groups.google.com/d/topic/opensocial-and-gadgets-spec/7lGzhk67nY8/discussion
>
>
> This addresses bug SHINDIG-1707.
> https://issues.apache.org/jira/browse/SHINDIG-1707
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.html PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.xml PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1311965
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1311965
>
> Diff: https://reviews.apache.org/r/4710/diff
>
>
> Testing
> -------
>
> Updated unit tests to expect the new url.
> All tests pass.
>
> Added a sample url gadget and html page to show the feature working.
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Path To JS Servlet Not Passed As Param For Gadgets Of
Type URL
Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4710/#review6893
-----------------------------------------------------------
Could you take a look at the RenderingGadgetRewriter.injectFeatureLibraries() method? Looks like its trying to get "libs" from context. For HTML based gadget the libs in the URL could just be colon separated feature names I believe.
- Henry
On 2012-04-12 13:34:54, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4710/
> -----------------------------------------------------------
>
> (Updated 2012-04-12 13:34:54)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> Fix shindig to implement the stated spec, that the libs param should be a url to download feature impl, not just a list of features.
>
> Additional conversation on the spec list here: https://groups.google.com/d/topic/opensocial-and-gadgets-spec/7lGzhk67nY8/discussion
>
>
> This addresses bug SHINDIG-1707.
> https://issues.apache.org/jira/browse/SHINDIG-1707
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.html PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.xml PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1311965
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1311965
>
> Diff: https://reviews.apache.org/r/4710/diff
>
>
> Testing
> -------
>
> Updated unit tests to expect the new url.
> All tests pass.
>
> Added a sample url gadget and html page to show the feature working.
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Path To JS Servlet Not Passed As Param For Gadgets Of
Type URL
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4710/#review6890
-----------------------------------------------------------
Ship it!
LGTM
- Ryan
On 2012-04-12 13:34:54, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4710/
> -----------------------------------------------------------
>
> (Updated 2012-04-12 13:34:54)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> Fix shindig to implement the stated spec, that the libs param should be a url to download feature impl, not just a list of features.
>
> Additional conversation on the spec list here: https://groups.google.com/d/topic/opensocial-and-gadgets-spec/7lGzhk67nY8/discussion
>
>
> This addresses bug SHINDIG-1707.
> https://issues.apache.org/jira/browse/SHINDIG-1707
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.html PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.xml PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1311965
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1311965
>
> Diff: https://reviews.apache.org/r/4710/diff
>
>
> Testing
> -------
>
> Updated unit tests to expect the new url.
> All tests pass.
>
> Added a sample url gadget and html page to show the feature working.
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Path To JS Servlet Not Passed As Param For Gadgets Of
Type URL
Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4710/#review6896
-----------------------------------------------------------
Ship it!
+1
- Henry
On 2012-04-12 13:34:54, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4710/
> -----------------------------------------------------------
>
> (Updated 2012-04-12 13:34:54)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> Fix shindig to implement the stated spec, that the libs param should be a url to download feature impl, not just a list of features.
>
> Additional conversation on the spec list here: https://groups.google.com/d/topic/opensocial-and-gadgets-spec/7lGzhk67nY8/discussion
>
>
> This addresses bug SHINDIG-1707.
> https://issues.apache.org/jira/browse/SHINDIG-1707
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.html PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/url-gadget-with-features/url.xml PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java 1311965
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java 1311965
>
> Diff: https://reviews.apache.org/r/4710/diff
>
>
> Testing
> -------
>
> Updated unit tests to expect the new url.
> All tests pass.
>
> Added a sample url gadget and html page to show the feature working.
>
>
> Thanks,
>
> Dan
>
>