Fix: Trap focus in overlaid popups (Issue #3141)#3206
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Thank you for opening this PR! I tested each of the nag modals and focus trapping works!
However, the trap works now because #instruction comes before all other focusable elements in the DOM. Focus isn't explicitly moved into the modal when it opens. If a focusable element is ever added earlier in popup.html, the first Tab press will focus there instead of inside the modal, and focus won't be trapped in the modal until the user tabs into it.
| let is_tab = e.key === "Tab" || e.keycode === 9; | ||
| if (!is_tab) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
keycode will be undefined because the property is keyCode, but I think you can remove the check because the property is deprecated (https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode) and key is supported by Privacy Badger's minimum browser versions
|
Thank you for contributing to Privacy Badger! If we take a step back, what we're dealing with here is a partially-implemented As far as I can see though, there is no need at this point to re-implement Would you be interested in opening a new pull request that fixes #3141 by switching |
|
Sure, I work on that approach. With that being the case, please feel free to close this PR. |
|
Let me know if I can help with the |
3 participants
This code update should address issue #3141 (Keyboard navigation not properly trapped within welcome page reminder/critical error/important update notification modal). Changes are scoped to elements in "#instruction" dialog element in popup.html.