Fix popover crash bug
It was previously possible to hit a CHECK (and I think some
underspecified behavior) with something like this:
<div popover id=p1>Popover 1
<div popover id=p2>Popover 2</div>
</div>
<script>
p1.showPopover();
p1.addEventListener('beforetoggle',() => p2.showPopover());
p1.hidePopover();
</script>
The problem is that "hide all popovers until" doesn't end up with
the desired "until" popover on the top of the stack in this case.
There is already a similar situation within the "hide all..."
algorithm itself, but that only handles the case where a popover
being hidden by "hide all..." has the beforetoggle listener. This
is the same problem, but for the case that the "until" popover
has that listener.
I believe this needs a spec update. WebKit doesn't crash (likely
it doesn't have the same CHECKs that Blink has), but also doesn't
result in both p1 and p2 being hidden in the above example.
Fixed: 1513282
Change-Id: I463888096db9a8983d355f246991bbbad596d1f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5141430
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1246585}
diff --git a/html/semantics/popovers/popover-open-in-beforetoggle.html b/html/semantics/popovers/popover-open-in-beforetoggle.html
new file mode 100644
index 0000000..1e22b73
--- /dev/null
+++ b/html/semantics/popovers/popover-open-in-beforetoggle.html
@@ -0,0 +1,66 @@
+<!DOCTYPE html>
+<meta charset="utf-8" />
+<title>Popover beforetoggle event opening new popovers</title>
+<link rel="author" href="mailto:masonf@chromium.org">
+<link rel=help href="https://html.spec.whatwg.org/multipage/indices.html#event-beforetoggle">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-actions.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<script src="resources/popover-utils.js"></script>
+
+<div popover id=p1>Popover 1
+ <div popover id=p2>Popover 2
+ <div popover id=p3>Popover 3</div>
+ </div>
+</div>
+<div id=outside>Outside</div>
+<dialog id=dialog>Dialog</dialog>
+
+<script>
+ function getSignal(t) {
+ const controller = new AbortController();
+ t.add_cleanup(() => controller.abort());
+ return controller.signal;
+ }
+
+ test((t) => {
+ p1.showPopover();
+ p1.addEventListener('beforetoggle',() => p2.showPopover(),{signal: getSignal(t)});
+ p1.hidePopover(); // Potential crash here
+ assert_false(p1.matches(':popover-open'));
+ assert_false(p2.matches(':popover-open'));
+ },'Open popover from closing beforetoggle event');
+
+ test((t) => {
+ p1.showPopover();
+ p1.addEventListener('beforetoggle',() => p2.showPopover(),{signal: getSignal(t)});
+ p2.addEventListener('beforetoggle',() => p3.showPopover(),{signal: getSignal(t)});
+ p1.hidePopover(); // Potential crash here
+ assert_false(p1.matches(':popover-open'));
+ assert_false(p2.matches(':popover-open'));
+ assert_false(p3.matches(':popover-open'));
+ },'Open double-nested popovers from closing beforetoggle event');
+
+ test(t => {
+ p1.showPopover();
+ p1.addEventListener('beforetoggle',() => p2.showPopover(),{signal: getSignal(t)});
+ p2.addEventListener('beforetoggle',() => p3.showPopover(),{signal: getSignal(t)});
+ dialog.showModal(); // Potential crash here
+ assert_false(p1.matches(':popover-open'));
+ assert_false(p2.matches(':popover-open'));
+ assert_false(p3.matches(':popover-open'));
+ dialog.close();
+ },'Open double-nested popovers from closing beforetoggle event, dialog open');
+
+ promise_test(async t => {
+ p1.showPopover();
+ p1.addEventListener('beforetoggle',() => p2.showPopover(),{signal: getSignal(t)});
+ p2.addEventListener('beforetoggle',() => p3.showPopover(),{signal: getSignal(t)});
+ await clickOn(outside); // Potential crash here
+ assert_false(p1.matches(':popover-open'));
+ assert_false(p2.matches(':popover-open'));
+ assert_false(p3.matches(':popover-open'));
+ },'Open double-nested popovers from closing beforetoggle event, light dismiss');
+</script>
diff --git a/html/semantics/popovers/popover-open-in-event-crash.html b/html/semantics/popovers/popover-open-in-event-crash.html
deleted file mode 100644
index 0a86238..0000000
--- a/html/semantics/popovers/popover-open-in-event-crash.html
+++ /dev/null
@@ -1,20 +0,0 @@
-<!DOCTYPE html>
-<meta charset="utf-8" />
-<link rel="author" href="mailto:masonf@chromium.org">
-<link rel=help href="https://html.spec.whatwg.org/multipage/popover.html#attr-popover">
-
-<div popover id=p1>Popover 1
- <div popover id=p2>Popover 2</div>
-</div>
-<script>
- const p1 = document.querySelector('#p1');
- const p2 = document.querySelector('#p2');
- p1.addEventListener('beforetoggle',e => {
- if (e.newState === "closed") {
- p2.showPopover();
- }
- })
- p1.showPopover();
- p1.hidePopover();
- // This test passes if it does not crash
-</script>
