-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1692/#review1728
-----------------------------------------------------------

Ship it!


LGTM in theory.  This code is used by all of the rewriters, so it's hard to 
wrap my head around potential ripple effect scenarios.  I'll put my faith in 
the JUnits to catch any major problems introduced by making this change.

- Stanton


On 2011-09-01 16:38:04, Brian Lillie wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1692/
> -----------------------------------------------------------
> 
> (Updated 2011-09-01 16:38:04)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> There are at least two cases where a URI is presented as a Gadget URI and is 
> rather a URI based upon an incoming request.
> 
> 1) When a resource such as a CSS file is loaded and has the links/URLs 
> rewritten, the gadget parameter supplied on the rewritten link contains a URI 
> associated with the resource requested, rather than the gadget associated 
> with the request
> 2) When a request is made to the Gadget Blacklist, the URI parameter may not 
> represent the intended gadget, but rather a random resource
> 
> With both of thse instances, the common pattern is that the 
> DomWalker.makeGadget( HttpRequest ) is called, and the returned GadgetContext 
> uses the request URI, rather than the Gadget URI.
> 
> Modify the DOMWalker makeGadget to prefer the use of the gadget URI to the 
> request URI when constructing a Gadget/GadgetContext for use in rewriting or 
> generating other requests
> 
> 
> This addresses bug SHINDIG-1613.
>     https://issues.apache.org/jira/browse/SHINDIG-1613
> 
> 
> Diffs
> -----
> 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DomWalker.java
>  1164090 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/rewritebasic-expected.css
>  1164090 
> 
> Diff: https://reviews.apache.org/r/1692/diff
> 
> 
> Testing
> -------
> 
> Modified rewrite junits to handle expected result
> 
> 
> Thanks,
> 
> Brian
> 
>

Reply via email to