From c825fa6f1b6fbc960b9e0199968ffb58cf55f677 Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Tue, 16 May 2023 15:21:32 -0700 Subject: [PATCH] Don't throw when popovers and dialogs are in requested state This is being changed in the HTML spec here: https://github.com/whatwg/html/pull/9142 Change-Id: Ib8aaaf314c2a1de5d082494e5172e029d531c8e8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4494823 Reviewed-by: Mason Freed Commit-Queue: Joey Arhar Cr-Commit-Position: refs/heads/main@{#1144952} --- .../dialog-no-throw-requested-state.html | 29 ++++++++++++++++ .../popovers/popover-attribute-basic.html | 16 +++++++-- .../popovers/popover-light-dismiss.html | 7 ++-- .../popovers/popover-move-documents.html | 33 +++++++++++++++---- .../popovers/resources/popover-utils.js | 6 ++-- 5 files changed, 73 insertions(+), 18 deletions(-) create mode 100644 html/semantics/interactive-elements/the-dialog-element/dialog-no-throw-requested-state.html 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 new file mode 100644 index 00000000000000..c86cbe84a62294 --- /dev/null +++ b/html/semantics/interactive-elements/the-dialog-element/dialog-no-throw-requested-state.html @@ -0,0 +1,29 @@ + + + + + + +hello + + diff --git a/html/semantics/popovers/popover-attribute-basic.html b/html/semantics/popovers/popover-attribute-basic.html index eab61407c8b387..32d3deb3848c22 100644 --- a/html/semantics/popovers/popover-attribute-basic.html +++ b/html/semantics/popovers/popover-attribute-basic.html @@ -254,11 +254,21 @@ },{once: true}); assert_true(popover.matches(':popover-open')); assert_true(other_popover.matches(':popover-open')); - assert_throws_dom('InvalidStateError', () => popover.hidePopover()); + popover.hidePopover(); // Calling hidePopover on a hidden popover should not throw. 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(); // Calling hidePopover on a hidden popover should not throw. + },`Changing the popover type in a "beforetoggle" event handler during hidePopover() should not throw an exception`); + + test(t => { + const popover = document.createElement('div'); + assert_throws_dom('NotSupportedError', () => popover.hidePopover(), + 'Calling hidePopover on an element without a popover attribute should throw.'); + popover.setAttribute('popover', 'auto'); + popover.hidePopover(); // Calling hidePopover on a disconnected popover should not throw. + assert_throws_dom('InvalidStateError', () => popover.showPopover(), + 'Calling showPopover on a disconnected popover should throw.'); + },'Calling hidePopover on a disconnected popover should not throw.'); 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 index d7d1edd3a4b1fa..4b888169e1becc 100644 --- a/html/semantics/popovers/popover-light-dismiss.html +++ b/html/semantics/popovers/popover-light-dismiss.html @@ -532,7 +532,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')); @@ -579,10 +579,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 index 9feaa4b2bf8756..11f52c2f2f0d14 100644 --- a/html/semantics/popovers/popover-move-documents.html +++ b/html/semantics/popovers/popover-move-documents.html @@ -4,10 +4,30 @@ + +
p1