Bug 40485

Summary: tables/mozilla/bugs/bug1188.html needlessly depends on HTML Parser text node handling
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, eric, tonyg, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 39259    
Attachments:
Description Flags
Patch
none
Patch for landing none

Eric Seidel (no email)
Reported 2010-06-11 11:31:30 PDT
tables/mozilla/bugs/bug1188.html needlessly depends on HTML Parser text node handling
Attachments
Patch (3.92 KB, patch)
2010-06-11 11:42 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Patch for landing (1.77 KB, patch)
2010-06-11 14:30 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2010-06-11 11:42:08 PDT
Created attachment 58492 [details] Patch
Eric Seidel (no email)
Comment 2 2010-06-11 11:43:49 PDT
Comment on attachment 58492 [details] Patch I've validated with hyatt and by reading https://bugzilla.mozilla.org/show_bug.cgi?id=1188 that it's OK to modify this test and that the modification does not change what the test was testing. As far as I can understand, the </table> in the table was causing some sort of corruption of mozilla's column cache.
Adam Barth
Comment 3 2010-06-11 12:20:11 PDT
Comment on attachment 58492 [details] Patch Ok. Shouldn't the new test fail in test-html5-parser? If so, where's the updated expectation?
Eric Seidel (no email)
Comment 4 2010-06-11 12:25:11 PDT
Comment on attachment 58492 [details] Patch Nope. It fails with the new parser, but not the old one. The old one gets this case "right". The one one doesn't correctly coalesce text nodes yet. But for now that's an acceptable "regression" in behavior. Once we add text node coalescing we'll likely break more tests, but in the other direction. :)
WebKit Commit Bot
Comment 5 2010-06-11 12:37:38 PDT
Comment on attachment 58492 [details] Patch Clearing flags on attachment: 58492 Committed r61033: <http://trac.webkit.org/changeset/61033>
WebKit Commit Bot
Comment 6 2010-06-11 12:37:44 PDT
All reviewed patches have been landed. Closing bug.
Adam Barth
Comment 7 2010-06-11 14:03:19 PDT
Right, but test-html5-parser tests the new parser. You need to update the -html5-expected.txt results or else you're not testing what you think you're testing.
Eric Seidel (no email)
Comment 8 2010-06-11 14:11:28 PDT
Oh true.
Eric Seidel (no email)
Comment 9 2010-06-11 14:30:53 PDT
Created attachment 58507 [details] Patch for landing
Eric Seidel (no email)
Comment 10 2010-06-11 14:31:18 PDT
re-opening so the commit-queue will see this to land.
WebKit Commit Bot
Comment 11 2010-06-12 00:10:03 PDT
Comment on attachment 58507 [details] Patch for landing Clearing flags on attachment: 58507 Committed r61054: <http://trac.webkit.org/changeset/61054>
WebKit Commit Bot
Comment 12 2010-06-12 00:10:10 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 13 2010-06-12 00:34:13 PDT
http://trac.webkit.org/changeset/61054 might have broken SnowLeopard Intel Release (Tests)
Note You need to log in before you can comment on or make changes to this bug.