video-timeupdate-duringplayback.html missing closing angle
Created attachment 58160 [details] Patch
We shouldn't land this until verifying there is an appropriate parser test. I figured I'd check with you first to see if you know where it is off-hand. If not, I'll dig around and/or add one.
You can always add another test. fast/invalid doesn't seem to cover this exact case, but I would expect html5lib/resources/* does. I would just add another test to html5lib/resources/webkit01.dat to be sure. :)
Created attachment 58173 [details] Patch
Comment on attachment 58173 [details] Patch Your added test does not test anything interesting. I think you meant </p
(In reply to comment #5) > (From update of attachment 58173 [details]) > Your added test does not test anything interesting. I think you meant </p Look at it again; it is the first "/p" that is missing the ">". The bug with the other test was that in "<p>foo</p<script ...>", we were ignoring the <script>. The point of this test is to ensure the second <p> shows up. The HTML5 parser currently produces: <html> <head> <body> <p> "Test" "Test2" I'm checking for: <html> <head> <body> <p> "Test" <p> "Test2"
Did you check the HTML5 spec/ Mozilla minefield to see which behavior is correct?
Comment on attachment 58173 [details] Patch ok
(In reply to comment #8) > (From update of attachment 58173 [details]) > ok (In reply to comment #7) > Did you check the HTML5 spec/ Mozilla minefield to see which behavior is correct? Both minefield and the spec don't allow for </foo<bar> to be parsed as the closing of foo and the opening of bar. However, the legacy webkit parser does. So I'd like to fix the video-timeupdate-duringplayback.html layout test so that it doesn't have this bad markup because that isn't at all the point of the test. But I think it is important to track this difference somehow. Should I update the expectation to pass with the HTML5 parser and fail with the legacy parser?
(In reply to comment #9) > (In reply to comment #8) > > (From update of attachment 58173 [details] [details]) > > ok > > (In reply to comment #7) > > Did you check the HTML5 spec/ Mozilla minefield to see which behavior is correct? > > Both minefield and the spec don't allow for </foo<bar> to be parsed as the closing of foo and the opening of bar. However, the legacy webkit parser does. > > So I'd like to fix the video-timeupdate-duringplayback.html layout test so that it doesn't have this bad markup because that isn't at all the point of the test. But I think it is important to track this difference somehow. Should I update the expectation to pass with the HTML5 parser and fail with the legacy parser? I see you r+'d this. But I'm not sure the test expectation is set correctly. Should it be set based on [1] the spec (less compatible, old parser fails, new one passes) or based on [2] the old behavior (more compatible, old parser passes, new one fails)? Currently this patch does #2.
The #document field of the HTML5lib tests should be set to what the spec says to do. The -expected.txt should be set to the current behavior of the old parser (so the bots stay green). the -html5-expected.txt should be set to the current behavior of the HTML5 parser so we can track regressions. I'll fix the expectations before landing.
Comment on attachment 58173 [details] Patch The new tests fails in the HTML5 parser. Interestingly, the test passes in Minefield. That means one of us is not following the spec. :) Test 15 of 15 in resources/webkit01.dat failed. Input: <p>Test</p<p>Test2</p> Got: | <html> | <head> | <body> | <p> | "Test" | "Test2" Expected: | <html> | <head> | <body> | <p> | "Test" | <p> | "Test2"
Comment on attachment 58173 [details] Patch Sorry for the confusion. I was running Minefield on the wrong instance of the tests. We match Minefield, as expected. I'll land with the corrected expectations.
Created attachment 58258 [details] Patch for landing
Committed r60900: <http://trac.webkit.org/changeset/60900>