commit fc08bd4c083de68f37b69b91e5420655cf5af954
parent 72bf441d8ee08eeac5e63b77a2dec65b236637df
Author: Emilio Cobos Álvarez <emilio@crisal.io>
Date: Mon, 27 Oct 2025 19:18:34 +0000
Bug 1996182 - Fix up focus within state on focus redirect as well. r=smaug
This is... Not particularly pretty, to be clear. Maybe I should file a
spec issue to get better define when and how the :focus-within state
gets updated, because on one hand it's weird that when in a tree like:
<div id=parent>
<input id=a>
<input id=b>
</div>
Moving focus from a to b would allow you to observe parent without
:focus-within. But on the other the current set-up is clearly
error-prone.
Maybe we should add more state to nsFocusManager (an stack with
currently :focus-within ancestors) and fix it up "more properly" when
the focus actually moves (which could be on the blur event of a), but I
think I'd like to at least have some draft spec text for how does that
look.
I filed https://github.com/whatwg/html/issues/11835 to discuss this a
bit more thoroughly.
Differential Revision: https://phabricator.services.mozilla.com/D270166
Diffstat:
2 files changed, 83 insertions(+), 21 deletions(-)
diff --git a/dom/base/nsFocusManager.cpp b/dom/base/nsFocusManager.cpp
@@ -911,6 +911,45 @@ void nsFocusManager::ContentAppended(nsIContent* aFirstNewContent,
FocusedElementMayHaveMoved(aFirstNewContent, aInfo.mOldParent);
}
+static void UpdateFocusWithinState(Element* aElement,
+ nsIContent* aCommonAncestor,
+ bool aGettingFocus) {
+ for (nsIContent* content = aElement; content && content != aCommonAncestor;
+ content = content->GetFlattenedTreeParent()) {
+ Element* element = Element::FromNode(content);
+ if (!element) {
+ continue;
+ }
+
+ if (aGettingFocus) {
+ if (element->State().HasState(ElementState::FOCUS_WITHIN)) {
+ break;
+ }
+ element->AddStates(ElementState::FOCUS_WITHIN);
+ } else {
+ element->RemoveStates(ElementState::FOCUS_WITHIN);
+ }
+ }
+}
+
+static void MaybeFixUpFocusWithinState(Element* aElementToFocus,
+ Element* aFocusedElement) {
+ if (!aElementToFocus || aElementToFocus == aFocusedElement ||
+ !aElementToFocus->IsInComposedDoc()) {
+ return;
+ }
+ // Focus was redirected, make sure the :focus-within state remains consistent.
+ auto* commonAncestor = [&]() -> nsIContent* {
+ if (!aFocusedElement ||
+ aElementToFocus->OwnerDoc() != aFocusedElement->OwnerDoc()) {
+ return nullptr;
+ }
+ return nsContentUtils::GetCommonFlattenedTreeAncestor(aFocusedElement,
+ aElementToFocus);
+ }();
+ UpdateFocusWithinState(aElementToFocus, commonAncestor, false);
+}
+
nsresult nsFocusManager::ContentRemoved(Document* aDocument,
nsIContent* aContent,
const ContentRemoveInfo& aInfo) {
@@ -927,7 +966,7 @@ nsresult nsFocusManager::ContentRemoved(Document* aDocument,
return NS_OK;
}
- const Element* focusWithinElement = [&]() -> Element* {
+ Element* focusWithinElement = [&]() -> Element* {
if (auto* el = Element::FromNode(aContent)) {
return el;
}
@@ -957,10 +996,7 @@ nsresult nsFocusManager::ContentRemoved(Document* aDocument,
if (hasFocusWithinInThisDocument) {
// If we're in-between a blur and an incoming focus, we might have stale
// :focus-within in our ancestor chain. Fix it up now.
- for (auto* el :
- focusWithinElement->InclusiveFlatTreeAncestorsOfType<Element>()) {
- el->RemoveStates(ElementState::FOCUS_WITHIN, true);
- }
+ UpdateFocusWithinState(focusWithinElement, nullptr, false);
}
return NS_OK;
}
@@ -1494,22 +1530,7 @@ void nsFocusManager::NotifyFocusStateChange(Element* aElement,
}
}
- for (nsIContent* content = aElement; content && content != commonAncestor;
- content = content->GetFlattenedTreeParent()) {
- Element* element = Element::FromNode(content);
- if (!element) {
- continue;
- }
-
- if (aGettingFocus) {
- if (element->State().HasState(ElementState::FOCUS_WITHIN)) {
- break;
- }
- element->AddStates(ElementState::FOCUS_WITHIN);
- } else {
- element->RemoveStates(ElementState::FOCUS_WITHIN);
- }
- }
+ UpdateFocusWithinState(aElement, commonAncestor, aGettingFocus);
}
// static
@@ -1915,6 +1936,7 @@ Maybe<uint64_t> nsFocusManager::SetFocusInner(Element* aNewContent,
: nullptr),
commonAncestor, focusMovesToDifferentBC, aAdjustWidget,
remainActive, actionId, elementToFocus)) {
+ MaybeFixUpFocusWithinState(elementToFocus, mFocusedElement);
return Some(actionId);
}
}
@@ -2866,6 +2888,9 @@ void nsFocusManager::Focus(
}
}
} else {
+ // We only need this on this branch, on the branch above
+ // NotifyFocusStateChange takes care of it.
+ MaybeFixUpFocusWithinState(elementToFocus, mFocusedElement);
if (!mFocusedElement && mFocusedWindow == aWindow) {
// When there is no focused element, IMEStateManager needs to adjust IME
// enabled state with the document.
diff --git a/testing/web-platform/tests/css/selectors/focus-within-focus-move.html b/testing/web-platform/tests/css/selectors/focus-within-focus-move.html
@@ -0,0 +1,37 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>Moving focus from its own focus() call doesn't leave stale :focus-within state</title>
+<link rel="help" href="https://drafts.csswg.org/selectors-4/#the-focus-within-pseudo">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1996182">
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="author" href="https://mozilla.com" title="Mozilla">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div id="wrapper">
+ <div id="outer">
+ <input id="tab">
+ <input id="input" onblur="outside.focus()">
+ </div>
+ <input id="outside">
+</div>
+<script>
+onload = function() {
+ test(function() {
+ let wrapper = document.getElementById("wrapper");
+ let outer = document.getElementById("outer");
+ let tab = document.getElementById("tab");
+ let input = document.getElementById("input");
+ let outside = document.getElementById("outside");
+
+ input.focus();
+ assert_equals(document.activeElement, input, "activeElement after focus");
+ assert_true(outer.matches(":focus-within"), "outer matches :focus-within");
+ assert_true(wrapper.matches(":focus-within"), "wrapper matches :focus-within");
+ // This ends up shifting to `outside` rather than `tab`.
+ tab.focus();
+ assert_equals(document.activeElement, outside, "activeElement after trying to focus sibling");
+ assert_true(wrapper.matches(":focus-within"), "wrapper still matches :focus-within");
+ assert_false(outer.matches(":focus-within"), "outer no longer matches :focus-within");
+ });
+}
+</script>
