Bug 42804

Summary: Enable HTML5 tree builder
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, eric, peter, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 41123    
Attachments:
Description Flags
Patch
none
Patch
none
Patch abarth: review+

Adam Barth
Reported 2010-07-21 21:50:34 PDT
Enabled HTML5 tree builder
Attachments
Patch (510.82 KB, patch)
2010-07-21 21:52 PDT, Adam Barth
no flags
Patch (1.99 KB, patch)
2010-08-04 18:17 PDT, Adam Barth
no flags
Patch (817.53 KB, patch)
2010-08-04 19:27 PDT, Eric Seidel (no email)
abarth: review+
Adam Barth
Comment 1 2010-07-21 21:52:18 PDT
Adam Barth
Comment 2 2010-07-21 21:55:57 PDT
The diffs to the test expectations are hard to read. :(
Eric Seidel (no email)
Comment 3 2010-07-22 05:12:14 PDT
Comment on attachment 62263 [details] Patch LayoutTests/ChangeLog:8 + Update test expectations. Would be nice to explain the various changes in the ChangeLog. We certainly hav that data. LayoutTests/ChangeLog:10 + * dom/html/level2/html/HTMLIsIndexElement01-expected.txt: e.g. these fail because there is no longer ever a parser-created <isindex> element, per HTML5. LayoutTests/fast/dom/no-elements-expected.txt:1 + This tests that the elements noframes, nolayer, noscript and noembed are created as elements and put in the DOM tree. The elements other than <nolayer> should not contain any children. If the test is successful, the four messages below will list 0, 0, 0, and 1 child. This text needs update, no? LayoutTests/fast/events/tabindex-focus-blur-all-expected.txt:1 + 333 focus / 333 blur events dispatched, and should be 337 / 337 PASSED I don't understand why this is changing? LayoutTests/fast/forms/datalist-nonoption-child-expected.txt:8 + FAIL datalist.firstChild.nodeName should be #text. Was DIV. And this? LayoutTests/platform/mac/editing/selection/designmode-no-caret-expected.txt:20 + caret: position 0 of child 0 {#text} of body This is due to text node coalescing? LayoutTests/platform/mac/fast/events/focusingUnloadedFrame-expected.txt:22 + layer at (0,0) size 8x8 This looks like a quirks/non-quirks mode change. is this intentional? I think this is great! But if we want this reviewed, we need to post more explanation in the ChnageLog as to why each test is changing. Certainly it's difficult for me to review as-is, and probably impossible for anyone else to review with understanding of what's changing.
Adam Barth
Comment 4 2010-07-22 09:48:46 PDT
> Would be nice to explain the various changes in the ChangeLog. We certainly hav that data. Ok. > e.g. these fail because there is no longer ever a parser-created <isindex> element, per HTML5. Yep. > LayoutTests/fast/dom/no-elements-expected.txt:1 > + This tests that the elements noframes, nolayer, noscript and noembed are created as elements and put in the DOM tree. The elements other than <nolayer> should not contain any children. If the test is successful, the four messages below will list 0, 0, 0, and 1 child. > This text needs update, no? Ok. > LayoutTests/fast/events/tabindex-focus-blur-all-expected.txt:1 > + 333 focus / 333 blur events dispatched, and should be 337 / 337 PASSED > I don't understand why this is changing? I don't quite understand why it's changing either. I'll spend some time investigating, but this test is really scattershot. Hopefully there's a reduction. > LayoutTests/fast/forms/datalist-nonoption-child-expected.txt:8 > + FAIL datalist.firstChild.nodeName should be #text. Was DIV. > And this? datalist used to have magical custom parsing. > LayoutTests/platform/mac/editing/selection/designmode-no-caret-expected.txt:20 > + caret: position 0 of child 0 {#text} of body > This is due to text node coalescing? I believe so. > LayoutTests/platform/mac/fast/events/focusingUnloadedFrame-expected.txt:22 > + layer at (0,0) size 8x8 > This looks like a quirks/non-quirks mode change. is this intentional? Oh, I hadn't considered that. I'll check document.compatMode. > I think this is great! But if we want this reviewed, we need to post more explanation in the ChnageLog as to why each test is changing. Certainly it's difficult for me to review as-is, and probably impossible for anyone else to review with understanding of what's changing. It's very difficult to understand what's changing. Maybe we need some more incremental patches that move the old parser to be closer to the new parser?
Adam Barth
Comment 5 2010-08-04 18:17:34 PDT
Eric Seidel (no email)
Comment 6 2010-08-04 18:20:31 PDT
Comment on attachment 63524 [details] Patch Although I think it's kinda cool to do this w/o results. I think it would be better to include at least the non-platform specific results.
Eric Seidel (no email)
Comment 7 2010-08-04 19:27:40 PDT
Adam Barth
Comment 8 2010-08-04 19:29:56 PDT
Comment on attachment 63535 [details] Patch Okiedokes. Fingers crossed.
WebKit Review Bot
Comment 9 2010-08-04 19:30:27 PDT
Attachment 63535 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 LayoutTests/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] LayoutTests/ChangeLog:9: Line contains tab character. [whitespace/tab] [5] LayoutTests/ChangeLog:10: Line contains tab character. [whitespace/tab] [5] LayoutTests/ChangeLog:11: Line contains tab character. [whitespace/tab] [5] WebCore/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] WebCore/ChangeLog:9: Line contains tab character. [whitespace/tab] [5] Total errors found: 6 in 80 files If any of these errors are false positives, please file a bug against check-webkit-style.
Adam Barth
Comment 10 2010-08-04 22:58:04 PDT
Adam Barth
Comment 11 2010-08-04 23:16:19 PDT
WebKit Review Bot
Comment 12 2010-08-04 23:16:57 PDT
http://trac.webkit.org/changeset/64712 might have broken Qt Linux Release
Note You need to log in before you can comment on or make changes to this bug.