Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[popover] Improve popover-focus-2.html
https://bugs.webkit.org/show_bug.cgi?id=255730

Reviewed by Ryosuke Niwa.

This implements the following focus behavior [see 1]:
1. Moves focus from an invoking element to its invoked popover, regardless of where in the DOM that popover lives.
2. Moves focus back to the next focusable element after the invoking element once focus leaves the invoked popover.
3. Skips over an open invoked popover otherwise.

[1] https://html.spec.whatwg.org/#focus-navigation-scope-owner

* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-2-expected.txt:
* Source/WebCore/page/FocusController.cpp:
(WebCore::isOpenPopoverWithInvoker):
(WebCore::openPopoverInvokerCast):
(WebCore::FocusNavigationScope::firstChildInScope const):
(WebCore::FocusNavigationScope::lastChildInScope const):
(WebCore::FocusNavigationScope::nextSiblingInScope const):
(WebCore::FocusNavigationScope::previousSiblingInScope const):
(WebCore::FocusNavigationScope::FocusNavigationScope):
(WebCore::FocusNavigationScope::owner const):
(WebCore::FocusNavigationScope::scopeOf):
(WebCore::FocusNavigationScope::scopeOwnedByPopoverInvoker):
(WebCore::FocusController::findFocusableElementAcrossFocusScope):

Canonical link: https://commits.webkit.org/263532@main
  • Loading branch information
rwlbuis committed Apr 29, 2023
1 parent 63a6557 commit d8c15ed
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 9 deletions.
Expand Up @@ -3,7 +3,7 @@ Invoker after
Show popover
Toggle popover Other focusable element

FAIL Popover focus navigation assert_equals: Focus should move from invoker into the open popover expected Element node <button id="inside_popover1" tabindex="0">Inside1</button> but got Element node <button id="button3" tabindex="0">Button3</button>
PASS Popover focus navigation
PASS Circular reference tab navigation
PASS Popover focus returns when popover is hidden by invoker
FAIL Popover focus only returns to invoker when focus is within the popover assert_equals: focus does not move because it was not inside the popover expected Element node <span tabindex="0">Other focusable element</span> but got Element node <button popovertarget="focus-return2-p" tabindex="0">Togg...
Expand Down
80 changes: 72 additions & 8 deletions Source/WebCore/page/FocusController.cpp
Expand Up @@ -70,6 +70,23 @@ namespace WebCore {

using namespace HTMLNames;

static bool isOpenPopoverWithInvoker(const Node* node)
{
RefPtr popover = dynamicDowncast<HTMLElement>(node);
return popover && popover->popoverData() && popover->popoverData()->visibilityState() == PopoverVisibilityState::Showing && popover->popoverData()->invoker();
}

static const HTMLFormControlElement* invokerForPopoverShowingState(const Node* node)
{
RefPtr invoker = dynamicDowncast<HTMLFormControlElement>(node);
if (!invoker)
return nullptr;
HTMLElement* popover = invoker->popoverTargetElement();
if (popover && popover->popoverData() && popover->popoverData()->visibilityState() == PopoverVisibilityState::Showing && popover->popoverData()->invoker() == invoker)
return invoker.get();
return nullptr;
}

static inline bool hasCustomFocusLogic(const Element& element)
{
return is<HTMLElement>(element) && downcast<HTMLElement>(element).hasCustomFocusLogic();
Expand All @@ -93,6 +110,7 @@ class FocusNavigationScope {
WEBCORE_EXPORT static FocusNavigationScope scopeOf(Node&);
static FocusNavigationScope scopeOwnedByScopeOwner(Element&);
static FocusNavigationScope scopeOwnedByIFrame(HTMLFrameOwnerElement&);
static FocusNavigationScope scopeOwnedByPopoverInvoker(const HTMLFormControlElement&);

Node* firstNodeInScope() const;
Node* lastNodeInScope() const;
Expand All @@ -112,6 +130,7 @@ class FocusNavigationScope {

explicit FocusNavigationScope(TreeScope&);
explicit FocusNavigationScope(HTMLSlotElement&, SlotKind);
explicit FocusNavigationScope(Element&);

RefPtr<ContainerNode> m_treeScopeRootNode;
RefPtr<HTMLSlotElement> m_slotElement;
Expand All @@ -123,14 +142,20 @@ Node* FocusNavigationScope::firstChildInScope(const Node& node) const
{
if (is<Element>(node) && isFocusScopeOwner(downcast<Element>(node)))
return nullptr;
return node.firstChild();
auto* first = node.firstChild();
while (isOpenPopoverWithInvoker(first))
first = first->nextSibling();
return first;
}

Node* FocusNavigationScope::lastChildInScope(const Node& node) const
{
if (is<Element>(node) && isFocusScopeOwner(downcast<Element>(node)))
return nullptr;
return node.lastChild();
auto* last = node.lastChild();
while (isOpenPopoverWithInvoker(last))
last = last->previousSibling();
return last;
}

Node* FocusNavigationScope::parentInScope(const Node& node) const
Expand Down Expand Up @@ -162,7 +187,12 @@ Node* FocusNavigationScope::nextSiblingInScope(const Node& node) const
}
return nullptr;
}
return node.nextSibling();
if (m_treeScopeRootNode == &node)
return nullptr;
auto* next = node.nextSibling();
while (isOpenPopoverWithInvoker(next))
next = next->nextSibling();
return next;
}

Node* FocusNavigationScope::previousSiblingInScope(const Node& node) const
Expand All @@ -174,7 +204,12 @@ Node* FocusNavigationScope::previousSiblingInScope(const Node& node) const
}
return nullptr;
}
return node.previousSibling();
if (m_treeScopeRootNode == &node)
return nullptr;
auto* previous = node.previousSibling();
while (isOpenPopoverWithInvoker(previous))
previous = previous->previousSibling();
return previous;
}

