Skip to content

script: Implement autofocus delegate#45826

Open
skyz1 wants to merge 4 commits into
servo:mainfrom
skyz1:autofocus-delegate
Open

script: Implement autofocus delegate#45826
skyz1 wants to merge 4 commits into
servo:mainfrom
skyz1:autofocus-delegate

Conversation

@skyz1

@skyz1 skyz1 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

This implements the autofocus delegate in order to focus elements with the autofocus attribute when opening dialogs.

Testing: Passes additional web platform tests.

@skyz1 skyz1 requested a review from gterzian as a code owner June 19, 2026 15:40
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jun 19, 2026
Signed-off-by: Glenn Skrzypczak <glenn.skrzypczak@gmail.com>
@skyz1 skyz1 force-pushed the autofocus-delegate branch from 7fc3827 to cae6316 Compare June 19, 2026 15:42
@TimvdLippe TimvdLippe requested a review from mrobinson June 19, 2026 16:29

@mrobinson mrobinson left a comment

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.

Nicely done. I think we should add support for the focus trigger for this too. You should be able to follow the cross-references of the various functions to see where it should be set, but generally I'd just fill it based on the caller.

Comment thread components/script/dom/node/focus.rs Outdated
continue;
}

// > 6.2. Let focusable area be descendant, if descendant is a focusable area; otherwise

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.

Suggested change
// > 6.2. Let focusable area be descendant, if descendant is a focusable area; otherwise
// > 1.2. Let focusable area be descendant, if descendant is a focusable area; otherwise

Comment on lines +305 to +312
let focusable_area = if !kind.is_empty() {
return Some(FocusableArea::Node {
node: descendant,
kind,
});
} else {
descendant.get_the_focusable_area()
};

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.

Any particular reason you are erasing the type of the focusable area here? I think losing IframeViewport is a bit of an issue. If there's no reason, this can be simplified:

Suggested change
let focusable_area = if !kind.is_empty() {
return Some(FocusableArea::Node {
node: descendant,
kind,
});
} else {
descendant.get_the_focusable_area()
};
let focusable_area = match {
Some(focusable_area) => return Some(focusable_area),
None => descendant.get_the_focusable_area()
};

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I do not quite get your suggestion. There is no focusable_area variable at that point.

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.

Oh, indeed. The thing missing here is how <iframe> is handled in focus.rs:129 (get_the_focusable_area). Maybe we can add a helper directly above that function fn focusable_area() with a comment that it is a helper for get_the_focusable_area (the specification function). This helper would handle <iframe> and also be used here.

Comment thread components/script/dom/node/focus.rs Outdated
Comment thread components/script/dom/node/focus.rs
@servo-highfive servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jun 23, 2026
skyz1 added 2 commits June 24, 2026 11:38
Signed-off-by: Glenn Skrzypczak <glenn.skrzypczak@gmail.com>
Signed-off-by: Glenn Skrzypczak <glenn.skrzypczak@gmail.com>
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Jun 24, 2026
@skyz1 skyz1 requested a review from mrobinson June 24, 2026 09:38
Signed-off-by: Glenn Skrzypczak <glenn.skrzypczak@gmail.com>
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.

3 participants