From 9e89159594be66bdce4ab706ac1255157d1098a7 Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Thu, 8 Feb 2024 15:18:19 +0000 Subject: [PATCH] Bug 1878708 - Dialogs HideAllPopoversUntil nearest popover, not document. r=smaug Given some markup like: ```html
I'm a dialog!
``` With some JS like ```js openDialog.onclick = () => { dialog.showModal(); } openPopover.onclick = () => { popover.showPopover(); } ``` Clicking the "Open Popover" button followed by the "Open Dialog" button results in both the Dialog and Popover being hidden, however the dialog will still be open as modal, rendering the whole page inert, nothing is clickable and there seems to be no way to undo this (unless you use a CloseWatcher gesture such as the Esc key - if you have one available on your device). It is expected that the popover should not close the dialog, as it causes the invisible Modal Dialog to make the whole page inert, and it is very difficult for users (and developers) to discover how to undo this action (pressing escape). This was reported in https://github.com/whatwg/html/issues/9998, and WhatWG resolved to fix this, which in https://github.com/whatwg/html/pull/10116. Differential Revision: https://phabricator.services.mozilla.com/D200686 UltraBlame original commit: 52284e30cea2c52ab073310e4010161702dc92e4 --- dom/base/Element.cpp | 8 ++++++-- dom/base/Element.h | 3 ++- dom/html/HTMLDialogElement.cpp | 14 ++++++++++++-- dom/html/HTMLDialogElement.h | 4 ++-- dom/html/nsGenericHTMLElement.cpp | 2 +- ...er-top-layer-nesting-anchor.tentative.html.ini | 9 --------- ...ver-top-layer-nesting-hints.tentative.html.ini | 3 --- .../popover-top-layer-nesting.tentative.html.ini | 15 --------------- 8 files changed, 23 insertions(+), 35 deletions(-) diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index 79870b580a2ef..8eecb372cdd28 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -4330,7 +4330,8 @@ bool Element::IsPopoverOpen() const { return htmlElement && htmlElement->PopoverOpen(); } -Element* Element::GetTopmostPopoverAncestor(const Element* aInvoker) const { +Element* Element::GetTopmostPopoverAncestor(const Element* aInvoker, + bool isPopover) const { const Element* newPopover = this; nsTHashMap, size_t> popoverPositions; @@ -4338,7 +4339,10 @@ Element* Element::GetTopmostPopoverAncestor(const Element* aInvoker) const { for (Element* popover : OwnerDoc()->AutoPopoverList()) { popoverPositions.LookupOrInsert(popover, index++); } - popoverPositions.LookupOrInsert(newPopover, index); + + if (isPopover) { + popoverPositions.LookupOrInsert(newPopover, index); + } Element* topmostPopoverAncestor = nullptr; diff --git a/dom/base/Element.h b/dom/base/Element.h index 5ac2efa9dab8b..cbe4955afe587 100644 --- a/dom/base/Element.h +++ b/dom/base/Element.h @@ -583,7 +583,8 @@ class Element : public FragmentOrElement { - Element* GetTopmostPopoverAncestor(const Element* aInvoker) const; + Element* GetTopmostPopoverAncestor(const Element* aInvoker, + bool isPopover) const; ElementAnimationData* GetAnimationData() const { if (!MayHaveAnimations()) { diff --git a/dom/html/HTMLDialogElement.cpp b/dom/html/HTMLDialogElement.cpp index aee12db792017..b69d6c9e695a8 100644 --- a/dom/html/HTMLDialogElement.cpp +++ b/dom/html/HTMLDialogElement.cpp @@ -63,7 +63,12 @@ void HTMLDialogElement::Show(ErrorResult& aError) { StorePreviouslyFocusedElement(); - OwnerDoc()->HideAllPopoversWithoutRunningScript(); + RefPtr hideUntil = GetTopmostPopoverAncestor(nullptr, false); + if (!hideUntil) { + hideUntil = OwnerDoc(); + } + + OwnerDoc()->HideAllPopoversUntil(*hideUntil, false, true); FocusDialog(); } @@ -130,7 +135,12 @@ void HTMLDialogElement::ShowModal(ErrorResult& aError) { StorePreviouslyFocusedElement(); - OwnerDoc()->HideAllPopoversWithoutRunningScript(); + RefPtr hideUntil = GetTopmostPopoverAncestor(nullptr, false); + if (!hideUntil) { + hideUntil = OwnerDoc(); + } + + OwnerDoc()->HideAllPopoversUntil(*hideUntil, false, true); FocusDialog(); aError.SuppressException(); diff --git a/dom/html/HTMLDialogElement.h b/dom/html/HTMLDialogElement.h index 9ad8563105dd8..88c46ac60ed3e 100644 --- a/dom/html/HTMLDialogElement.h +++ b/dom/html/HTMLDialogElement.h @@ -38,8 +38,8 @@ class HTMLDialogElement final : public nsGenericHTMLElement { void UnbindFromTree(bool aNullParent = true) override; void Close(const mozilla::dom::Optional& aReturnValue); - void Show(ErrorResult& aError); - void ShowModal(ErrorResult& aError); + MOZ_CAN_RUN_SCRIPT void Show(ErrorResult& aError); + MOZ_CAN_RUN_SCRIPT void ShowModal(ErrorResult& aError); bool IsInTopLayer() const; void QueueCancelDialog(); diff --git a/dom/html/nsGenericHTMLElement.cpp b/dom/html/nsGenericHTMLElement.cpp index 4838dcf7c0341..3d8404dcc2e5e 100644 --- a/dom/html/nsGenericHTMLElement.cpp +++ b/dom/html/nsGenericHTMLElement.cpp @@ -3418,7 +3418,7 @@ void nsGenericHTMLElement::ShowPopoverInternal(Element* aInvoker, nsWeakPtr originallyFocusedElement; if (IsAutoPopover()) { auto originalState = GetPopoverAttributeState(); - RefPtr ancestor = GetTopmostPopoverAncestor(aInvoker); + RefPtr ancestor = GetTopmostPopoverAncestor(aInvoker, true); if (!ancestor) { ancestor = document; } diff --git a/testing/web-platform/meta/html/semantics/popovers/popover-top-layer-nesting-anchor.tentative.html.ini b/testing/web-platform/meta/html/semantics/popovers/popover-top-layer-nesting-anchor.tentative.html.ini index 4b2e819a52ac5..0d5f002b1616d 100644 --- a/testing/web-platform/meta/html/semantics/popovers/popover-top-layer-nesting-anchor.tentative.html.ini +++ b/testing/web-platform/meta/html/semantics/popovers/popover-top-layer-nesting-anchor.tentative.html.ini @@ -1,9 +1,6 @@ [popover-top-layer-nesting-anchor.tentative.html] expected: if (os == "mac") and not debug: TIMEOUT - [Single popover=auto ancestor with dialog] - expected: FAIL - [Single popover=auto ancestor with dialog, anchor attribute] expected: FAIL @@ -13,9 +10,6 @@ [Single popover=auto ancestor with fullscreen, anchor attribute] expected: FAIL - [Single popover=manual ancestor with dialog] - expected: FAIL - [Single popover=manual ancestor with dialog, anchor attribute] expected: FAIL @@ -32,7 +26,6 @@ [Nested popover=auto ancestors with dialog] expected: if (os == "mac") and not debug: NOTRUN - FAIL [Nested popover=auto ancestors with dialog, anchor attribute] expected: @@ -52,7 +45,6 @@ [Nested popover=auto ancestors, target is outer with dialog] expected: if (os == "mac") and not debug: NOTRUN - FAIL [Nested popover=auto ancestors, target is outer with dialog, anchor attribute] expected: @@ -72,7 +64,6 @@ [Top layer inside of nested element with dialog] expected: if (os == "mac") and not debug: NOTRUN - FAIL [Top layer inside of nested element with dialog, anchor attribute] expected: diff --git a/testing/web-platform/meta/html/semantics/popovers/popover-top-layer-nesting-hints.tentative.html.ini b/testing/web-platform/meta/html/semantics/popovers/popover-top-layer-nesting-hints.tentative.html.ini index d0c1edf9e9bb1..95a888b6ee2ba 100644 --- a/testing/web-platform/meta/html/semantics/popovers/popover-top-layer-nesting-hints.tentative.html.ini +++ b/testing/web-platform/meta/html/semantics/popovers/popover-top-layer-nesting-hints.tentative.html.ini @@ -1,7 +1,4 @@ [popover-top-layer-nesting-hints.tentative.html] - [Nested auto/hint ancestors with dialog] - expected: FAIL - [Nested auto/hint ancestors with fullscreen] expected: FAIL diff --git a/testing/web-platform/meta/html/semantics/popovers/popover-top-layer-nesting.tentative.html.ini b/testing/web-platform/meta/html/semantics/popovers/popover-top-layer-nesting.tentative.html.ini index 21aab78b7aa2c..b2f0cf2562cde 100644 --- a/testing/web-platform/meta/html/semantics/popovers/popover-top-layer-nesting.tentative.html.ini +++ b/testing/web-platform/meta/html/semantics/popovers/popover-top-layer-nesting.tentative.html.ini @@ -1,30 +1,15 @@ [popover-top-layer-nesting.tentative.html] - [Single popover=auto ancestor with dialog] - expected: FAIL - [Single popover=auto ancestor with fullscreen] expected: FAIL - [Single popover=manual ancestor with dialog] - expected: FAIL - [Single popover=manual ancestor with fullscreen] expected: FAIL - [Nested popover=auto ancestors with dialog] - expected: FAIL - [Nested popover=auto ancestors with fullscreen] expected: FAIL - [Nested popover=auto ancestors, target is outer with dialog] - expected: FAIL - [Nested popover=auto ancestors, target is outer with fullscreen] expected: FAIL - [Top layer inside of nested element with dialog] - expected: FAIL - [Top layer inside of nested element with fullscreen] expected: FAIL