Node* FocusNavigationScope::firstNodeInScope() const
Expand Down Expand Up @@ -242,6 +277,11 @@ FocusNavigationScope::FocusNavigationScope(HTMLSlotElement& slotElement, SlotKin
{
}

FocusNavigationScope::FocusNavigationScope(Element& element)
: m_treeScopeRootNode(&element)
{
}

Element* FocusNavigationScope::owner() const
{
if (m_slotElement)
Expand All @@ -250,6 +290,10 @@ Element* FocusNavigationScope::owner() const
ASSERT(m_treeScopeRootNode);
if (is<ShadowRoot>(*m_treeScopeRootNode))
return downcast<ShadowRoot>(*m_treeScopeRootNode).host();
if (isOpenPopoverWithInvoker(m_treeScopeRootNode.get())) {
RELEASE_ASSERT(is<HTMLElement>(m_treeScopeRootNode.get()));
return downcast<HTMLElement>(*m_treeScopeRootNode).popoverData()->invoker();
}
if (auto* frame = m_treeScopeRootNode->document().frame())
return frame->ownerElement();
return nullptr;
Expand All @@ -268,6 +312,8 @@ FocusNavigationScope FocusNavigationScope::scopeOf(Node& startingNode)
}
if (is<ShadowRoot>(currentNode))
return FocusNavigationScope(downcast<ShadowRoot>(*currentNode));
if (isOpenPopoverWithInvoker(currentNode.get()))
return FocusNavigationScope(downcast<Element>(*currentNode));
parentNode = currentNode->parentNode();
// The scope of a fallback content of a HTMLSlotElement is the slot element
// but the scope of a HTMLSlotElement is its parent scope.
Expand Down Expand Up @@ -295,6 +341,14 @@ FocusNavigationScope FocusNavigationScope::scopeOwnedByIFrame(HTMLFrameOwnerElem
return FocusNavigationScope(*downcast<LocalFrame>(frame.contentFrame())->document());
}

FocusNavigationScope FocusNavigationScope::scopeOwnedByPopoverInvoker(const HTMLFormControlElement& invoker)
{
ASSERT(invokerForPopoverShowingState(&invoker));
HTMLElement* popover = invoker.popoverTargetElement();
ASSERT(isOpenPopoverWithInvoker(popover));
return FocusNavigationScope(*popover);
}

static inline void dispatchEventsOnWindowAndFocusedElement(Document* document, bool focused)
{
// If we have a focused node we should dispatch blur on it before we blur the window.
Expand Down Expand Up @@ -572,13 +626,23 @@ Element* FocusController::findFocusableElementAcrossFocusScope(FocusDirection di
{
ASSERT(!is<Element>(currentNode) || !isNonFocusableScopeOwner(downcast<Element>(*currentNode), event));

if (currentNode && direction == FocusDirection::Forward && is<Element>(currentNode) && isFocusableScopeOwner(downcast<Element>(*currentNode), event)) {
if (Element* candidateInInnerScope = findFocusableElementWithinScope(direction, FocusNavigationScope::scopeOwnedByScopeOwner(downcast<Element>(*currentNode)), 0, event))
return candidateInInnerScope;
if (currentNode && direction == FocusDirection::Forward && is<Element>(currentNode)) {
if (isFocusableScopeOwner(downcast<Element>(*currentNode), event)) {
if (Element* candidateInInnerScope = findFocusableElementWithinScope(direction, FocusNavigationScope::scopeOwnedByScopeOwner(downcast<Element>(*currentNode)), 0, event))
return candidateInInnerScope;
} else if (auto* invoker = invokerForPopoverShowingState(currentNode)) {
if (Element* candidateInInnerScope = findFocusableElementWithinScope(direction, FocusNavigationScope::scopeOwnedByPopoverInvoker(*invoker), nullptr, event))
return candidateInInnerScope;
}
}

if (Element* candidateInCurrentScope = findFocusableElementWithinScope(direction, scope, currentNode, event))
if (Element* candidateInCurrentScope = findFocusableElementWithinScope(direction, scope, currentNode, event)) {
if (direction == FocusDirection::Backward) {
while (auto* invoker = invokerForPopoverShowingState(candidateInCurrentScope))
candidateInCurrentScope = findFocusableElementWithinScope(direction, FocusNavigationScope::scopeOwnedByPopoverInvoker(*invoker), nullptr, event);
}
return candidateInCurrentScope;
}

// If there's no focusable node to advance to, move up the focus scopes until we find one.
Element* owner = scope.owner();
Expand Down

0 comments on commit d8c15ed

Please sign in to comment.