Bug 42346

Summary: LegacyHTMLTreeBuilder should insert an implicit <colgroup> before inserting <col> to match HTML5
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, hyatt, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on: 42404    
Bug Blocks: 41123    
Attachments:
Description Flags
Patch
none
Patch none

Description Eric Seidel (no email) 2010-07-15 02:31:47 PDT
LegacyHTMLTreeBuilder should insert an implicit <colgroup> before inserting <col> to match HTML5
Comment 1 Eric Seidel (no email) 2010-07-15 02:35:34 PDT
Hyatt might like to see the table changes go by.
Comment 2 Eric Seidel (no email) 2010-07-15 02:36:17 PDT
Created attachment 61624 [details]
Patch
Comment 3 Adam Barth 2010-07-15 02:40:23 PDT
Comment on attachment 61624 [details]
Patch

The code change looks fine, but the diffs to insertCols and deleteCols look suspect.  We need to understand more about what those tests are trying to do.
Comment 4 Eric Seidel (no email) 2010-07-15 02:43:11 PDT
Yeah, those tests now "fail" since they assume that the "col" will be a direct child of the "table".  Minefield "fails" the test the same way we do now.

Would be interesting to see why HTML5 is adding the implicit <colgroup>.  It's possible that those tests would fail in IE too.
Comment 5 Adam Barth 2010-07-15 14:32:26 PDT
Created attachment 61721 [details]
Patch
Comment 6 Adam Barth 2010-07-15 14:33:47 PDT
This is just Eric's patch with the results updated after the {insert|delete}Cols{1,2,3} tests are fixed in Bug 42404.
Comment 7 Adam Barth 2010-07-15 14:50:56 PDT
Comment on attachment 61721 [details]
Patch

I think this patch is now correct.  Might be another fun landing.  :-/
Comment 8 Eric Seidel (no email) 2010-07-15 14:51:24 PDT
Comment on attachment 61721 [details]
Patch

Thanks Adam.
Comment 9 Adam Barth 2010-07-15 15:17:16 PDT
Comment on attachment 61721 [details]
Patch

Clearing flags on attachment: 61721

Committed r63472: <http://trac.webkit.org/changeset/63472>
Comment 10 Adam Barth 2010-07-15 15:17:23 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 WebKit Review Bot 2010-07-15 15:38:49 PDT
http://trac.webkit.org/changeset/63472 might have broken Qt Linux Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/63472
http://trac.webkit.org/changeset/63473
Comment 12 Adam Barth 2010-07-15 15:44:20 PDT
Qt in http://trac.webkit.org/changeset/63477