http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/Composite.java
File user/src/com/google/gwt/user/client/ui/Composite.java (right):
http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/Composite.java#newcode111
user/src/com/google/gwt/user/client/ui/Composite.java:111:
builder.append(TEMPLATE.renderWithId(stamper.getToken()));
On 2011/07/06 13:53:47, rdcastro wrote:
On 2011/07/06 13:29:23, jlabanca wrote:
> getToken() is deprecated, use:
> builder.append(stamper.stamp(TEMPLATE.render()));
Explained on other comment.
@SuppressWarning("deprecation")
http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/RenderablePanel.java
File user/src/com/google/gwt/user/client/ui/RenderablePanel.java
(right):
http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/RenderablePanel.java#newcode198
user/src/com/google/gwt/user/client/ui/RenderablePanel.java:198: String
id = stamper.getToken();
On 2011/07/06 13:53:47, rdcastro wrote:
On 2011/07/06 13:29:23, jlabanca wrote:
> Can you stamp the SafeHtml returned from the TEMPLATE instead of
using the ID
> directly? Removing getToken() from our code will help confirm that
its safe
to
> hide the implementation.
I can, but I didn't want to do that in this CL because I know there'll
be _some_
performance regression, but I don't if it'll be measurable. I'd like
to get this
checked in and then I can work separately on orkut measuring and
comparing both
approaches and decide which works best (and if the difference is
actually
noticeable). I think I'll end up converting everything to
ElementBuilder, which
will be a biggish CL, but centered on orkut code.
In any case, I'll have a follow-up to this CL that hides the getToken
method at
most within a week.
SGTM - but can you add a TODO explaining that you are going to test
performance so the next person doesn't try to fix it.
Also, you'll have to add a @SuppressWarning("deprecation") annotation
now that getToken() is deprecated.
http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/RenderableStamper.java
File user/src/com/google/gwt/user/client/ui/RenderableStamper.java
(right):
http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/RenderableStamper.java#newcode58
user/src/com/google/gwt/user/client/ui/RenderableStamper.java:58: int
endOfFirstTag = html.indexOf('>');
It would still break with "<div/>".
1-) trim() and verify that the first character is a '<'. If not,
return.
2-) Find the first occurrence of either ' ' or '/>' or '>'; if none is
found, return
(this can't happen, I think, because then the SafeHtml would be
malformed)
3-) Inject the ID
On 2011/07/06 13:53:47, rdcastro wrote:
On 2011/07/06 13:29:23, jlabanca wrote:
> This doesn't handle all forms of SafeHtml. For example, a simple
string
"hello
> world" is considered safe html, as is a self closing tag "<div
> style='color:red;' />"
Good point. I'm inclined to change this to:
1-) Find the first occurrence of '<'; if none is found, return;
2-) Find the first occurrence of either ' ' or '>'; if none is found,
return
(this can't happen, I think, because then the SafeHtml would be
malformed)
3-) Inject the ID
how does that sound?
http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/RenderableStamper.java#newcode62
user/src/com/google/gwt/user/client/ui/RenderableStamper.java:62:
.append(token)
On 2011/07/06 13:53:47, rdcastro wrote:
On 2011/07/06 13:29:23, jlabanca wrote:
> Are we sure that the token is always safe? We should escape in
> RenderableStamper's constructor, or at least add a comment
explaining that we
> assume it is safe.
Escaping sounds like the right solution. What do you guys usually use
for
escaping?
SafeHtmlUtils.htmlEscape(String)
By default, you generate the ID using alphanumeric characters, so it
will be unchanged.
http://gwt-code-reviews.appspot.com/1472801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
