script: Align focusable area logic to be closer to the specification by mrobinson · Pull Request #43058 · servo/servo

mrobinson · 2026-03-06T10:47:08Z

This abstracts out a common helper to determine focusable area kind
which is used by Element::is_click_focusable,
Element::is_sequentially_focusable, and Element::is_focusable_area.
This avoid having the latter depend on the former two, which was the
reverse of what the specification said. The helper is necessary because
the specification defines click and sequential focusability as subsets
of focusable elements, but going from "This is a focusable element" to
determining what kind of focusable element something is requires
duplicating a lot of the logic that was used to determine that something
is a focusable area.

It's likely that the specification needs more work here to improve the
definition (and indeed to unify all of the places that talk about what
elements are focusable areas). For now, this just tries to make our code
more similar to the text.

See: whatwg/html#4607
Testing: This shouldn't change behavior, so should be tested by existing tests.

mrobinson · 2026-03-06T10:47:27Z

cc @TimvdLippe PTAL

TimvdLippe

Only a couple of nits. Nice solution to use bitflags for this.

TimvdLippe · 2026-03-06T13:40:49Z

+    /// > An element is being rendered if it has any associated CSS layout boxes, SVG layout boxes,
+    /// > or some equivalent in other styling languages.


Nit: generally we only have the link to the spec in the rustdoc and then the explanation of the spec inside the method body.

Sure. I'm happy to move this.

TimvdLippe · 2026-03-06T13:41:37Z

+            // >    navigation scope owners, except the elements whose tabindex value is a negative integer.
+            // >  - It contains all of the focusable areas whose DOM anchor is an element in owner's focus
+            // >    navigation scope, except the focusable areas whose tabindex value is a negative integer.
+            Some(-1) => return FocusableAreaKind::Click,


Nit: should cover all negative integers

Suggested change

Some(-1) => return FocusableAreaKind::Click,

Some(index) if index < 0 => return FocusableAreaKind::Click,

TimvdLippe · 2026-03-06T13:42:55Z

        // > Modulo platform conventions, it is suggested that the following elements should be
        // > considered as focusable areas and be sequentially focusable:
-        match node.type_id() {
+ let is_focusable_are_due_to_type = match node.type_id() {


It took me a while to figure out what the typo was here 😛

Suggested change

let is_focusable_are_due_to_type = match node.type_id() {

let is_focusable_area_due_to_type = match node.type_id() {

mrobinson

Thanks for the review.

mrobinson · 2026-03-06T14:42:48Z

        // > Modulo platform conventions, it is suggested that the following elements should be
        // > considered as focusable areas and be sequentially focusable:
-        match node.type_id() {
+ let is_focusable_are_due_to_type = match node.type_id() {


mrobinson · 2026-03-06T14:42:54Z

+            // >    navigation scope owners, except the elements whose tabindex value is a negative integer.
+            // >  - It contains all of the focusable areas whose DOM anchor is an element in owner's focus
+            // >    navigation scope, except the focusable areas whose tabindex value is a negative integer.
+            Some(-1) => return FocusableAreaKind::Click,


mrobinson · 2026-03-06T14:43:03Z

+    /// > An element is being rendered if it has any associated CSS layout boxes, SVG layout boxes,
+    /// > or some equivalent in other styling languages.


Sure. I'm happy to move this.

This abstracts out a common helper to determine focusable area kind which is used by `Element::is_click_focusable`, `Element::is_sequentially_focusable`, and `Element::is_focusable_area`. This avoid having the latter depend on the former two, which was the reverse of what the specification said. The helper is necessary because the specification defines click and sequential focusability as subsets of focusable elements, but going from "This is a focusable element" to determining what kind of focusable element something is requires duplicating a lot of the logic that was used to determine that something is a focusable area. It's likely that the specification needs more work here to improve the definition (and indeed to unify all of the places that talk about what elements are focusable areas). For now, this just tries to make our code more similar to the text. Signed-off-by: Martin Robinson <mrobinson@igalia.com>

…ervo#43058) This abstracts out a common helper to determine focusable area kind which is used by `Element::is_click_focusable`, `Element::is_sequentially_focusable`, and `Element::is_focusable_area`. This avoid having the latter depend on the former two, which was the reverse of what the specification said. The helper is necessary because the specification defines click and sequential focusability as subsets of focusable elements, but going from "This is a focusable element" to determining what kind of focusable element something is requires duplicating a lot of the logic that was used to determine that something is a focusable area. It's likely that the specification needs more work here to improve the definition (and indeed to unify all of the places that talk about what elements are focusable areas). For now, this just tries to make our code more similar to the text. See: whatwg/html#4607 Testing: This shouldn't change behavior, so should be tested by existing tests. Signed-off-by: Martin Robinson <mrobinson@igalia.com> (cherry picked from commit e2f6852)

mrobinson requested a review from gterzian as a code owner March 6, 2026 10:47

servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 6, 2026

TimvdLippe approved these changes Mar 6, 2026

View reviewed changes

servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Mar 6, 2026

mrobinson commented Mar 6, 2026

View reviewed changes

mrobinson force-pushed the focus-area-cleanup branch from b520208 to 0f5fb0e Compare March 6, 2026 14:45

servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 6, 2026

mrobinson enabled auto-merge March 6, 2026 14:45

mrobinson added this pull request to the merge queue Mar 6, 2026

servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Mar 6, 2026

Merged via the queue into servo:main with commit e2f6852 Mar 6, 2026
33 checks passed

mrobinson deleted the focus-area-cleanup branch March 6, 2026 15:40

servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Mar 6, 2026

mrobinson mentioned this pull request Mar 14, 2026

Improve focusable area algorithm for element #19745

Closed

		/// > An element is being rendered if it has any associated CSS layout boxes, SVG layout boxes,
		/// > or some equivalent in other styling languages.

	Some(-1) => return FocusableAreaKind::Click,
	Some(index) if index < 0 => return FocusableAreaKind::Click,

	let is_focusable_are_due_to_type = match node.type_id() {
	let is_focusable_area_due_to_type = match node.type_id() {

Uh oh!

Conversation

mrobinson commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrobinson commented Mar 6, 2026

Uh oh!

TimvdLippe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrobinson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mrobinson commented Mar 6, 2026 •

edited

Loading