corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1965052 - Only check top-most dialog for closedby in HandleEsc r=edgar,dom-core

Browse Source 
We do not need to iterate over all open dialogs, as the top-most dialog
must respond _somehow_ to the escape key press, it either closes, or it
blocks the escape key-press, disallowing outer dialogs to handle the
key-press.

Differential Revision: https://phabricator.services.mozilla.com/D248309
This commit is contained in:
Keith Cirkel
2025-05-09 16:17:06 +00:00
committed by mozilla@keithcirkel.co.uk
parent fdf96a1cd2
commit c71aafeaae
3 changed files with 136 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
dom/base/Document.cpp
View File

@@ -15029,14 +15029,17 @@ void Document::HandleEscKey() {
} }
} }
// Not all dialogs exist in the top layer, so despite already iterating // Not all dialogs exist in the top layer, so despite already iterating
// through all top layer elements we also need to iterate over non-modal // through all top layer elements we also need to check open dialogs that are
// dialogs, as they may have a specified `closedby` value which may allow them // _not_ open via the top-layer (showModal).
// to be closed via Escape key. // The top-most dialog in mOpenDialogs may need to be closed.
for (RefPtr<HTMLDialogElement> dialog : Reversed(mOpenDialogs)) { if (RefPtr<HTMLDialogElement> dialog =
mOpenDialogs.SafeLastElement(nullptr)) {
if (dialog->GetClosedBy() != HTMLDialogElement::ClosedBy::None) { if (dialog->GetClosedBy() != HTMLDialogElement::ClosedBy::None) {
MOZ_ASSERT(StaticPrefs::dom_dialog_light_dismiss_enabled(),
"Light Dismiss must have been enabled for GetClosedBy() "
"returns != ClosedBy::None");
const mozilla::dom::Optional<nsAString> returnValue; const mozilla::dom::Optional<nsAString> returnValue;
dialog->RequestClose(returnValue); dialog->RequestClose(returnValue);
return;
} }
} }
} }

1
testing/web-platform/meta/html/semantics/interactive-elements/the-dialog-element/dialog-closedby-show-stacked.html.ini Normal file
View File

@@ -0,0 +1 @@
prefs: [dom.closewatcher.enabled: true, dom.dialog.light-dismiss.enabled: true]

127
testing/web-platform/tests/html/semantics/interactive-elements/the-dialog-element/dialog-closedby-show-stacked.html Normal file
View File

@@ -0,0 +1,127 @@
<!DOCTYPE html>
<meta charset="utf-8">
<link rel="author" href="mailto:wpt@keithcirkel.co.uk">
<link rel=help href="https://html.spec.whatwg.org/multipage/interactive-elements.html#dialog-light-dismiss">
<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="../../popovers/resources/popover-utils.js"></script>
<button id="outside">Outside</button>
<dialog id=outer closedby="closerequest">
<dialog id=inner>
</dialog>
</dialog>
<script>
function awaitEvent(el, type, signal) {
const {promise, resolve} = Promise.withResolvers();
el.addEventListener(type, resolve, { once: true, signal });
return promise
}
promise_test(async (t) => {
inner.setAttribute('closedby', 'none');
outer.show();
// Need to ensure that CloseWatcher does not collapse
// both of these shows into a single CloseWatcher group.
await test_driver.bless();
inner.show();
assert_true(inner.open, "The inner dialog is open");
assert_true(outer.open, "The outer dialog is open");
let cancelFired = false;
inner.addEventListener('cancel', () => cancelFired = true, t.get_signal());
outer.addEventListener('cancel', () => cancelFired = true, t.get_signal());
const ESC = '\uE00C';
await test_driver.send_keys(document.documentElement,ESC);
assert_false(cancelFired, "The cancel event was not fired");
assert_true(inner.open, "The inner dialog is still open");
assert_true(outer.open, "The outer dialog is still open");
},'With an inner closedby=none, the outer & inner dialogs stays open when Esc is pressed');
promise_test(async (t) => {
inner.setAttribute('closedby', 'none');
outer.show();
// Need to ensure that CloseWatcher does not collapse
// both of these shows into a single CloseWatcher group.
await test_driver.bless();
inner.show();
assert_true(inner.open, "The inner dialog is open");
assert_true(outer.open, "The outer dialog is open");
let cancelFired = false;
inner.addEventListener('cancel', () => cancelFired = true, t.get_signal());
outer.addEventListener('cancel', () => cancelFired = true, t.get_signal());
// Try clicking outside
await clickOn(outside);
assert_false(cancelFired, "The cancel event was not fired");
assert_true(inner.open, "The inner dialog is open");
assert_true(outer.open, "The outer dialog is open");
},'With an inner closedby=none, the outer & inner dialogs stays open when clicked outside');
promise_test(async (t) => {
inner.setAttribute('closedby', 'any');
outer.show();
// Need to ensure that CloseWatcher does not collapse
// both of these shows into a single CloseWatcher group.
await test_driver.bless();
inner.show();
assert_true(inner.open, "The inner dialog is open");
assert_true(outer.open, "The outer dialog is open");
let innerCancelled = false;
let outerCancelled = false;
inner.addEventListener('cancel', () => innerCancelled = true, t.get_signal());
outer.addEventListener('cancel', () => outerCancelled = true, t.get_signal());
let innerClosed = awaitEvent(inner, 'close', t.get_signal());
// Try clicking outside
const ESC = '\uE00C';
await test_driver.send_keys(document.documentElement,ESC);
await innerClosed;
assert_false(outerCancelled, "The outer cancel event was not fired");
assert_true(innerCancelled, "The inner cancel event was fired");
assert_false(inner.open, "The inner dialog is NOT open");
assert_true(outer.open, "The outer dialog is open");
},'With an inner closedby=any, the outer dialog stays open but the inner dialogs should close, when Esc is pressed');
promise_test(async (t) => {
inner.setAttribute('closedby', 'any');
outer.show();
// Need to ensure that CloseWatcher does not collapse
// both of these shows into a single CloseWatcher group.
await test_driver.bless();
inner.show();
assert_true(inner.open, "The inner dialog is open");
assert_true(outer.open, "The outer dialog is open");
let innerCancelled = false;
let outerCancelled = false;
inner.addEventListener('cancel', () => innerCancelled = true, t.get_signal());
outer.addEventListener('cancel', () => outerCancelled = true, t.get_signal());
let innerClosed = awaitEvent(inner, 'close', t.get_signal());
// Try clicking outside
await clickOn(outside);
await innerClosed;
assert_false(outerCancelled, "The outer cancel event was not fired");
assert_true(innerCancelled, "The inner cancel event was fired");
assert_false(inner.open, "The inner dialog is NOT open");
assert_true(outer.open, "The outer dialog is open");
},'With an inner closedby=any, the outer dialog stays open but the inner dialogs should close, when clicked outside');
</script>