Bug 42346 - LegacyHTMLTreeBuilder should insert an implicit <colgroup> before inserting <col> to match HTML5
Summary: LegacyHTMLTreeBuilder should insert an implicit <colgroup> before inserting <...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on: 42404
Blocks: 41123
  Show dependency tree / graph
 
Reported: 2010-07-15 02:31 PDT by Eric Seidel (no email)
Modified: 2010-07-15 15:44 PDT (History)
4 users (show)

See Also:

Attachments
Patch (71.12 KB, patch)
2010-07-15 02:36 PDT, Eric Seidel (no email) 		no flags Details | Formatted Diff | Diff
Patch (63.77 KB, patch)
2010-07-15 14:32 PDT, Adam Barth 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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