From f4a34ff850554d9c85d9114b14a3a12bb01f1ccf Mon Sep 17 00:00:00 2001 From: Rob Buis Date: Fri, 31 Mar 2023 14:10:22 -0700 Subject: [PATCH] [popover] Implement "check and possibly close popover stack" algorithm https://bugs.webkit.org/show_bug.cgi?id=254382 Reviewed by Tim Nguyen. Implement "check and possibly close popover stack" algorithm as specified here: https://github.com/whatwg/html/pull/9048 This patch also imports the latest version of popover-target-element-disabled.html. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-target-element-disabled-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-target-element-disabled.html: * Source/WebCore/html/HTMLElement.cpp: (WebCore::HTMLElement::checkAndPossiblyClosePopoverStackInternal): * Source/WebCore/html/HTMLElement.h: (WebCore::HTMLElement::checkAndPossiblyClosePopoverStack): * Source/WebCore/html/HTMLFormControlElement.cpp: (WebCore::HTMLFormControlElement::removedFromAncestor): (WebCore::HTMLFormControlElement::parseAttribute): (WebCore::HTMLFormControlElement::disabledStateChanged): (WebCore::HTMLFormControlElement::didChangeForm): * Source/WebCore/html/HTMLFormControlElement.h: * Source/WebCore/html/HTMLInputElement.cpp: (WebCore::HTMLInputElement::updateType): Canonical link: https://commits.webkit.org/262440@main --- ...popover-target-element-disabled-expected.txt | 12 ++++++------ Source/WebCore/html/HTMLElement.cpp | 17 +++++++++++++++++ Source/WebCore/html/HTMLElement.h | 9 +++++++++ Source/WebCore/html/HTMLFormControlElement.cpp | 14 ++++++++++++-- Source/WebCore/html/HTMLFormControlElement.h | 2 ++ Source/WebCore/html/HTMLInputElement.cpp | 2 ++ 6 files changed, 48 insertions(+), 8 deletions(-) diff --git a/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-target-element-disabled-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-target-element-disabled-expected.txt index a1d9355a84086..a135633496929 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-target-element-disabled-expected.txt +++ b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-target-element-disabled-expected.txt @@ -2,11 +2,11 @@ form helloother button PASS Disabled popover*target buttons should not affect the popover heirarchy. -FAIL Disabling popover*target buttons when popovers are open should still cause all popovers to be closed when the formerly outer popover is closed. assert_false: The inner popover should be closed when the hierarchy is broken. expected false got true -FAIL Disabling popover*target buttons when popovers are open should still cause all popovers to be closed when the formerly inner popover is closed. assert_false: The inner popover be should be closed when the hierarchy is broken. expected false got true -FAIL Setting the form attribute on popover*target buttons when popovers are open should close all popovers. assert_false: The inner popover be should be closed when the hierarchy is broken. expected false got true -FAIL Changing the input type on a popover*target button when popovers are open should close all popovers. assert_false: The inner popover be should be closed when the hierarchy is broken. expected false got true -FAIL Disconnecting popover*target buttons when popovers are open should close all popovers. assert_false: The inner popover be should be closed when the hierarchy is broken. expected false got true -FAIL Changing the popovertarget attribute to break the chain should close all popovers. assert_false: The inner popover be should be closed when the hierarchy is broken. expected false got true +PASS Disabling popover*target buttons when popovers are open should still cause all popovers to be closed when the formerly outer popover is closed. +PASS Disabling popover*target buttons when popovers are open should still cause all popovers to be closed when the formerly inner popover is closed. +PASS Setting the form attribute on popover*target buttons when popovers are open should close all popovers. +PASS Changing the input type on a popover*target button when popovers are open should close all popovers. +PASS Disconnecting popover*target buttons when popovers are open should close all popovers. +PASS Changing the popovertarget attribute to break the chain should close all popovers. PASS Modifying popovertarget on a button which doesn't break the chain shouldn't close any popovers. diff --git a/Source/WebCore/html/HTMLElement.cpp b/Source/WebCore/html/HTMLElement.cpp index 05ad5cc11aec8..8e9747782b4b4 100644 --- a/Source/WebCore/html/HTMLElement.cpp +++ b/Source/WebCore/html/HTMLElement.cpp @@ -1284,6 +1284,23 @@ static HTMLElement* topmostPopoverAncestor(Element& newPopover) return topmostAncestor.get(); } +void HTMLElement::checkAndPossiblyClosePopoverStackInternal() +{ + Vector> autoPopoverList; + for (auto& element : document().topLayerElements()) { + if (!is(element) || downcast(element.get()).popoverState() != PopoverState::Auto) + continue; + autoPopoverList.append(element.ptr()); + } + + for (size_t i = autoPopoverList.size(); i-- > 1;) { + if (topmostPopoverAncestor(*autoPopoverList[i]) != autoPopoverList[i - 1]) { + document().hideAllPopoversUntil(nullptr, FocusPreviousElement::No, FireEvents::No); + return; + } + } +} + // https://html.spec.whatwg.org/#popover-focusing-steps static void runPopoverFocusingSteps(HTMLElement& popover) { diff --git a/Source/WebCore/html/HTMLElement.h b/Source/WebCore/html/HTMLElement.h index 6c24e9d69391d..5ebff3ebef944 100644 --- a/Source/WebCore/html/HTMLElement.h +++ b/Source/WebCore/html/HTMLElement.h @@ -23,6 +23,7 @@ #pragma once #include "ColorTypes.h" +#include "Document.h" #include "HTMLNames.h" #include "InputMode.h" #include "StyledElement.h" @@ -191,6 +192,13 @@ class HTMLElement : public StyledElement { void childrenChanged(const ChildChange&) override; void updateEffectiveDirectionalityOfDirAuto(); + void checkAndPossiblyClosePopoverStack() + { + if (!document().settings().popoverAttributeEnabled() || !document().hasTopLayerElement()) + return; + checkAndPossiblyClosePopoverStackInternal(); + } + using EventHandlerNameMap = HashMap; static const AtomString& eventNameForEventHandlerAttribute(const QualifiedName& attributeName, const EventHandlerNameMap&); @@ -214,6 +222,7 @@ class HTMLElement : public StyledElement { enum class UseCSSPXAsUnitType : bool { No, Yes }; enum class IsMultiLength : bool { No, Yes }; void addHTMLLengthToStyle(MutableStyleProperties&, CSSPropertyID, StringView value, AllowPercentage, UseCSSPXAsUnitType, IsMultiLength, AllowZeroValue = AllowZeroValue::Yes); + void checkAndPossiblyClosePopoverStackInternal(); }; inline HTMLElement::HTMLElement(const QualifiedName& tagName, Document& document, ConstructionType type = CreateHTMLElement) diff --git a/Source/WebCore/html/HTMLFormControlElement.cpp b/Source/WebCore/html/HTMLFormControlElement.cpp index df0b3c5d1e6e6..9fdf61b4faffb 100644 --- a/Source/WebCore/html/HTMLFormControlElement.cpp +++ b/Source/WebCore/html/HTMLFormControlElement.cpp @@ -141,6 +141,7 @@ void HTMLFormControlElement::removedFromAncestor(RemovalType removalType, Contai { HTMLElement::removedFromAncestor(removalType, oldParentOfRemovedTree); ValidatedFormListedElement::removedFromAncestor(removalType, oldParentOfRemovedTree); + checkAndPossiblyClosePopoverStack(); } void HTMLFormControlElement::parseAttribute(const QualifiedName& name, const AtomString& value) @@ -152,7 +153,9 @@ void HTMLFormControlElement::parseAttribute(const QualifiedName& name, const Ato m_isRequired = newRequired; requiredStateChanged(); } - } else { + } else if (name == popovertargetAttr) + checkAndPossiblyClosePopoverStack(); + else { HTMLElement::parseAttribute(name, value); ValidatedFormListedElement::parseAttribute(name, value); } @@ -170,6 +173,7 @@ void HTMLFormControlElement::disabledStateChanged() ValidatedFormListedElement::disabledStateChanged(); if (renderer() && renderer()->style().hasEffectiveAppearance()) renderer()->theme().stateChanged(*renderer(), ControlStates::States::Enabled); + checkAndPossiblyClosePopoverStack(); } void HTMLFormControlElement::readOnlyStateChanged() @@ -407,4 +411,10 @@ bool HTMLFormControlElement::needsMouseFocusableQuirk() const return document().quirks().needsFormControlToBeMouseFocusable(); } -} // namespace WebCore +void HTMLFormControlElement::didChangeForm() +{ + ValidatedFormListedElement::didChangeForm(); + checkAndPossiblyClosePopoverStack(); +} + +} // namespace Webcore diff --git a/Source/WebCore/html/HTMLFormControlElement.h b/Source/WebCore/html/HTMLFormControlElement.h index e15d0a571686e..507688cb0671b 100644 --- a/Source/WebCore/html/HTMLFormControlElement.h +++ b/Source/WebCore/html/HTMLFormControlElement.h @@ -127,6 +127,8 @@ class HTMLFormControlElement : public HTMLElement, public ValidatedFormListedEle void handlePopoverTargetAction() const; + void didChangeForm() override; + private: void refFormAssociatedElement() const final { ref(); } void derefFormAssociatedElement() const final { deref(); } diff --git a/Source/WebCore/html/HTMLInputElement.cpp b/Source/WebCore/html/HTMLInputElement.cpp index f592978ac19f6..7a688d964e643 100644 --- a/Source/WebCore/html/HTMLInputElement.cpp +++ b/Source/WebCore/html/HTMLInputElement.cpp @@ -586,6 +586,8 @@ void HTMLInputElement::updateType() } updateValidity(); + + checkAndPossiblyClosePopoverStack(); } inline void HTMLInputElement::runPostTypeUpdateTasks()