WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
39623
Prepare HTML5TreeBuilder for addition of new HTML5 parser code
https://bugs.webkit.org/show_bug.cgi?id=39623
Summary
Prepare HTML5TreeBuilder for addition of new HTML5 parser code
Eric Seidel (no email)
Reported
2010-05-24 15:21:30 PDT
Prepare HTML5TreeBuilder for addition of new HTML5 parser code
Attachments
Patch
(7.07 KB, patch)
2010-05-24 15:26 PDT
,
Eric Seidel (no email)
abarth
: review+
abarth
: commit-queue-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1
2010-05-24 15:26:32 PDT
Created
attachment 56935
[details]
Patch
Adam Barth
Comment 2
2010-05-24 15:32:12 PDT
Comment on
attachment 56935
[details]
Patch 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 ?
Eric Seidel (no email)
Comment 3
2010-05-24 16:00:22 PDT
(In reply to
comment #2
)
> (From update of
attachment 56935
[details]
) > 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.
AtomicString has a String operator and String has a bool operator, by their powers combined we are captain planet.
> 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.
Fixed. Added "takeCharacters()" and "takeComment" with a FIXME to remove when we move off the old parser.
> WebCore/html/HTML5TreeBuilder.cpp:116 > + for (size_t x = 0; x < characters.size(); x++) > You should use an iterator here.
Fixed.
> WebCore/html/HTML5TreeBuilder.cpp:117 > + processToken(token, characters[x]); > Maybe processCharacter ?
Nah, this is intentional. All the tokens go through the same big switch statement.
Eric Seidel (no email)
Comment 4
2010-05-24 16:16:14 PDT
Committed
r60095
: <
http://trac.webkit.org/changeset/60095
>
WebKit Review Bot
Comment 5
2010-05-24 16:23:19 PDT
http://trac.webkit.org/changeset/60095
might have broken Chromium Linux Release
Darin Adler
Comment 6
2010-05-24 17:31:54 PDT
This broke the WIndows build due to "unreachable code" warnings. It’s been broken all day. What should we do?
Eric Seidel (no email)
Comment 7
2010-05-24 17:37:34 PDT
Where "all day" == 75 minutes ago. :) Regardless, completely my fault. I'm committing a fix now. My apologies for the break.
Darin Adler
Comment 8
2010-05-24 17:38:28 PDT
(In reply to
comment #6
)
> This broke the WIndows build due to "unreachable code" warnings. It’s been broken all day. What should we do?
LOL, my mistake not all day. It was as far back as I could get on the waterfall so I thought it was longer than it was. I didn’t realize it was just a couple hours ago!
Eric Seidel (no email)
Comment 9
2010-05-24 17:53:17 PDT
Committed
r60108
: <
http://trac.webkit.org/changeset/60108
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug