[webkit-reviews] review granted: [Bug 39623] Prepare HTML5TreeBuilder for addition of new HTML5 parser code : [Attachment 56935] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 24 15:32:11 PDT 2010


Adam Barth <abarth at webkit.org> has granted Eric Seidel <eric at webkit.org>'s
request for review:
Bug 39623: Prepare HTML5TreeBuilder for addition of new HTML5 parser code
https://bugs.webkit.org/show_bug.cgi?id=39623

Attachment 56935: Patch
https://bugs.webkit.org/attachment.cgi?id=56935&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
Please fix the below before landing.

WebCore/html/HTML5Token.h:163
 +	    ASSERT(!m_dataAsNameAtom); // An attempt to make sure this isn't
called twice.
I'm surprised this compiles.

WebCore/html/HTML5TreeBuilder.cpp:87
 +	    oldStyleToken.text = token.adoptDataAsStringImpl();
This breaks the abstraction.  The client isn't supposed to know that
characters() is backed by the m_data member variable.  Also, we lost the ASSERT
about the type of the token.

WebCore/html/HTML5TreeBuilder.cpp:116
 +	    for (size_t x = 0; x < characters.size(); x++)
You should use an iterator here.

WebCore/html/HTML5TreeBuilder.cpp:117
 +		processToken(token, characters[x]);
Maybe processCharacter ?


More information about the webkit-reviews mailing list