Add tests for <summary>'s activation behavior

[domenic]

Follows whatwg/html#2256.

[wpt-pr-bot]

Notifying @Ms2ger, @ayg, @gsnedders, @jdm, @jgraham, @sideshowbarker, @zcorpan, and @zqzhang. (Learn how reviewing works.)

[wpt-stability-bot]

Firefox

Testing revision 2065c4f
Starting 10 test iterations
All results were stable

All results

/html/semantics/interactive-elements/the-summary-element/activation-behavior.html

Subtest	Results
	OK
Should close an open details if all conditions are met	PASS
Should open a closed details if another summary element *nested inside a span* precedes the summary	PASS
Should open a closed details if a span element precedes the summary	PASS
toggle events should be coalesced even when using the activation behavior of a summary	PASS
Should stay closed if the summary element is nested inside a span element	PASS
Should open a closed details if all conditions are met	PASS
Should stay closed if another summary element precedes the summary	PASS
Should open a closed details even if the summary is not being rendered	PASS
Should open a closed details even if the details is not being rendered	PASS

[wpt-stability-bot]

Chrome

Testing revision 5d41b89
Starting 10 test iterations

Unstable results

Test	Subtest	Results
/html/semantics/interactive-elements/the-summary-element/activation-behavior.html	Should close an open details if all conditions are met	FAIL: 3/10, PASS: 7/10
/html/semantics/interactive-elements/the-summary-element/activation-behavior.html	Should open a closed details if another summary element *nested inside a span* precedes the summary	FAIL: 3/10, PASS: 7/10
/html/semantics/interactive-elements/the-summary-element/activation-behavior.html	Should open a closed details if a span element precedes the summary	FAIL: 3/10, PASS: 7/10
/html/semantics/interactive-elements/the-summary-element/activation-behavior.html	toggle events should be coalesced even when using the activation behavior of a summary	FAIL: 3/10, PASS: 7/10
/html/semantics/interactive-elements/the-summary-element/activation-behavior.html	Should open a closed details if all conditions are met	FAIL: 3/10, PASS: 7/10

All results

/html/semantics/interactive-elements/the-summary-element/activation-behavior.html

Subtest	Results
	OK
Should close an open details if all conditions are met	FAIL: 3/10, PASS: 7/10
Should open a closed details if another summary element *nested inside a span* precedes the summary	FAIL: 3/10, PASS: 7/10
Should open a closed details if a span element precedes the summary	FAIL: 3/10, PASS: 7/10
toggle events should be coalesced even when using the activation behavior of a summary	FAIL: 3/10, PASS: 7/10
Should stay closed if the summary element is nested inside a span element	PASS
Should open a closed details if all conditions are met	FAIL: 3/10, PASS: 7/10
Should stay closed if another summary element precedes the summary	PASS
Should open a closed details even if the summary is not being rendered	FAIL
Should open a closed details even if the details is not being rendered	FAIL

[domenic]

Any ideas on what would be causing this test to be unstable in Chrome? Is the stability checker on the fritz perchance, or is this some actual bug in my tests that I just can't decipher?

[sideshowbarker]

Is the stability checker on the fritz perchance, or is this some actual bug in my tests that I just can't decipher?

The stability checker is working as designed as far as I know. At least I’m not aware of anything that’s known to have broken in it recently.

As far as the test failures, maybe @jgraham has some insight.

[zcorpan]

I can't reproduce the unstable results, having reloaded the test ~20 times. :-(

[jugglinmike]

I can confirm instability in the following contexts:

• GNU/Linux: Chromium version 57.0.2979.0 (64-bit)
• macOS: Google Chrome version 55.0.2883.95

Firefox Nightly behaves as expected on both platforms.

It looks like Chromium does not update the <details> element when a click event is triggered during initial page rendering:

<details>
  <summary id="d">Summary</summary>
  Content
</details>

<script>document.getElementById('d').click();</script>

Demo: https://bl.ocks.org/jugglinmike/e5f13288f675836e2cca8aec8d2a2424

Note that the click event is still created and propagated as expected.

The test is reported as flaky because a DOM with more elements (such as the one under test) occasionally prompts Chromium to honor the click.

Demo: https://bl.ocks.org/jugglinmike/661f878cc5448a86072c40c3d3336a93

The flakyness may be more prominent when the document is loaded directly (in other words, not from within an <iframe>): https://bl.ocks.org/jugglinmike/raw/661f878cc5448a86072c40c3d3336a93

So this may be a bug Chromium. Probably I'm overlooking some reality of expected behavior during DOM loading, but just in case this is legitimate, I've filed a report on the Chromium issue tracker: https://bugs.chromium.org/p/chromium/issues/detail?id=681711

[domenic]

Thanks so much @jugglinmike, that's super-valuable! I added my diagnosis of the Chrome issue in https://bugs.chromium.org/p/chromium/issues/detail?id=681711#c2 .

Now the question becomes, what is our strategy for merging or not merging tests that expose flakiness bugs in engines? Should I work around the Chrome bug (e.g. delay for some time to wait for the page to render and all layout objects to be created before starting the test suite), thus giving Chrome a free pass here? Or should we merge anyway, despite Travis not wanting us to? @gsnedders, @jgraham, thoughts?

[jgraham]

So the problem with flaky tests is that they will tend to just be disabled when you import them, so you end up without coverage for either the original issue or the flakiness. So unfortunately it's probably better to work around the issue in the test.

The counterargument in this case is that Chrome may not import the test immediately anyway so hopefully the bug will be fixed by the time that the test is imported into the Blink repo.

Basically I'm happy for you to take either path in this specific case.

[zcorpan]

I think if you work around a bug in a test, it would be good to have a separate test testing the bug you're working around, or at least an issue in web-platform-tests repo to add such a test.

[domenic]

I think if you work around a bug in a test, it would be good to have a separate test testing the bug you're working around, or at least an issue in web-platform-tests repo to add such a test.

Wouldn't such a separate test not pass the stability checker?

[domenic]

Since tkent fixed the suspected Chrome bug already I think we should merge this as-is. However, I don't have privileges to do so since the stability checker failed. @jgraham, is that something you can do?

[zcorpan]

Depends on how you write the test. You could repeat the unstable thing in the test say 50 times and fail the test if it ever gives an unexpected result. Or you could let it have unstable result, I suppose.

[zcorpan]

Or in some cases you can make one of the relevant things take a long time to complete, like here maybe having a slow-loading stylesheet...