Bug 1878797 [wpt PR 44408] - Close the dialog element when the open a…

…ttribute is removed, a=testonly Automatic update from web-platform-tests 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 <dbaronchromium.org> Commit-Queue: Joey Arhar <jarharchromium.org> Cr-Commit-Position: refs/heads/main{#1258629} -- wpt-commits: c0538d5500ef425b051f7dcb37772af5025324d0 wpt-pr: 44408 UltraBlame original commit: 1ad3db0d81f20d2319adf0d009fabe2d4b1b8ca0
marco-c · Feb 20, 2024 · d1b9209 · d1b9209
1 parent 110b0db
commit d1b9209
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 2 deletions.
diff --git a/...ts/html/semantics/interactive-elements/the-dialog-element/dialog-close-via-attribute.html b/...ts/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/...atform/tests/html/semantics/interactive-elements/the-dialog-element/dialog-showModal.html b/...atform/tests/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();