Skip to content

Commit

Permalink
Bug 1856539 - Add focus navigation support for popover. r=emilio,smau…
Browse files Browse the repository at this point in the history
…g,edgar

This implements the focus behavior described in [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

Differential Revision: https://phabricator.services.mozilla.com/D190560
  • Loading branch information
ziransun committed Nov 15, 2023
1 parent 7601cd7 commit 3dc8d26
Show file tree
Hide file tree
Showing 5 changed files with 193 additions and 30 deletions.
133 changes: 117 additions & 16 deletions dom/base/nsFocusManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3354,15 +3354,15 @@ nsresult nsFocusManager::DetermineElementToMoveFocus(
}
return GetNextTabbableContent(presShell, startContent, nullptr,
startContent, true, 1, false, false,
aNavigateByKey, aNextContent);
aNavigateByKey, false, aNextContent);
}
if (aType == MOVEFOCUS_LAST) {
if (!aStartContent) {
startContent = rootElement;
}
return GetNextTabbableContent(presShell, startContent, nullptr,
startContent, false, 0, false, false,
aNavigateByKey, aNextContent);
aNavigateByKey, false, aNextContent);
}

bool forward = (aType == MOVEFOCUS_FORWARD || aType == MOVEFOCUS_FORWARDDOC ||
Expand Down Expand Up @@ -3537,7 +3537,8 @@ nsresult nsFocusManager::DetermineElementToMoveFocus(
MOZ_KnownLive(skipOriginalContentCheck ? nullptr
: originalStartContent.get()),
startContent, forward, tabIndex, ignoreTabIndex,
forDocumentNavigation, aNavigateByKey, getter_AddRefs(nextFocus));
forDocumentNavigation, aNavigateByKey, false,
getter_AddRefs(nextFocus));
NS_ENSURE_SUCCESS(rv, rv);
if (rv == NS_SUCCESS_DOM_NO_OPERATION) {
// Navigation was redirected to a child process, so just return.
Expand Down Expand Up @@ -3813,6 +3814,29 @@ void ScopedContentTraversal::Prev() {
SetCurrent(parent == mOwner ? nullptr : parent);
}

static bool IsOpenPopoverWithInvoker(nsIContent* aContent) {
if (auto* popover = Element::FromNode(aContent)) {
return popover && popover->IsPopoverOpen() &&
popover->GetPopoverData()->GetInvoker();
}
return false;
}

static nsIContent* InvokerForPopoverShowingState(nsIContent* aContent) {
Element* invoker = Element::FromNode(aContent);
if (!invoker) {
return nullptr;
}

nsGenericHTMLElement* popover = invoker->GetEffectivePopoverTargetElement();
if (popover && popover->IsPopoverOpen() &&
popover->GetPopoverData()->GetInvoker() == invoker) {
return aContent;
}

return nullptr;
}

/**
* Returns scope owner of aContent.
* A scope owner is either a shadow host, or slot.
Expand Down Expand Up @@ -3866,7 +3890,9 @@ nsIContent* nsFocusManager::GetNextTabbableContentInScope(
nsIContent* aOriginalStartContent, bool aForward, int32_t aCurrentTabIndex,
bool aIgnoreTabIndex, bool aForDocumentNavigation, bool aNavigateByKey,
bool aSkipOwner) {
MOZ_ASSERT(IsHostOrSlot(aOwner), "Scope owner should be host or slot");
MOZ_ASSERT(
IsHostOrSlot(aOwner) || IsOpenPopoverWithInvoker(aOwner),
"Scope owner should be host, slot or an open popover with invoker set.");

// XXX: Why don't we ignore tabindex when the current tabindex < 0?
MOZ_ASSERT_IF(aCurrentTabIndex < 0, aIgnoreTabIndex);
Expand Down Expand Up @@ -4049,7 +4075,8 @@ static nsIContent* GetTopLevelScopeOwner(nsIContent* aContent) {
topLevelScopeOwner = aContent;
} else {
aContent = aContent->GetParent();
if (aContent && HTMLSlotElement::FromNode(aContent)) {
if (aContent && (HTMLSlotElement::FromNode(aContent) ||
IsOpenPopoverWithInvoker(aContent))) {
topLevelScopeOwner = aContent;
}
}
Expand All @@ -4062,7 +4089,7 @@ nsresult nsFocusManager::GetNextTabbableContent(
PresShell* aPresShell, nsIContent* aRootContent,
nsIContent* aOriginalStartContent, nsIContent* aStartContent, bool aForward,
int32_t aCurrentTabIndex, bool aIgnoreTabIndex, bool aForDocumentNavigation,
bool aNavigateByKey, nsIContent** aResultContent) {
bool aNavigateByKey, bool aSkipPopover, nsIContent** aResultContent) {
*aResultContent = nullptr;

if (!aStartContent) {
Expand All @@ -4080,15 +4107,33 @@ nsresult nsFocusManager::GetNextTabbableContent(
// search in scope owned by startContent
if (aForward && IsHostOrSlot(startContent)) {
nsIContent* contentToFocus = GetNextTabbableContentInScope(
startContent, startContent, aOriginalStartContent, aForward,
aForward ? 1 : 0, aIgnoreTabIndex, aForDocumentNavigation,
aNavigateByKey, true /* aSkipOwner */);
startContent, startContent, aOriginalStartContent, aForward, 1,
aIgnoreTabIndex, aForDocumentNavigation, aNavigateByKey,
true /* aSkipOwner */);
if (contentToFocus) {
NS_ADDREF(*aResultContent = contentToFocus);
return NS_OK;
}
}

