Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

dialog: focus trap in FF seems to be broken in demos #7407

Closed
EladBezalel opened this issue Mar 4, 2016 · 3 comments
Closed

dialog: focus trap in FF seems to be broken in demos #7407

EladBezalel opened this issue Mar 4, 2016 · 3 comments
Assignees
Labels
browser: FireFox P0: critical Critical issues that must be addressed immediately. pr: merge ready This PR is ready for a caretaker to review
Milestone

Comments

@EladBezalel
Copy link
Member

Quoting @ErinCoughlan

On the demo site in Firefox, focus trap seems to be broken for alert and confirm dialogs.

@EladBezalel EladBezalel added browser: FireFox P0: critical Critical issues that must be addressed immediately. labels Mar 4, 2016
@EladBezalel EladBezalel self-assigned this Mar 4, 2016
@EladBezalel
Copy link
Member Author

@jelbourn can you explain why the bottom focus trap is a child of the dialog and not a sibling?
Seems like making it a sibling solving it.

EladBezalel added a commit that referenced this issue Mar 4, 2016
- bottomFocusTrap was appended to the dialog, in FF it seems like it didn't trapped the focus, ,moving it to be a sibling solves it.

fixes #7407
@EladBezalel EladBezalel added the pr: merge ready This PR is ready for a caretaker to review label Mar 4, 2016
@EladBezalel EladBezalel added this to the 1.0.7 milestone Mar 4, 2016
@jelbourn
Copy link
Member

jelbourn commented Mar 7, 2016

@EladBezalel it was easier to implement and I didn't see that it made a difference. Did you find that being a child was causing a problem?

@EladBezalel
Copy link
Member Author

yep, #7408 solves it

ThomasBurleson pushed a commit that referenced this issue Mar 7, 2016
- bottomFocusTrap was appended to the dialog, in FF it seems like it didn't trapped the focus, ,moving it to be a sibling solves it.

fixes #7407

  Closes #7408
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
browser: FireFox P0: critical Critical issues that must be addressed immediately. pr: merge ready This PR is ready for a caretaker to review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants