Close the dialog element when the open attribute is removed
This patch adds HTMLDialogElement::ParseAttribute which runs
HTMLDialogElement::close when the open attribute is removed to prevent a
bad state where the dialog is modal but hidden and inerting the rest of
the document.
Spec discussion is happening here:
https://github.com/whatwg/html/issues/5802
Change-Id: Ib90736ced952d7aeadc791c6863c3ac2a55deb62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5269905
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1258629}
diff --git a/html/semantics/interactive-elements/the-dialog-element/dialog-close-via-attribute.html b/html/semantics/interactive-elements/the-dialog-element/dialog-close-via-attribute.html
new file mode 100644
index 0000000..5c2e70f
--- /dev/null
+++ b/html/semantics/interactive-elements/the-dialog-element/dialog-close-via-attribute.html
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+<link rel=author href="mailto:jarhar@chromium.org">
+<link rel=help href="https://github.com/whatwg/html/issues/5802">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+
+<button>button</button>
+<dialog>hello world</dialog>
+
+<script>
+const dialog = document.querySelector('dialog');
+const button = document.querySelector('button');
+
+promise_test(async t => {
+ dialog.showModal();
+
+ let closeFired = false;
+ let cancelFired = false;
+ dialog.addEventListener('close', () => closeFired = true);
+ dialog.addEventListener('cancel', () => cancelFired = true);
+
+ dialog.removeAttribute('open');
+ await new Promise(resolve => t.step_timeout(resolve, 0));
+ await new Promise(requestAnimationFrame);
+
+ assert_false(dialog.matches(':modal'),
+ 'The dialog should not match :modal after closing.');
+ assert_false(cancelFired,
+ 'The cancel event should not fire when removing the open attribute.');
+ assert_true(closeFired,
+ 'The close event should be fired when removing the open attribute.');
+
+ let buttonFiredClick = false;
+ button.addEventListener('click', () => buttonFiredClick = true);
+ await test_driver.click(button);
+ assert_true(buttonFiredClick,
+ 'The page should not be inert or blocked after removing the open attribute.');
+}, 'Removing the open attribute from an open modal dialog should run the closing algorithm.');
+
+promise_test(async t => {
+ dialog.show();
+
+ let closeFired = false;
+ let cancelFired = false;
+ dialog.addEventListener('close', () => closeFired = true);
+ dialog.addEventListener('cancel', () => cancelFired = true);
+
+ dialog.removeAttribute('open');
+ await new Promise(resolve => t.step_timeout(resolve, 0));
+ await new Promise(requestAnimationFrame);
+
+ assert_false(cancelFired,
+ 'The cancel event should not fire when removing the open attribute.');
+ assert_true(closeFired,
+ 'The close event should be fired when removing the open attribute.');
+}, 'Removing the open attribute from an open non-modal dialog should fire a close event.');
+</script>
diff --git a/html/semantics/interactive-elements/the-dialog-element/dialog-showModal.html b/html/semantics/interactive-elements/the-dialog-element/dialog-showModal.html
index 5edff18..47612e7 100644
--- a/html/semantics/interactive-elements/the-dialog-element/dialog-showModal.html
+++ b/html/semantics/interactive-elements/the-dialog-element/dialog-showModal.html
@@ -161,11 +161,11 @@
assert_equals(topElement(), d11);
// Removing the open attribute and running through the showModal() algorithm
- // again should not promote d10 to the top.
+ // again should promote d10 to the top.
d10.removeAttribute("open");
assert_equals(topElement(), d11);
d10.showModal();
- assert_equals(topElement(), d11);
+ assert_equals(topElement(), d10);
// Closing d11 with close() should cause d10 to be the topmost element.
d11.close();