// If startContent is a popover invoker, search the popover scope.
if (!aSkipPopover) {
if (InvokerForPopoverShowingState(startContent)) {
if (aForward) {
RefPtr<nsIContent> popover =
startContent->GetEffectivePopoverTargetElement();
nsIContent* contentToFocus = GetNextTabbableContentInScope(
popover, popover, aOriginalStartContent, aForward, 1,
aIgnoreTabIndex, aForDocumentNavigation, aNavigateByKey,
true /* aSkipOwner */);
if (contentToFocus) {
NS_ADDREF(*aResultContent = contentToFocus);
return NS_OK;
}
}
}
}

// If startContent is in a scope owned by Shadow DOM search from scope
// including startContent
if (nsCOMPtr<nsIContent> owner = FindScopeOwner(startContent)) {
Expand Down Expand Up @@ -4193,8 +4238,11 @@ nsresult nsFocusManager::GetNextTabbableContent(
oldTopLevelScopeOwner = currentTopLevelScopeOwner;
}
currentTopLevelScopeOwner = GetTopLevelScopeOwner(currentContent);

// We handle popover case separately.
if (currentTopLevelScopeOwner &&
currentTopLevelScopeOwner == oldTopLevelScopeOwner) {
currentTopLevelScopeOwner == oldTopLevelScopeOwner &&
!IsOpenPopoverWithInvoker(currentTopLevelScopeOwner)) {
// We're within non-document scope, continue.
do {
if (aForward) {
Expand All @@ -4209,6 +4257,55 @@ nsresult nsFocusManager::GetNextTabbableContent(
continue;
}

// Stepping out popover scope.
// For forward, search for the next tabbable content after invoker.
// For backward, we should get back to the invoker.
if (oldTopLevelScopeOwner &&
IsOpenPopoverWithInvoker(oldTopLevelScopeOwner) &&
currentTopLevelScopeOwner != oldTopLevelScopeOwner) {
if (auto* popover = Element::FromNode(oldTopLevelScopeOwner)) {
RefPtr<nsIContent> invokerContent =
popover->GetPopoverData()->GetInvoker()->AsContent();
if (aForward) {
nsIFrame* frame = invokerContent->GetPrimaryFrame();
int32_t tabIndex = frame->IsFocusable().mTabIndex;
RefPtr<nsIContent> rootElement = invokerContent;
if (auto* doc = invokerContent->GetComposedDoc()) {
rootElement = doc->GetRootElement();
}
if (tabIndex >= 0 &&
(aIgnoreTabIndex || aCurrentTabIndex == tabIndex)) {
nsresult rv = GetNextTabbableContent(
aPresShell, rootElement, nullptr, invokerContent, true,
tabIndex, false, false, aNavigateByKey, true, aResultContent);
if (NS_SUCCEEDED(rv) && *aResultContent) {
return rv;
}
}
} else {
if (invokerContent && invokerContent->IsFocusable()) {
invokerContent.forget(aResultContent);
return NS_OK;
}
}
}
}

if (!aForward) {
if (InvokerForPopoverShowingState(currentContent)) {
RefPtr<nsIContent> popover =
currentContent->GetEffectivePopoverTargetElement();
nsIContent* contentToFocus = GetNextTabbableContentInScope(
popover, popover, aOriginalStartContent, aForward, 0,
aIgnoreTabIndex, aForDocumentNavigation, aNavigateByKey,
true /* aSkipOwner */);

if (contentToFocus) {
NS_ADDREF(*aResultContent = contentToFocus);
return NS_OK;
}
}
}
// For document navigation, check if this element is an open panel. Since
// panels aren't focusable (tabIndex would be -1), we'll just assume that
// for document navigation, the tabIndex is 0.
Expand Down Expand Up @@ -4243,7 +4340,7 @@ nsresult nsFocusManager::GetNextTabbableContent(
// want to locate the first content, not the first document.
nsresult rv = GetNextTabbableContent(
aPresShell, currentContent, nullptr, currentContent, true, 1,
false, false, aNavigateByKey, aResultContent);
false, false, aNavigateByKey, false, aResultContent);
if (NS_SUCCEEDED(rv) && *aResultContent) {
return rv;
}
Expand All @@ -4258,7 +4355,8 @@ nsresult nsFocusManager::GetNextTabbableContent(
// append ELEMENT to NAVIGATION-ORDER."
// and later in "For each element ELEMENT in NAVIGATION-ORDER: "
// hosts and slots are handled before other elements.
if (currentTopLevelScopeOwner) {
if (currentTopLevelScopeOwner &&
!IsOpenPopoverWithInvoker(currentTopLevelScopeOwner)) {
bool focusableHostSlot;
int32_t tabIndex = HostOrSlotTabIndexValue(currentTopLevelScopeOwner,
&focusableHostSlot);
Expand Down Expand Up @@ -4291,8 +4389,11 @@ nsresult nsFocusManager::GetNextTabbableContent(
continue;
}

MOZ_ASSERT(!GetTopLevelScopeOwner(currentContent),
"currentContent should be in top-level-scope at this point");
MOZ_ASSERT(
!GetTopLevelScopeOwner(currentContent) ||
IsOpenPopoverWithInvoker(GetTopLevelScopeOwner(currentContent)),
"currentContent should be in top-level-scope at this point unless "
"for popover case");

// TabIndex not set defaults to 0 for form elements, anchors and other
// elements that are normally focusable. Tabindex defaults to -1
Expand Down Expand Up @@ -4491,7 +4592,7 @@ bool nsFocusManager::TryToMoveFocusToSubDocument(
nsresult rv = GetNextTabbableContent(
subPresShell, rootElement, aOriginalStartContent, rootElement,
aForward, (aForward ? 1 : 0), false, aForDocumentNavigation,
aNavigateByKey, aResultContent);
aNavigateByKey, false, aResultContent);
NS_ENSURE_SUCCESS(rv, false);
if (*aResultContent) {
return true;
Expand Down Expand Up @@ -4641,7 +4742,7 @@ nsresult nsFocusManager::FocusFirst(Element* aRootElement,
if (RefPtr<PresShell> presShell = doc->GetPresShell()) {
return GetNextTabbableContent(presShell, aRootElement, nullptr,
aRootElement, true, 1, false, false, true,
aNextContent);
false, aNextContent);
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion dom/base/nsFocusManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -658,14 +658,17 @@ class nsFocusManager final : public nsIFocusManager,
* from where the selection is. Similarly, if the starting element isn't
* focusable, since it doesn't really have a defined tab index.
*
* aSkipPopover should be true to avoid an invoker triggering to step into
* the popover that was already been visited again.
*
* aNavigateByKey to move focus by keyboard as a side effect of computing the
* next target.
*/
MOZ_CAN_RUN_SCRIPT nsresult GetNextTabbableContent(
mozilla::PresShell* aPresShell, nsIContent* aRootContent,
nsIContent* aOriginalStartContent, nsIContent* aStartContent,
bool aForward, int32_t aCurrentTabIndex, bool aIgnoreTabIndex,
bool aForDocumentNavigation, bool aNavigateByKey,
bool aForDocumentNavigation, bool aNavigateByKey, bool aSkipPopover,
nsIContent** aResultContent);

/**
Expand Down
38 changes: 30 additions & 8 deletions layout/base/nsFrameTraversal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ class nsFrameIterator : public nsIFrameEnumerator {
nsIFrame* GetPlaceholderFrame(nsIFrame* aFrame);
bool IsPopupFrame(nsIFrame* aFrame);

bool IsInvokerOpenPopoverFrame(nsIFrame* aFrame);

nsPresContext* const mPresContext;
const bool mLockScroll;
const bool mFollowOOFs;
Expand Down Expand Up @@ -353,8 +355,11 @@ nsIFrame* nsFrameIterator::GetFirstChild(nsIFrame* aFrame) {
if (result && mFollowOOFs) {
result = nsPlaceholderFrame::GetRealFrameFor(result);

if (IsPopupFrame(result)) result = GetNextSibling(result);
if (IsPopupFrame(result) || IsInvokerOpenPopoverFrame(result)) {
result = GetNextSibling(result);
}
}

return result;
}

Expand All @@ -364,8 +369,11 @@ nsIFrame* nsFrameIterator::GetLastChild(nsIFrame* aFrame) {
if (result && mFollowOOFs) {
result = nsPlaceholderFrame::GetRealFrameFor(result);

if (IsPopupFrame(result)) result = GetPrevSibling(result);
if (IsPopupFrame(result) || IsInvokerOpenPopoverFrame(result)) {
result = GetPrevSibling(result);
}
}

return result;
}

Expand All @@ -375,12 +383,14 @@ nsIFrame* nsFrameIterator::GetNextSibling(nsIFrame* aFrame) {
if (aFrame == mLimiter) return nullptr;
if (aFrame) {
result = GetNextSiblingInner(aFrame);
if (result && mFollowOOFs)
if (result && mFollowOOFs) {
result = nsPlaceholderFrame::GetRealFrameFor(result);
if (IsPopupFrame(result) || IsInvokerOpenPopoverFrame(result)) {
result = GetNextSibling(result);
}
}
}

if (mFollowOOFs && IsPopupFrame(result)) result = GetNextSibling(result);

return result;
}

Expand All @@ -390,12 +400,14 @@ nsIFrame* nsFrameIterator::GetPrevSibling(nsIFrame* aFrame) {
if (aFrame == mLimiter) return nullptr;
if (aFrame) {
result = GetPrevSiblingInner(aFrame);
if (result && mFollowOOFs)
if (result && mFollowOOFs) {
result = nsPlaceholderFrame::GetRealFrameFor(result);
if (IsPopupFrame(result) || IsInvokerOpenPopoverFrame(result)) {
result = GetPrevSibling(result);
}
}
}

if (mFollowOOFs && IsPopupFrame(result)) result = GetPrevSibling(result);

return result;
}

Expand Down Expand Up @@ -431,6 +443,16 @@ bool nsFrameIterator::IsPopupFrame(nsIFrame* aFrame) {
return aFrame && aFrame->IsMenuPopupFrame();
}

bool nsFrameIterator::IsInvokerOpenPopoverFrame(nsIFrame* aFrame) {
if (const nsIContent* currentContent = aFrame->GetContent()) {
if (const auto* popover = mozilla::dom::Element::FromNode(currentContent)) {
return popover && popover->IsPopoverOpen() &&
popover->GetPopoverData()->GetInvoker();
}
}
return false;
}

// nsVisualIterator implementation

nsIFrame* nsVisualIterator::GetFirstChildInner(nsIFrame* aFrame) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,23 @@
[popover-focus-2.html]
max-asserts: 2
expected:
if (os == "win") and (processor == "x86_64") and not debug: [OK, TIMEOUT]
if (os == "mac") and (processor == "x86_64") and not debug: [OK, TIMEOUT]
[Popover focus navigation]
expected: FAIL
expected:
if (os == "win") and (processor == "x86_64") and not debug: [PASS, TIMEOUT]
if (os == "mac") and (processor == "x86_64") and not debug: [PASS, TIMEOUT]

[Circular reference tab navigation]
expected:
if (os == "win") and (processor == "x86_64") and not debug: [PASS, NOTRUN]
if (os == "mac") and (processor == "x86_64") and not debug: [PASS, NOTRUN]

[Popover focus returns when popover is hidden by invoker]
expected:
if (os == "win") and (processor == "x86_64") and not debug: [PASS, NOTRUN]
if (os == "mac") and (processor == "x86_64") and not debug: [PASS, NOTRUN]

[Popover focus only returns to invoker when focus is within the popover]
expected:
if (os == "win") and (processor == "x86_64") and not debug: [PASS, NOTRUN]
if (os == "mac") and (processor == "x86_64") and not debug: [PASS, NOTRUN]
Loading

0 comments on commit 3dc8d26

Please sign in to comment.