From 3dc8d2660f1d502b97c3e49a52cc8b1ca8699c53 Mon Sep 17 00:00:00 2001 From: Ziran Sun Date: Wed, 15 Nov 2023 18:51:36 +0000 Subject: [PATCH] Bug 1856539 - Add focus navigation support for popover. r=emilio,smaug,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 --- dom/base/nsFocusManager.cpp | 133 +++++++++++++++--- dom/base/nsFocusManager.h | 5 +- layout/base/nsFrameTraversal.cpp | 38 +++-- .../popovers/popover-focus-2.html.ini | 23 ++- .../semantics/popovers/popover-focus-2.html | 24 +++- 5 files changed, 193 insertions(+), 30 deletions(-) diff --git a/dom/base/nsFocusManager.cpp b/dom/base/nsFocusManager.cpp index d02f5e5c2c3e..0d9e4df42f7e 100644 --- a/dom/base/nsFocusManager.cpp +++ b/dom/base/nsFocusManager.cpp @@ -3354,7 +3354,7 @@ nsresult nsFocusManager::DetermineElementToMoveFocus( } return GetNextTabbableContent(presShell, startContent, nullptr, startContent, true, 1, false, false, - aNavigateByKey, aNextContent); + aNavigateByKey, false, aNextContent); } if (aType == MOVEFOCUS_LAST) { if (!aStartContent) { @@ -3362,7 +3362,7 @@ nsresult nsFocusManager::DetermineElementToMoveFocus( } return GetNextTabbableContent(presShell, startContent, nullptr, startContent, false, 0, false, false, - aNavigateByKey, aNextContent); + aNavigateByKey, false, aNextContent); } bool forward = (aType == MOVEFOCUS_FORWARD || aType == MOVEFOCUS_FORWARDDOC || @@ -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. @@ -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. @@ -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); @@ -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; } } @@ -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) { @@ -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 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 owner = FindScopeOwner(startContent)) { @@ -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) { @@ -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 invokerContent = + popover->GetPopoverData()->GetInvoker()->AsContent(); + if (aForward) { + nsIFrame* frame = invokerContent->GetPrimaryFrame(); + int32_t tabIndex = frame->IsFocusable().mTabIndex; + RefPtr 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 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. @@ -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; } @@ -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); @@ -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 @@ -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; @@ -4641,7 +4742,7 @@ nsresult nsFocusManager::FocusFirst(Element* aRootElement, if (RefPtr presShell = doc->GetPresShell()) { return GetNextTabbableContent(presShell, aRootElement, nullptr, aRootElement, true, 1, false, false, true, - aNextContent); + false, aNextContent); } } } diff --git a/dom/base/nsFocusManager.h b/dom/base/nsFocusManager.h index 9309bde0d6b7..7dceda6706c2 100644 --- a/dom/base/nsFocusManager.h +++ b/dom/base/nsFocusManager.h @@ -658,6 +658,9 @@ 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. */ @@ -665,7 +668,7 @@ class nsFocusManager final : public nsIFocusManager, 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); /** diff --git a/layout/base/nsFrameTraversal.cpp b/layout/base/nsFrameTraversal.cpp index 355ce94edc7f..1bfb98ae507c 100644 --- a/layout/base/nsFrameTraversal.cpp +++ b/layout/base/nsFrameTraversal.cpp @@ -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; @@ -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; } @@ -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; } @@ -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; } @@ -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; } @@ -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) { diff --git a/testing/web-platform/meta/html/semantics/popovers/popover-focus-2.html.ini b/testing/web-platform/meta/html/semantics/popovers/popover-focus-2.html.ini index 4d5744a6ab65..ada963570469 100644 --- a/testing/web-platform/meta/html/semantics/popovers/popover-focus-2.html.ini +++ b/testing/web-platform/meta/html/semantics/popovers/popover-focus-2.html.ini @@ -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] diff --git a/testing/web-platform/tests/html/semantics/popovers/popover-focus-2.html b/testing/web-platform/tests/html/semantics/popovers/popover-focus-2.html index b5ad7a586ee1..892e5fd68f77 100644 --- a/testing/web-platform/tests/html/semantics/popovers/popover-focus-2.html +++ b/testing/web-platform/tests/html/semantics/popovers/popover-focus-2.html @@ -12,12 +12,16 @@
+
+
+
+
@@ -55,17 +59,31 @@ promise_test(async t => { assert_equals(document.activeElement,button2,'Hidden popover should be skipped'); await sendShiftTab(); assert_equals(document.activeElement,button1,'Hidden popover should be skipped backwards'); + popover_no_invoker.showPopover(); await sendTab(); await sendTab(); - assert_equals(document.activeElement,invoker1); + assert_equals(document.activeElement,popover_no_invoker,"Focusable popover that is opened without an invoker should get focused"); + await sendTab(); + assert_equals(document.activeElement,invoker0); + await sendEnter(); // Activate the invoker0 + assert_true(popover0.matches(':popover-open'), 'popover0 should be invoked by invoker0'); + assert_equals(document.activeElement,invoker0,'Focus should not move when popover is shown'); + await sendTab(); await sendEnter(); // Activate the invoker assert_true(popover1.matches(':popover-open'), 'popover1 should be invoked by invoker1'); assert_equals(document.activeElement,invoker1,'Focus should not move when popover is shown'); await sendTab(); + // Make invoker1 non-focusable. + invoker1.disabled = true; assert_equals(document.activeElement,inside_popover1,'Focus should move from invoker into the open popover'); await sendTab(); assert_equals(document.activeElement,invoker2,'Focus should move within popover'); - await verifyFocusOrder([button1, button2, invoker1, inside_popover1, invoker2, inside_popover2, button3, button4],'set 1'); + await sendShiftTab(); + await sendShiftTab(); + assert_equals(document.activeElement, button1 ,'Focus should not move back to invoker as it is non-focusable'); + // Reset invoker1 to focusable. + invoker1.disabled = false; + await verifyFocusOrder([button1, button2, invoker0, invoker1, inside_popover1, invoker2, inside_popover2, button3, button4],'set 1'); invoker2.focus(); await sendEnter(); // Activate the nested invoker assert_true(popover2.matches(':popover-open'), 'popover2 should be invoked by nested invoker'); @@ -88,7 +106,7 @@ promise_test(async t => { await sendTab(); assert_equals(document.activeElement,button4,'Focus should skip popovers'); button1.focus(); - await verifyFocusOrder([button1, button2, invoker1, inside_popover1, invoker2, inside_popover3, invoker3, inside_popover2, button3, button4],'set 2'); + await verifyFocusOrder([button1, button2, invoker0, invoker1, inside_popover1, invoker2, inside_popover3, invoker3, inside_popover2, button3, button4],'set 2'); }, "Popover focus navigation");