Summary: | Implement InTableBodyMode | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||
Component: | New Bugs | Assignee: | Adam Barth <abarth> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | eric | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Other | ||||||
OS: | OS X 10.5 | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 41123 | ||||||
Attachments: |
|
Description
Adam Barth
2010-07-06 02:37:53 PDT
Created attachment 60607 [details]
Patch
Comment on attachment 60607 [details]
Patch
WebCore/html/HTMLTreeBuilder.cpp:772
+ ASSERT(InColumnGroupMode);
I've considered embedding this ASSERT in some variant of processFake*.
WebCore/html/HTMLTreeBuilder.cpp:808
+ if (currentElement()->hasTagName(tableTag) || currentElement()->hasTagName(tbodyTag) || currentElement()->hasTagName(tfootTag) || currentElement()->hasTagName(theadTag) || currentElement()->hasTagName(trTag))
I would think we would already have an inline for this?
WebCore/html/HTMLTreeBuilder.cpp:940
+ ASSERT(currentElement()->tagQName() == tbodyTag || currentElement()->tagQName() == tfootTag || currentElement()->tagQName() == theadTag);
We ceratinly already have an inline for this, no?
WebCore/html/HTMLTreeBuilder.cpp:1597
+ case InTableBodyMode:
Why not just start this one off as its own function instead of in the switch? Since we seem to be moving to that pattern...
WebCore/html/HTMLTreeBuilder.cpp:1599
+ if (token.name() == tbodyTag || token.name() == tfootTag || token.name() == theadTag) {
We totally have an inline for this.
WebCore/html/HTMLTreeBuilder.cpp:1611
+ if (!m_openElements.inTableScope(tbodyTag.localName()) && !m_openElements.inTableScope(theadTag.localName()) && !m_openElements.inTableScope(tfootTag.localName())) {
You do this twice, seems we should make an function for this check, so we only have one place to make more efficient later.
WebCore/html/HTMLTreeBuilder.cpp:1617
+ ASSERT(currentElement()->tagQName() == tbodyTag || currentElement()->tagQName() == tfootTag || currentElement()->tagQName() == theadTag);
Again, if we don't have an inline for this, we should.
WebCore/html/HTMLTreeBuilder.cpp:1622
+ if (token.name() == bodyTag || token.name() == captionTag || token.name() == colTag || token.name() == colgroupTag || token.name() == htmlTag || token.name() == tdTag || token.name() == thTag || token.name() == trTag) {
Here too... :)
This looks OK, but really needs some more code sharing.
> WebCore/html/HTMLTreeBuilder.cpp:1597
> + case InTableBodyMode:
> Why not just start this one off as its own function instead of in the switch? Since we seem to be moving to that pattern...
I've only split out the insertion modes that need to be called by other insertion modes. I haven't seen any other modes that need to call InTableBodyMode. We could move them all out for consistency, but that seems like something to do during a cleanup phase.
Committed r62573: <http://trac.webkit.org/changeset/62573> Comment on attachment 60607 [details]
Patch
I totally forgot to address your comments. I got distracted merging the conflicts. I'll do a follow up patch.
Committed r62579: <http://trac.webkit.org/changeset/62579> |