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: whatwg/html#5802 Change-Id: Ib90736ced952d7aeadc791c6863c3ac2a55deb62 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5269905 Reviewed-by: David Baron <[email protected]> Commit-Queue: Joey Arhar <[email protected]> Cr-Commit-Position: refs/heads/main@{#1258629}
web-platform-tests · Feb 9, 2024 · c0538d5 · c0538d5
1 parent 703bd88
commit c0538d5
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 2 deletions.
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
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+<link rel=author href="mailto:[email protected]">
+<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
@@ -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();