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

Eric Seidel (no email)
Reported 2010-07-15 02:31:47 PDT
LegacyHTMLTreeBuilder should insert an implicit <colgroup> before inserting <col> to match HTML5
Attachments
Patch (71.12 KB, patch)
2010-07-15 02:36 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Patch (63.77 KB, patch)
2010-07-15 14:32 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2010-07-15 02:35:34 PDT
Hyatt might like to see the table changes go by.
Eric Seidel (no email)
Comment 2 2010-07-15 02:36:17 PDT
Created attachment 61624 [details] Patch
Adam Barth
Comment 3 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.
Eric Seidel (no email)
Comment 4 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.
Adam Barth
Comment 5 2010-07-15 14:32:26 PDT
Created attachment 61721 [details] Patch
Adam Barth
Comment 6 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.
Adam Barth
Comment 7 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. :-/
Eric Seidel (no email)
Comment 8 2010-07-15 14:51:24 PDT
Comment on attachment 61721 [details] Patch Thanks Adam.
Adam Barth
Comment 9 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>
Adam Barth
Comment 10 2010-07-15 15:17:23 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 11 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
Adam Barth
Comment 12 2010-07-15 15:44:20 PDT
Qt in http://trac.webkit.org/changeset/63477
Note You need to log in before you can comment on or make changes to this bug.