(Actually looked at everything this time.)
http://gwt-code-reviews.appspot.com/1455802/diff/5002/user/src/com/google/gwt/dom/builder/shared/HtmlBuilderImpl.java
File user/src/com/google/gwt/dom/builder/shared/HtmlBuilderImpl.java
(right):
http://gwt-code-reviews.appspot.com/1455802/diff/5002/user/src/com/google/gwt/dom/builder/shared/HtmlBuilderImpl.java#newcode160
user/src/com/google/gwt/dom/builder/shared/HtmlBuilderImpl.java:160:
sb.append("<").append(escape(tagName));
On 2011/06/24 16:48:18, jlabanca wrote:
I think it would be difficult to know for sure when we need escaping.
I tracked
down the w3c syntax for element tag names:
http://www.w3.org/TR/xml11/#NT-Name
That's general XML and I'm not sure if it makes sense for HTML; do we
care about the ability to generate unknown HTML tags? Does that even
work? I have never seen tag name escaping in HTML and I'm not sure if
that works either. (There would be interesting security implications if
it did; for example you could hide script tags from naive scanners that
look for "<script".)
Since this is a new API and we're generating, not parsing, we could be
relatively restrictive here and just allow [a-z][a-z0-9]+. I believe
that covers all known HTML5 tags. If there's some odd use-case we can
relax the restriction later.
Or we could just not expose any way to create arbitrary tags, and then
it's irrelevant.
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/client/DomBuilderImpl.java
File user/src/com/google/gwt/dom/builder/client/DomBuilderImpl.java
(right):
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/client/DomBuilderImpl.java#newcode154
user/src/com/google/gwt/dom/builder/client/DomBuilderImpl.java:154: if
(stackElements.isEmpty()) {
It looks like we have the invariant that currentElement is null when the
stack is empty, so these isEmpty() checks could be replaced by null
checks (if it matters).
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/ElementBuilderBase.java
File user/src/com/google/gwt/dom/builder/shared/ElementBuilderBase.java
(right):
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/ElementBuilderBase.java#newcode76
user/src/com/google/gwt/dom/builder/shared/ElementBuilderBase.java:76: *
@throws ClassCastException if the parent builder does not match the
This needs a bit more explanation. The end() method isn't really
throwing the exception, is it?
I also think we need to give an example of the casting trick in the
Javadoc, since it's a fairly obscure part of the Java language.
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/ElementBuilderBase.java#newcode175
user/src/com/google/gwt/dom/builder/shared/ElementBuilderBase.java:175:
DivBuilder startDiv();
SelectBuilder and OptionBuilder are descendants of this interface, but
this method doesn't make sense for them. (Do we care? Future CL?)
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/ElementBuilderBase.java#newcode192
user/src/com/google/gwt/dom/builder/shared/ElementBuilderBase.java:192:
* Start the style element.
Did you mean the style attribute?
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/ElementBuilderBase.java#newcode214
user/src/com/google/gwt/dom/builder/shared/ElementBuilderBase.java:214:
* String based implementation will escape the text to prevent
HTML/javascript
"A string-based implementation"
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/ElementBuilderFactory.java
File
user/src/com/google/gwt/dom/builder/shared/ElementBuilderFactory.java
(right):
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/ElementBuilderFactory.java#newcode133
user/src/com/google/gwt/dom/builder/shared/ElementBuilderFactory.java:133:
public abstract ElementBuilder
createElementBuilderFromTrustedTagName(String tagName);
On 2011/06/24 17:40:01, jlabanca wrote:
Does this method name work? createTrustedElementBuilder(String
tagName) sounds
like the return value is trusted rather than the parameter tagName.
Hopefully
it won't be used very often, as we'll provide builders for most
elements.
What do you think about trustedCreate("tag")? We seem to have a
convention going of putting "trusted" first in the names of method calls
to be checked, and that seems like enough of a heads-up to look at the
javadoc.
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/ElementBuilderImpl.java
File user/src/com/google/gwt/dom/builder/shared/ElementBuilderImpl.java
(right):
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/ElementBuilderImpl.java#newcode88
user/src/com/google/gwt/dom/builder/shared/ElementBuilderImpl.java:88:
doEndTagImpl(tagName);
It looks like this could be trustedEnd()
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/ElementBuilderImpl.java#newcode177
user/src/com/google/gwt/dom/builder/shared/ElementBuilderImpl.java:177:
* Get the {@link StylesBuilder} used to add style properties.
"used to add style properties to the current element."
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/HtmlBuilderImpl.java
File user/src/com/google/gwt/dom/builder/shared/HtmlBuilderImpl.java
(right):
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/HtmlBuilderImpl.java#newcode119
user/src/com/google/gwt/dom/builder/shared/HtmlBuilderImpl.java:119:
sb.append("</").append(escape(tagName)).append(">");
Another escape() here. Should this be trustedEnd()?
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/HtmlElementBuilderBase.java
File
user/src/com/google/gwt/dom/builder/shared/HtmlElementBuilderBase.java
(right):
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/HtmlElementBuilderBase.java#newcode87
user/src/com/google/gwt/dom/builder/shared/HtmlElementBuilderBase.java:87:
return (B) delegate.end();
I'm wondering what this cast actually does after type erasure.
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/HtmlStylesBuilder.java
File user/src/com/google/gwt/dom/builder/shared/HtmlStylesBuilder.java
(right):
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/HtmlStylesBuilder.java#newcode43
user/src/com/google/gwt/dom/builder/shared/HtmlStylesBuilder.java:43: *
Regex to match a camelCase word.
A bit unclear what this is supposed to match. Examples?
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/HtmlStylesBuilder.java#newcode298
user/src/com/google/gwt/dom/builder/shared/HtmlStylesBuilder.java:298:
private String fromCamelCase(String name) {
There will be a limited number of inputs. I wonder if we should cache
this?
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/OptionBuilder.java
File user/src/com/google/gwt/dom/builder/shared/OptionBuilder.java
(right):
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/OptionBuilder.java#newcode35
user/src/com/google/gwt/dom/builder/shared/OptionBuilder.java:35: * The
control is unavailable in this context.
"Prevents the user from selecting this option."
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/OptionBuilder.java#newcode44
user/src/com/google/gwt/dom/builder/shared/OptionBuilder.java:44: *
Option label for use in hierarchical menus.
remove "hierarchical"? Does it also get used in simple dropdown menus?
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/StylesBuilder.java
File user/src/com/google/gwt/dom/builder/shared/StylesBuilder.java
(right):
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/StylesBuilder.java#newcode35
user/src/com/google/gwt/dom/builder/shared/StylesBuilder.java:35: *
Builds the style object.
Needs a bit of explanation - which styles are we setting?
Perhaps: "Builds the style attribute on an element, or sets the
element's styles directly."
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/StylesBuilder.java#newcode40
user/src/com/google/gwt/dom/builder/shared/StylesBuilder.java:40: *
StyleBuilder uses camelCase names to be consistent with Style.
Regardless
"StyleBuilder methods take camelCase property names to be consistent"
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/StylesBuilder.java#newcode41
user/src/com/google/gwt/dom/builder/shared/StylesBuilder.java:41: * of
whether we use camcelCase of hyphenated, one of the implementations
sp: "camelCase or hyphenated"
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/StylesBuilder.java#newcode233
user/src/com/google/gwt/dom/builder/shared/StylesBuilder.java:233: *
@param value the background color
@param seems redundant?
http://gwt-code-reviews.appspot.com/1455802/diff/8002/user/src/com/google/gwt/dom/builder/shared/StylesBuilder.java#newcode308
user/src/com/google/gwt/dom/builder/shared/StylesBuilder.java:308: *
@param name the camelCase name
I was curious, so I looked at what jQuery does. It takes both formats
and converts to camelCase internally. It looks like they can convert
from dashes to camelCase in a single string.replace call.
https://github.com/jquery/jquery/blob/master/src/core.js
(Line 596.)
http://gwt-code-reviews.appspot.com/1455802/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
