diff --git a/LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-no-throw-requested-state-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-no-throw-requested-state-expected.txt new file mode 100644 index 0000000000000..64762b2618297 --- /dev/null +++ b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-no-throw-requested-state-expected.txt @@ -0,0 +1,4 @@ +hello + +PASS dialog-no-throw-requested-state + diff --git a/LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-no-throw-requested-state.html b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-no-throw-requested-state.html new file mode 100644 index 0000000000000..0f00956811150 --- /dev/null +++ b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-no-throw-requested-state.html @@ -0,0 +1,24 @@ + + + + + +hello + diff --git a/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-attribute-basic.html b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-attribute-basic.html index 335f817e75d8f..eb15ab6949af2 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-attribute-basic.html +++ b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-attribute-basic.html @@ -254,10 +254,10 @@ },{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'); + other_popover.hidePopover(); },`Changing the popover type in a "beforetoggle" event handler should throw an exception (during hidePopover())`); function interpretedType(typeString,method) { diff --git a/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss.html b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss.html index 74b4dc6536911..02781f71b22f3 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss.html +++ b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss.html @@ -531,7 +531,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')); diff --git a/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-move-documents.html b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-move-documents.html index 9feaa4b2bf875..2ead18a2b7395 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-move-documents.html +++ b/LayoutTests/imported/w3c/web-platform-tests/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/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/resources/popover-utils.js b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/resources/popover-utils.js index 04d2a2cbe75ef..a1b23c443c0b6 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/resources/popover-utils.js +++ b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/resources/popover-utils.js @@ -13,7 +13,7 @@ async function clickOn(element) { async function sendTab() { await waitForRender(); const kTab = '\uE004'; - await new test_driver.send_keys(document.body,kTab); + await new test_driver.send_keys(document.documentElement,kTab); await waitForRender(); } // Waiting for crbug.com/893480: @@ -31,12 +31,12 @@ async function sendTab() { // } async function sendEscape() { await waitForRender(); - await new test_driver.send_keys(document.body,'\uE00C'); // Escape + await new test_driver.send_keys(document.documentElement,'\uE00C'); // Escape await waitForRender(); } async function sendEnter() { await waitForRender(); - await new test_driver.send_keys(document.body,'\uE007'); // Enter + await new test_driver.send_keys(document.documentElement,'\uE007'); // Enter await waitForRender(); } function isElementVisible(el) { @@ -114,7 +114,6 @@ function popoverHintSupported() { testElement.popover = 'hint'; return testElement.popover === 'hint'; } - function assertPopoverVisibility(popover, isPopover, expectedVisibility, message) { const isVisible = isElementVisible(popover); assert_equals(isVisible, expectedVisibility,`${message}: Expected this element to be ${expectedVisibility ? "visible" : "not visible"}`); @@ -127,15 +126,14 @@ function assertPopoverVisibility(popover, isPopover, expectedVisibility, message assert_false(popover.matches(':popover-open'),`${message}: Non-showing popovers should *not* match :popover-open`); } } - 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,11 +149,10 @@ 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); } - function assertNotAPopover(nonPopover) { // If the non-popover element nonetheless has a 'popover' attribute, it should // be invisible. Otherwise, it should be visible. diff --git a/LayoutTests/platform/ios/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt b/LayoutTests/platform/ios/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt index c8c777a386522..d2fd15e767b50 100644 --- a/LayoutTests/platform/ios/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt +++ b/LayoutTests/platform/ios/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt @@ -10,11 +10,11 @@ PASS Clicking inside a child popover shouldn't close either popover FAIL Clicking inside a parent popover should close child popover assert_false: expected false got true PASS Clicking on invoking element, after using it for activation, shouldn't close its popover FAIL Clicking on invoking element, after using it for activation, shouldn't close its popover (nested case) assert_true: button2 should activate popover2 expected true got false -FAIL Clicking on invoking element, after using it for activation, shouldn't close its popover (nested case, not used for invocation) promise_test: Unhandled rejection with value: object "InvalidStateError: Element has unexpected visibility state" -FAIL Clicking on invoking element, even if it wasn't used for activation, shouldn't close its popover promise_test: Unhandled rejection with value: object "InvalidStateError: Element has unexpected visibility state" -FAIL Clicking on popovertarget element, even if it wasn't used for activation, should hide it exactly once promise_test: Unhandled rejection with value: object "InvalidStateError: Element has unexpected visibility state" -FAIL Clicking on anchor element (that isn't an invoking element) shouldn't prevent its popover from being closed promise_test: Unhandled rejection with value: object "InvalidStateError: Element has unexpected visibility state" -FAIL Dragging from an open popover outside an open popover should leave the popover open promise_test: Unhandled rejection with value: object "InvalidStateError: Element has unexpected visibility state" +PASS Clicking on invoking element, after using it for activation, shouldn't close its popover (nested case, not used for invocation) +PASS Clicking on invoking element, even if it wasn't used for activation, shouldn't close its popover +FAIL Clicking on popovertarget element, even if it wasn't used for activation, should hide it exactly once assert_false: popover1 should be hidden by popovertarget expected false got true +FAIL Clicking on anchor element (that isn't an invoking element) shouldn't prevent its popover from being closed assert_false: popover1 should close expected false got true +PASS Dragging from an open popover outside an open popover should leave the popover open FAIL A popover inside an invoking element doesn't participate in that invoker's ancestor chain assert_true: invoking element should open popover expected true got false PASS An invoking element that was not used to invoke the popover can still be part of the ancestor chain FAIL Scrolling within a popover should not close the popover promise_test: Unhandled rejection with value: object "Error: Unknown source type "wheel"." diff --git a/Source/WebCore/html/HTMLDialogElement.cpp b/Source/WebCore/html/HTMLDialogElement.cpp index 8c68bad93eda1..4ad6d5c1d8dd9 100644 --- a/Source/WebCore/html/HTMLDialogElement.cpp +++ b/Source/WebCore/html/HTMLDialogElement.cpp @@ -53,8 +53,11 @@ HTMLDialogElement::HTMLDialogElement(const QualifiedName& tagName, Document& doc ExceptionOr HTMLDialogElement::show() { // If the element already has an open attribute, then return. - if (isOpen()) - return { }; + if (isOpen()) { + if (!isModal()) + return { }; + return Exception { InvalidStateError, "Cannot call show() on an open modal dialog."_s }; + } if (popoverData() && popoverData()->visibilityState() == PopoverVisibilityState::Showing) return Exception { InvalidStateError, "Element is already an open popover."_s }; @@ -72,8 +75,11 @@ ExceptionOr HTMLDialogElement::show() ExceptionOr HTMLDialogElement::showModal() { // If subject already has an open attribute, then throw an "InvalidStateError" DOMException. - if (isOpen()) - return Exception { InvalidStateError, "Element is already open."_s }; + if (isOpen()) { + if (isModal()) + return { }; + return Exception { InvalidStateError, "Cannot call showModal() on an open non-modal dialog."_s }; + } // If subject is not connected, then throw an "InvalidStateError" DOMException. if (!isConnected()) diff --git a/Source/WebCore/html/HTMLElement.cpp b/Source/WebCore/html/HTMLElement.cpp index b7c0c6a359423..06103df779714 100644 --- a/Source/WebCore/html/HTMLElement.cpp +++ b/Source/WebCore/html/HTMLElement.cpp @@ -1239,20 +1239,20 @@ ExceptionOr> HTMLElement::attachInternals() return ElementInternals::create(*this); } -static ExceptionOr checkPopoverValidity(HTMLElement& element, PopoverVisibilityState expectedState, Document* expectedDocument = nullptr) +static ExceptionOr checkPopoverValidity(HTMLElement& element, PopoverVisibilityState expectedState, Document* expectedDocument = nullptr) { if (element.popoverState() == PopoverState::None) return Exception { NotSupportedError, "Element does not have the popover attribute"_s }; + if (element.popoverData()->visibilityState() != expectedState) + return false; + if (!element.isConnected()) return Exception { InvalidStateError, "Element is not connected"_s }; if (expectedDocument && element.document() != *expectedDocument) return Exception { InvalidStateError, "Invalid when the document changes while showing or hiding a popover element"_s }; - if (element.popoverData()->visibilityState() != expectedState) - return Exception { InvalidStateError, "Element has unexpected visibility state"_s }; - if (is(element) && element.hasAttributeWithoutSynchronization(HTMLNames::openAttr)) return Exception { InvalidStateError, "Element is an open element"_s }; @@ -1261,7 +1261,7 @@ static ExceptionOr checkPopoverValidity(HTMLElement& element, PopoverVisib return Exception { InvalidStateError, "Element is fullscreen"_s }; #endif - return { }; + return true; } // https://html.spec.whatwg.org/#topmost-popover-ancestor @@ -1375,8 +1375,11 @@ void HTMLElement::queuePopoverToggleEventTask(PopoverVisibilityState oldState, P ExceptionOr HTMLElement::showPopover() { - if (auto check = checkPopoverValidity(*this, PopoverVisibilityState::Hidden); check.hasException()) + auto check = checkPopoverValidity(*this, PopoverVisibilityState::Hidden); + if (check.hasException()) return check.releaseException(); + if (!check.returnValue()) + return { }; ASSERT(!isInTopLayer()); @@ -1386,8 +1389,11 @@ ExceptionOr HTMLElement::showPopover() if (event->defaultPrevented() || event->defaultHandled()) return { }; - if (auto check = checkPopoverValidity(*this, PopoverVisibilityState::Hidden, document.ptr()); check.hasException()) + check = checkPopoverValidity(*this, PopoverVisibilityState::Hidden, document.ptr()); + if (check.hasException()) return check.releaseException(); + if (!check.returnValue()) + return { }; ASSERT(popoverData()); @@ -1399,8 +1405,11 @@ ExceptionOr HTMLElement::showPopover() if (popoverState() != originalState) return Exception { InvalidStateError, "The value of the popover attribute was changed while hiding the popover."_s }; - if (auto check = checkPopoverValidity(*this, PopoverVisibilityState::Hidden, document.ptr()); check.hasException()) + check = checkPopoverValidity(*this, PopoverVisibilityState::Hidden, document.ptr()); + if (check.hasException()) return check.releaseException(); + if (!check.returnValue()) + return { }; } bool shouldRestoreFocus = !document->topmostAutoPopover(); @@ -1425,16 +1434,23 @@ ExceptionOr HTMLElement::showPopover() ExceptionOr HTMLElement::hidePopoverInternal(FocusPreviousElement focusPreviousElement, FireEvents fireEvents) { - if (auto check = checkPopoverValidity(*this, PopoverVisibilityState::Showing); check.hasException()) + auto check = checkPopoverValidity(*this, PopoverVisibilityState::Showing); + if (check.hasException()) return check.releaseException(); + if (!check.returnValue()) + return { }; + ASSERT(popoverData()); if (popoverState() == PopoverState::Auto) { document().hideAllPopoversUntil(this, focusPreviousElement, fireEvents); - if (auto check = checkPopoverValidity(*this, PopoverVisibilityState::Showing); check.hasException()) + check = checkPopoverValidity(*this, PopoverVisibilityState::Showing); + if (check.hasException()) return check.releaseException(); + if (!check.returnValue()) + return { }; } popoverData()->setInvoker(nullptr); @@ -1442,8 +1458,11 @@ ExceptionOr HTMLElement::hidePopoverInternal(FocusPreviousElement focusPre if (fireEvents == FireEvents::Yes) dispatchEvent(ToggleEvent::create(eventNames().beforetoggleEvent, { EventInit { }, "open"_s, "closed"_s }, Event::IsCancelable::No)); - if (auto check = checkPopoverValidity(*this, PopoverVisibilityState::Showing); check.hasException()) + check = checkPopoverValidity(*this, PopoverVisibilityState::Showing); + if (check.hasException()) return check.releaseException(); + if (!check.returnValue()) + return { }; ASSERT(popoverData());