From 2f265d58191c7a3495f046ba9c1bff9c4bfd75d6 Mon Sep 17 00:00:00 2001 From: Ziran Sun Date: Wed, 31 May 2023 08:59:10 +0000 Subject: [PATCH] Bug 1833570 - Change popover invoker type from boolean to Element. r=emilio The invoker type is currently implemented as boolean as suggested at https://github.com/whatwg/html/issues/9168. This issue is now closed and has been fixed at https://github.com/whatwg/html/pull/9171. This patch is to be follow above discussions and implement popover invoker type as element. Differential Revision: https://phabricator.services.mozilla.com/D178287 --- dom/base/Document.cpp | 4 +++- dom/base/Element.cpp | 16 ++-------------- dom/base/Element.h | 4 ---- dom/base/PopoverData.h | 25 +++++++++++++++++-------- dom/html/nsGenericHTMLElement.cpp | 2 +- 5 files changed, 23 insertions(+), 28 deletions(-) diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp index 57d8807c7c748..1073d843ba7ca 100644 --- a/dom/base/Document.cpp +++ b/dom/base/Document.cpp @@ -15074,7 +15074,9 @@ void Document::HidePopover(Element& aPopover, bool aFocusPreviousElement, } } - aPopover.SetHasPopoverInvoker(false); + auto* data = popoverHTMLEl->GetPopoverData(); + MOZ_ASSERT(data, "Should have popover data"); + data->SetInvoker(nullptr); // Fire beforetoggle event and re-check popover validity. if (aFireEvents && !wasHiding) { diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index 4a8b86149fe88..fde004dcf75fe 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -4248,19 +4248,6 @@ void Element::ClearServoData(Document* aDoc) { } } -bool Element::HasPopoverInvoker() const { - auto* popoverData = GetPopoverData(); - return popoverData && popoverData->HasPopoverInvoker(); -} - -void Element::SetHasPopoverInvoker(bool aHasInvoker) { - if (aHasInvoker) { - EnsurePopoverData().SetHasPopoverInvoker(true); - } else if (auto* popoverData = GetPopoverData()) { - popoverData->SetHasPopoverInvoker(false); - } -} - bool Element::IsAutoPopover() const { const auto* htmlElement = nsGenericHTMLElement::FromNode(this); return htmlElement && @@ -4305,8 +4292,9 @@ Element* Element::GetTopmostPopoverAncestor() const { checkAncestor(newPopover->GetFlattenedTreeParentElement()); - // TODO: To handle the button invokers // https://github.com/whatwg/html/issues/9160 + RefPtr invoker = newPopover->GetPopoverData()->GetInvoker(); + checkAncestor(invoker); return topmostPopoverAncestor; } diff --git a/dom/base/Element.h b/dom/base/Element.h index a618eadb210f7..509d789d43320 100644 --- a/dom/base/Element.h +++ b/dom/base/Element.h @@ -581,10 +581,6 @@ class Element : public FragmentOrElement { return CreatePopoverData(); } - // https://html.spec.whatwg.org/multipage/popover.html#popover-invoker - bool HasPopoverInvoker() const; - void SetHasPopoverInvoker(bool); - bool IsAutoPopover() const; bool IsPopoverOpen() const; diff --git a/dom/base/PopoverData.h b/dom/base/PopoverData.h index 6a703d6ed0590..e0afe7056061c 100644 --- a/dom/base/PopoverData.h +++ b/dom/base/PopoverData.h @@ -7,10 +7,12 @@ #ifndef mozilla_dom_PopoverData_h #define mozilla_dom_PopoverData_h -#include "nsStringFwd.h" +#include "Element.h" +#include "nsINode.h" #include "nsIRunnable.h" -#include "nsThreadUtils.h" #include "nsIWeakReferenceUtils.h" +#include "nsStringFwd.h" +#include "nsThreadUtils.h" namespace mozilla::dom { @@ -66,10 +68,14 @@ class PopoverData { mPreviouslyFocusedElement = aPreviouslyFocusedElement; } - bool HasPopoverInvoker() const { return mHasPopoverInvoker; } - void SetHasPopoverInvoker(bool aHasPopoverInvoker) { - mHasPopoverInvoker = aHasPopoverInvoker; + RefPtr GetInvoker() const { + return do_QueryReferent(mInvokerElement); + } + void SetInvoker(Element* aInvokerElement) { + mInvokerElement = + do_GetWeakReference(static_cast(aInvokerElement)); } + PopoverToggleEventTask* GetToggleEventTask() const { return mTask; } void SetToggleEventTask(PopoverToggleEventTask* aTask) { mTask = aTask; } void ClearToggleEventTask() { mTask = nullptr; } @@ -85,9 +91,12 @@ class PopoverData { // See, https://github.com/whatwg/html/issues/9063 nsWeakPtr mPreviouslyFocusedElement = nullptr; - // https://html.spec.whatwg.org/multipage/popover.html#popover-invoker, also - // see https://github.com/whatwg/html/issues/9168. - bool mHasPopoverInvoker = false; + // https://html.spec.whatwg.org/#popover-invoker + // Since having a popover invoker only makes a difference if the invoker + // is in the document (in another open popover to be precise) we can make + // this a weak reference, as if the element goes away it's necessarily not + // connected to our document. + nsWeakPtr mInvokerElement; bool mIsHiding = false; RefPtr mTask; }; diff --git a/dom/html/nsGenericHTMLElement.cpp b/dom/html/nsGenericHTMLElement.cpp index 8c7958736d305..4d521ce1ca446 100644 --- a/dom/html/nsGenericHTMLElement.cpp +++ b/dom/html/nsGenericHTMLElement.cpp @@ -2892,7 +2892,7 @@ void nsGenericHTMLFormControlElementWithState::HandlePopoverTargetAction() { if (canHide && target->IsPopoverOpen()) { target->HidePopover(IgnoreErrors()); } else if (canShow && !target->IsPopoverOpen()) { - target->SetHasPopoverInvoker(true); + target->GetPopoverData()->SetInvoker(this); target->ShowPopover(IgnoreErrors()); } }