Skip to content

Commit

Permalink
[popover] Implement "check and possibly close popover stack" algorithm
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=254382

Reviewed by Tim Nguyen.

Implement "check and possibly close popover stack" algorithm as specified here:
whatwg/html#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
  • Loading branch information
rwlbuis committed Mar 31, 2023
1 parent 6555fc3 commit f4a34ff
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.

17 changes: 17 additions & 0 deletions Source/WebCore/html/HTMLElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1284,6 +1284,23 @@ static HTMLElement* topmostPopoverAncestor(Element& newPopover)
return topmostAncestor.get();
}

void HTMLElement::checkAndPossiblyClosePopoverStackInternal()
{
Vector<RefPtr<Element>> autoPopoverList;
for (auto& element : document().topLayerElements()) {
if (!is<HTMLElement>(element) || downcast<HTMLElement>(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)
{
Expand Down
9 changes: 9 additions & 0 deletions Source/WebCore/html/HTMLElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#pragma once

#include "ColorTypes.h"
#include "Document.h"
#include "HTMLNames.h"
#include "InputMode.h"
#include "StyledElement.h"
Expand Down Expand Up @@ -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<AtomStringImpl*, AtomString>;
static const AtomString& eventNameForEventHandlerAttribute(const QualifiedName& attributeName, const EventHandlerNameMap&);

Expand All @@ -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)
Expand Down
14 changes: 12 additions & 2 deletions Source/WebCore/html/HTMLFormControlElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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);
}
Expand All @@ -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()
Expand Down Expand Up @@ -407,4 +411,10 @@ bool HTMLFormControlElement::needsMouseFocusableQuirk() const
return document().quirks().needsFormControlToBeMouseFocusable();
}

} // namespace WebCore
void HTMLFormControlElement::didChangeForm()
{
ValidatedFormListedElement::didChangeForm();
checkAndPossiblyClosePopoverStack();
}

} // namespace Webcore
2 changes: 2 additions & 0 deletions Source/WebCore/html/HTMLFormControlElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/html/HTMLInputElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,8 @@ void HTMLInputElement::updateType()
}

updateValidity();

checkAndPossiblyClosePopoverStack();
}

inline void HTMLInputElement::runPostTypeUpdateTasks()
Expand Down

0 comments on commit f4a34ff

Please sign in to comment.