http://codereview.appspot.com/65052/diff/1/2
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/DefaultTemplateProcessor.java
(right):

http://codereview.appspot.com/65052/diff/1/2#newcode446
Line 446: buffer.append("}).apply(e);");
On 2009/05/20 13:40:32, levik wrote:
On 2009/05/15 17:47:00, Evan Gilbert (code review) wrote:
> Might be easier to add an ID to the original inserted node (or use
existing
ID).
>
> Pseudo-code
> String id = el.getAttribute("id")
> if (id == null) {
>   id = generateUniqueId();
>   el.setAttribute(id);
> }
> ....
> buffer.append("}.apply(document.getElementById('" + jsEscape(id) +
"')");
>
>

This would indeed be simpler, but would misfire for the edge case
where existing
IDs are non-unique (which is frowned upon, but possible in HTML).

I'm a little worried about the complexity and performance of creating
and removing elements while processing scripts on the page. These can be
in repeated loops, so there can be a *lot* of these calls. Also, this
construction might be harder to sanitize.

I'm also fine if this fails in cases where users have manually assigned
duplicate HTML element IDs. If you're using JavaScript this seems like a
reasonable standard to enforce.

http://codereview.appspot.com/65052

Reply via email to