Bug 67725 – css2.1/t090204-display-change-01-b-ao.html is not attaching event listener properly

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Ryosuke Niwa]

Per discussion with Dave, we found that css2.1/t090204-display-change-01-b-ao.html  is trying to attach load event listener on document instead of window. We should fix that.

[Simon Fraser (smfr)]

This should be reported on [e-mail]

[Ryosuke Niwa]

Created attachment 106618 [details]
fixes the test

[Ryosuke Niwa]

(In reply to comment #1)
> This should be reported on [e-mail]

That's a good point but I'm not subscribed to that mailing list. Could you or someone else familiar with the matter do that on behalf of me?

[Dave Hyatt]

Comment on attachment 106618 [details]
fixes the test

r=me

[Eric Seidel (no email)]

http://www.w3.org/Bugs/Public/ might help :)

[Ryosuke Niwa]

Committed r94696: <http://trac.webkit.org/changeset/94696>

[Ryosuke Niwa]

(In reply to comment #5)
> http://www.w3.org/Bugs/Public/ might help :)

Ah, thanks for the pointer. Filed http://www.w3.org/Bugs/Public/show_bug.cgi?id=14064.

Is it no longer the policy to not modify third-party test suites?

[Simon Fraser (smfr)]

I don't see this test in the the current 2.1 test suite: <http://test.csswg.org/suites/css2.1/20110111/html4/toc.html>

[Ryosuke Niwa]

(In reply to comment #8)
> Is it no longer the policy to not modify third-party test suites?

(In reply to comment #9)
> I don't see this test in the the current 2.1 test suite: <http://test.csswg.org/suites/css2.1/20110111/html4/toc.html>

I am confused now. What is the correct solution then?

[Simon Fraser (smfr)]

We have an old snapshot of the CSS 2.1 suite. We should import the (huge) current one.

(In reply to comment #10)
> (In reply to comment #8)
> > Is it no longer the policy to not modify third-party test suites?
> 
> (In reply to comment #9)
> > I don't see this test in the the current 2.1 test suite: <http://test.csswg.org/suites/css2.1/20110111/html4/toc.html>
> 
> I am confused now. What is the correct solution then?

Unless the bug in the test is causing it to (a) crash or hang the test tool or (b) produce inconsistent results, I think the change should be reverted. If the bug in the test is causing it to be invalid or not cover what it is supposed to test, then the solution is to add a valid test somewhere outside the css2.1 directory.

[Darin Adler]

(In reply to comment #12)
> (In reply to comment #10)
> > (In reply to comment #8)
> > > Is it no longer the policy to not modify third-party test suites?
> > 
> > (In reply to comment #9)
> > > I don't see this test in the the current 2.1 test suite: <http://test.csswg.org/suites/css2.1/20110111/html4/toc.html>
> > 
> > I am confused now. What is the correct solution then?
> 
> Unless the bug in the test is causing it to (a) crash or hang the test tool or (b) produce inconsistent results, I think the change should be reverted. If the bug in the test is causing it to be invalid or not cover what it is supposed to test, then the solution is to add a valid test somewhere outside the css2.1 directory.

That’s right.

[Darin Adler]

(In reply to comment #11)
> We have an old snapshot of the CSS 2.1 suite. We should import the (huge) current one.

Also a great idea.

[Ryosuke Niwa]

(In reply to comment #12)
> Unless the bug in the test is causing it to (a) crash or hang the test tool or (b) produce inconsistent results, I think the change should be reverted. If the bug in the test is causing it to be invalid or not cover what it is supposed to test, then the solution is to add a valid test somewhere outside the css2.1 directory.

Okay. Then let's revert this change and add a fixed test in fast/css.

[Ryosuke Niwa]

i.e. I'm going to swap css2.1/t090204-display-change-01-b-ao.html and fast/css/line-after-floating-div.html.

[Ryosuke Niwa]

Created attachment 106667 [details]
swaps line-after-floating-div.html and t090204-display-change-01-b-ao.html

[Ryosuke Niwa]

Reopen the bug.

[Ryosuke Niwa]

Created attachment 106668 [details]
Fixed the change log

[Ryosuke Niwa]

Thanks for the timely review. Will land now.

[Ryosuke Niwa]

Committed r94724: <http://trac.webkit.org/changeset/94724>