Skip to content

script: Fully implement DocumentOrShadowRoot#activeElement#43861

Merged
mrobinson merged 1 commit into
servo:mainfrom
mrobinson:active-element
Apr 2, 2026
Merged

script: Fully implement DocumentOrShadowRoot#activeElement#43861
mrobinson merged 1 commit into
servo:mainfrom
mrobinson:active-element

Conversation

@mrobinson

@mrobinson mrobinson commented Apr 2, 2026

Copy link
Copy Markdown
Member

DocumentOrShadowRoot#activeElement should return retargeted results.
What that means is that if the DOM anchor of the Document's focused
focusable area is within a shadow root, Document#activeElement should
return the shadow host. This change implements that behavior, properly
returning the activeElement from both Document and ShadowRoot.

Testing: This causes a decent number of WPT tests and subtests to start
passing. One subtest starts to fail, because it uses the autofocus attribute
which we do not yet support.

@mrobinson mrobinson requested a review from gterzian as a code owner April 2, 2026 09:48
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 2, 2026
Comment thread components/script/dom/document/documentorshadowroot.rs
Comment thread components/script/dom/node/node.rs Outdated

impl Node {
/// <https://dom.spec.whatwg.org/#concept-tree-root>
pub(crate) fn root(&self) -> DomRoot<Node> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am surprised we didn't have this before, but I don't remember what the method was. Also can't find it right now...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also an Element::root_element(), but this returns the "document element" for the light tree, which isn't really in the specification and isn't what we need here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll use Node::GetRootNode as suggested by @simonwuelker below.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aah yes that's the one! Maybe add a link to the spec to that method so that we can find it.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Apr 2, 2026
Comment thread components/script/dom/node/node.rs Outdated
Comment on lines +270 to +280
/// <https://dom.spec.whatwg.org/#concept-tree-root>
pub(crate) fn root(&self) -> DomRoot<Node> {
if self.is_in_a_document_tree() {
DomRoot::upcast(self.owner_document())
} else {
self.inclusive_ancestors(ShadowIncluding::No)
.filter_map(DomRoot::downcast)
.last()
.expect("We know inclusive_ancestors will return `self` which is an element")
}
}

@simonwuelker simonwuelker Apr 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Node::GetRootNode(Default::default()) does the same, and its a bit more optimized, so please use that instead.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Sounds good!

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 2, 2026
@mrobinson mrobinson enabled auto-merge April 2, 2026 10:30
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Apr 2, 2026
@mrobinson mrobinson added this pull request to the merge queue Apr 2, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 2, 2026
if let Some(candidate) = DomRoot::downcast::<Element>(candidate.clone()) {
return Some(candidate);
}
assert_eq!(&*candidate, document.upcast::<Node>());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert_eq!(&*candidate, document.upcast::<Node>());
assert!(candidate.is::<Document>());

@mrobinson mrobinson removed this pull request from the merge queue due to a manual request Apr 2, 2026
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 2, 2026
`DocumentOrShadowRoot#activeElement` should return retarged results.
What that means is that if the DOM anchor of the `Document`'s focused
focusable area is within a shadow root, `Document#activeElement` should
return the shadow host. This change implements that behavior, properly
returning the `activeElement` from both `Document` and `ShadowRoot`.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 2, 2026
@mrobinson mrobinson enabled auto-merge April 2, 2026 11:03
@mrobinson mrobinson added this pull request to the merge queue Apr 2, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 2, 2026
Merged via the queue into servo:main with commit 33f74fe Apr 2, 2026
33 checks passed
@mrobinson mrobinson deleted the active-element branch April 2, 2026 12:00
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants