http://codereview.appspot.com/1726041/diff/6001/7002
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java
(right):
http://codereview.appspot.com/1726041/diff/6001/7002#newcode42
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java:42:
public enum TagsToMakeAbsolute {
On 2010/06/17 20:59:54, johnfargo wrote:
nit: since this is already a subclass of AbsolutePathReferenceVisitor
you can
probably just call this Tags
Done.
http://codereview.appspot.com/1726041/diff/6001/7002#newcode60
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java:60:
TagsToMakeAbsolute(Map<String, String> resourceTags) {
On 2010/06/17 20:59:54, johnfargo wrote:
make private
Done.
http://codereview.appspot.com/1726041/diff/6001/7002#newcode62
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java:62:
}
On 2010/06/17 20:59:54, johnfargo wrote:
nit: missing space here
Done.
http://codereview.appspot.com/1726041/diff/6001/7002#newcode69
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java:69:
Map<String, String> tagsToMakeAbsolute;
On 2010/06/17 20:59:54, johnfargo wrote:
should be private final
Done.
http://codereview.appspot.com/1726041/diff/6001/7002#newcode88
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java:88:
Uri baseUri = getUriToResolveUrisRelativeTo(gadget, node);
On 2010/06/17 20:59:54, johnfargo wrote:
just call this getBaseUri, or perhaps getBaseResolutionUri
Done.
http://codereview.appspot.com/1726041/diff/6001/7002#newcode171
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java:171:
protected String getBaseHref(Document doc) {
On 2010/06/17 20:59:54, johnfargo wrote:
this can probably be private -- when would you see a need to
customize?
made private. IMO reusable helper functions should be made protected,
but this is fine.
http://codereview.appspot.com/1726041/diff/6001/7002#newcode177
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java:177:
Attr attr = (Attr) list.item(0).getAttributes().getNamedItem("href");
On 2010/06/17 20:59:54, johnfargo wrote:
getAttributes() can return null - either:
A. check this to avoid NPE, or
B. cast list.item(0) to an Element (getElementsByTagName's NodeList is
populated
exclusively with them by contract) and use getAttribute("href") (note
that this
API returns "" rather than null if the attr doesn't exist)
checked for null.
http://codereview.appspot.com/1726041/show
