[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