Don't throw when popovers and dialogs are in requested state

This is being changed in the HTML spec here: whatwg/html#9142 Change-Id: Ib8aaaf314c2a1de5d082494e5172e029d531c8e8
web-platform-tests · May 1, 2023 · 6403f54 · 6403f54
1 parent e4f565a
commit 6403f54
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 15 deletions.
diff --git a/html/semantics/interactive-elements/the-dialog-element/dialog-no-throw-requested-state.html b/html/semantics/interactive-elements/the-dialog-element/dialog-no-throw-requested-state.html
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<link rel=author href="mailto:[email protected]">
+<link rel=help href="https://github.com/whatwg/html/pull/9142">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<dialog>hello</dialog>
+
+<script>
+test(() => {
+  const dialog = document.querySelector('dialog');
+
+  // calling close() on a dialog that is already closed should not throw.
+  dialog.close();
+
+  dialog.show();
+  // calling show() on a dialog that is already showing non-modal should not throw.
+  dialog.show();
+  assert_throws_dom('InvalidStateError', () => dialog.showModal(),
+    'Calling showModal() on a dialog that is already showing non-modal should throw.');
+  dialog.close();
+
+  dialog.showModal();
+  assert_throws_dom('InvalidStateError', () => dialog.show(),
+    'Calling show() on a dialog that is already showing modal should throw.');
+  // calling showModal() on a dialog that is already showing modal should not throw.
+  dialog.showModal();
+});
+</script>
diff --git a/html/semantics/popovers/popover-attribute-basic.html b/html/semantics/popovers/popover-attribute-basic.html
@@ -254,11 +254,11 @@
     },{once: true});
     assert_true(popover.matches(':popover-open'));
     assert_true(other_popover.matches(':popover-open'));
-    assert_throws_dom('InvalidStateError', () => popover.hidePopover());
+    popover.hidePopover();
     assert_false(other_popover.matches(':popover-open'),'unrelated popover is hidden');
     assert_false(popover.matches(':popover-open'),'popover is still hidden if its type changed during hide event');
-    assert_throws_dom("InvalidStateError",() => other_popover.hidePopover(),'Nested popover should already be hidden');
-  },`Changing the popover type in a "beforetoggle" event handler should throw an exception (during hidePopover())`);
+    other_popover.hidePopover();
+  },`Changing the popover type in a "beforetoggle" event handler during hidePopover() should not throw an exception`);
 
   function interpretedType(typeString,method) {
     if (validTypes.includes(typeString))

diff --git a/html/semantics/popovers/popover-light-dismiss.html b/html/semantics/popovers/popover-light-dismiss.html
@@ -530,7 +530,7 @@
     p14.hidePopover();
   },{once:true});
   assert_true(p13.matches(':popover-open') && p14.matches(':popover-open') && p15.matches(':popover-open'),'all three should be open');
-  assert_throws_dom('InvalidStateError',() => p14.hidePopover(),'should throw because the event listener has already hidden the popover');
+  p14.hidePopover();
   assert_true(p13.matches(':popover-open'),'p13 should still be open');
   assert_false(p14.matches(':popover-open'));
   assert_false(p15.matches(':popover-open'));
@@ -577,10 +577,7 @@
     p20.showPopover();
   });
   p20.addEventListener('beforetoggle', logEvents);
-  // Because the `beforetoggle` handler shows a different popover,
-  // and that action closes the p19 popover, the call to hidePopover()
-  // will result in an exception.
-  assert_throws_dom('InvalidStateError',() => p19.hidePopover());
+  p19.hidePopover();
   assert_array_equals(events,['hide p19','show p20'],'There should not be a second hide event for 19');
   assert_false(p19.matches(':popover-open'));
   assert_true(p20.matches(':popover-open'));

diff --git a/html/semantics/popovers/popover-move-documents.html b/html/semantics/popovers/popover-move-documents.html
@@ -27,10 +27,7 @@
   assert_true(p2.matches(':popover-open'),
     'The popover should be open after calling showPopover()');
 
-  // Because the `beforetoggle` handler changes the document,
-  // and that action closes the popover, the call to hidePopover()
-  // will result in an exception.
-  assert_throws_dom('InvalidStateError',() => p2.hidePopover());
+  p2.hidePopover();
   assert_false(p2.matches(':popover-open'),
     'The popover should be closed after moving it between documents.');
 }, 'Moving popovers between documents while hiding should not throw an exception.');

diff --git a/html/semantics/popovers/resources/popover-utils.js b/html/semantics/popovers/resources/popover-utils.js
@@ -132,10 +132,10 @@ function assertIsFunctionalPopover(popover, checkVisibility) {
   assertPopoverVisibility(popover, /*isPopover*/true, /*expectedVisibility*/false, 'A popover should start out hidden');
   popover.showPopover();
   if (checkVisibility) assertPopoverVisibility(popover, /*isPopover*/true, /*expectedVisibility*/true, 'After showPopover(), a popover should be visible');
-  assert_throws_dom("InvalidStateError",() => popover.showPopover(),'Calling showPopover on a showing popover should throw InvalidStateError');
+  popover.showPopover(); // Calling showPopover on a showing popover should not throw.
   popover.hidePopover();
   if (checkVisibility) assertPopoverVisibility(popover, /*isPopover*/true, /*expectedVisibility*/false, 'After hidePopover(), a popover should be hidden');
-  assert_throws_dom("InvalidStateError",() => popover.hidePopover(),'Calling hidePopover on a hidden popover should throw InvalidStateError');
+  popover.hidePopover(); // Calling hidePopover on a hidden popover should not throw.
   popover.togglePopover();
   if (checkVisibility) assertPopoverVisibility(popover, /*isPopover*/true, /*expectedVisibility*/true, 'After togglePopover() on hidden popover, it should be visible');
   popover.togglePopover();
@@ -151,7 +151,7 @@ function assertIsFunctionalPopover(popover, checkVisibility) {
   const parent = popover.parentElement;
   popover.remove();
   assert_throws_dom("InvalidStateError",() => popover.showPopover(),'Calling showPopover on a disconnected popover should throw InvalidStateError');
-  assert_throws_dom("InvalidStateError",() => popover.hidePopover(),'Calling hidePopover on a disconnected popover should throw InvalidStateError');
+  popover.hidePopover(); // Calling hidePopover on a disconnected popover should not throw.
   assert_throws_dom("InvalidStateError",() => popover.togglePopover(),'Calling hidePopover on a disconnected popover should throw InvalidStateError');
   parent.appendChild(popover);
 }