Bug 40485 - tables/mozilla/bugs/bug1188.html needlessly depends on HTML Parser text node handling
Summary: tables/mozilla/bugs/bug1188.html needlessly depends on HTML Parser text node ...
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:
Blocks: 39259
  Show dependency tree / graph
 
Reported: 2010-06-11 11:31 PDT by Eric Seidel (no email)
Modified: 2010-06-12 00:34 PDT (History)
5 users (show)

See Also:

Attachments
Patch (3.92 KB, patch)
2010-06-11 11:42 PDT, Eric Seidel (no email) 		no flags Details | Formatted Diff | Diff
Patch for landing (1.77 KB, patch)
2010-06-11 14:30 PDT, Eric Seidel (no email) 		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-06-11 11:31:30 PDT 
tables/mozilla/bugs/bug1188.html needlessly depends on HTML Parser text node handling
Comment 1 Eric Seidel (no email) 2010-06-11 11:42:08 PDT 
Created attachment 58492 [details]
Patch
Comment 2 Eric Seidel (no email) 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.
Comment 3 Adam Barth 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?
Comment 4 Eric Seidel (no email) 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. :)
Comment 5 WebKit Commit Bot 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>
Comment 6 WebKit Commit Bot 2010-06-11 12:37:44 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 7 Adam Barth 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.
Comment 8 Eric Seidel (no email) 2010-06-11 14:11:28 PDT 
Oh true.
Comment 9 Eric Seidel (no email) 2010-06-11 14:30:53 PDT 
Created attachment 58507 [details]
Patch for landing
Comment 10 Eric Seidel (no email) 2010-06-11 14:31:18 PDT 
re-opening so the commit-queue will see this to land.
Comment 11 WebKit Commit Bot 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>
Comment 12 WebKit Commit Bot 2010-06-12 00:10:10 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 13 WebKit Review Bot 2010-06-12 00:34:13 PDT 
http://trac.webkit.org/changeset/61054 might have broken SnowLeopard Intel Release (Tests)