Bug 41671 – Implement InTableBodyMode

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

Implement InTableBodyMode

[Adam Barth]

Created attachment 60607 [details]
Patch

[Eric Seidel (no email)]

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.

[Adam Barth]

> 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.

[Adam Barth]

Committed r62573: <http://trac.webkit.org/changeset/62573>

[Adam Barth]

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.

[Adam Barth]

Committed r62579: <http://trac.webkit.org/changeset/62579>