Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dialog focus restoring descendant check #10994

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

josepharhar
Copy link
Contributor

@josepharhar josepharhar commented Feb 4, 2025

In #9178 I accidentally made the currently focused element check look for the element to restore focus on as an ancestor instead of the dialog element "subject". This PR corrects this to look at the dialog element instead of the element to restore focus to.

This was identified here: #9178 (comment)

Not sure what to do with the checklist these days for a minor bug fix on already approved/agreed upon behavior.

  • At least two implementers are interested (and none opposed):
  • Tests are written and can be reviewed and commented upon at:
  • Implementation bugs are filed:
    • Chromium: …
    • Gecko: …
    • WebKit: …
    • Deno (only for timers, structured clone, base64 utils, channel messaging, module resolution, web workers, and web storage): …
    • Node.js (only for timers, structured clone, base64 utils, channel messaging, and module resolution): …
  • Corresponding HTML AAM & ARIA in HTML issues & PRs:
  • MDN issue is filed: …
  • The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)


/interactive-elements.html ( diff )

In whatwg#9178 I accidentally made the
currently focused element check look for the element to restore focus on
as an ancestor instead of the dialog element "subject". This PR corrects
this to look at the dialog element instead of the element to restore
focus to.

This was identified here: whatwg#9178 (comment)
@domenic
Copy link
Member

domenic commented Feb 5, 2025

Not sure what to do with the checklist these days for a minor bug fix on already approved/agreed upon behavior.

It would be good to ensure this is tested; could you link to tests which would fail if we did not implement the spec correctly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants