fix: IE11 and Edge focus on fullscreen toggle

[shaneafsar]

On IE11 and Edge, focus is moved to the window when fullscreen is toggled. This ensures screen readers keep focus on the FullscreenToggle element after activating the button.

Fixes #6242

Description

See #6242

Specific Changes proposed

-Added this.el().focus() to the FullscreenToggle component whenever "handleClick" is invoked.

Requirements Checklist

• [X] Feature implemented / Bug fixed
• [X] If necessary, more likely in a feature request than a bug fix
  • [X] Change has been verified in an actual browser (IE, Edge)
• [ ] Reviewed by Two Core Contributors

[welcome[bot]]

💖 Thanks for opening this pull request! 💖

Things that will help get your PR across the finish line:

• Run npm run lint -- --errors locally to catch formatting errors earlier.
• Include tests when adding/changing behavior.
• Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

[gjanblaszczyk]

Hi ima007, I have one question about testing... Did you test focus-visible.html example page from sandbox directory after your changes on Chrome/Firefox? Does it work the same if you click via the mouse on the fullscreen button? I am asking because I haven't checked your changes yet and I wonder if some behavior changed after your PR.

[shaneafsar]

Hi ima007, I have one question about testing... Did you test focus-visible.html example page from sandbox directory after your changes on Chrome/Firefox? Does it work the same if you click via the mouse on the fullscreen button? I am asking because I haven't checked your changes yet and I wonder if some behavior changed after your PR.

Hey @gjanblaszczyk , thanks for the question & consideration here. Tapping/clicking the fullscreen button on Chrome/Firefox should work the same (in testing before this PR, focus remained on the fullscreen toggle). I also just checked the focus-visible.html example page on Chrome and Firefox.

[shaneafsar]

You’re correct; I believe the fullscreenchange event is what gets fired when the promise for requesting fullscreen is resolved.

This change should likely be moved to handleFullscreenChange and detect if it was from activating the button.

On Thu, Oct 3, 2019 at 8:13 PM Owen Edwards [email protected] wrote:

@OwenEdwards commented on this pull request.

In src/js/control-bar/fullscreen-toggle.js https://github.com/videojs/video.js/pull/6243#discussion_r331301993:

@@ -74,8 +73,13 @@ class FullscreenToggle extends Button { } else { this.player_.exitFullscreen(); }

• /**

• * On IE11 and Edge, focus is moved to the window when fullscreen is
  

• * enabled. This ensures screen readers keep focus on the FullscreenToggle
  

• * element after activating the button.
  

• */
  

• this.el().focus();

I'm wondering if this is going to work consistently; isn't the switch to Full Screen a request that may happen asynchronously? In that case, this focus() may happen before the request has completed.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/videojs/video.js/pull/6243?email_source=notifications&email_token=AAIXY357ML4DH35O2D2ON3TQM2DELA5CNFSM4I2FVEE2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCG3R5XA#pullrequestreview-297213660, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIXY32XRTROTNRFTDJM7UTQM2DELANCNFSM4I2FVEEQ .

[shaneafsar]

@OwenEdwards -- Since the promise returned by requestFullScreen isn't available in all browsers, I took a quick approach with storing the current click target, and checking that in the fullscreenchange event handler for the FullscreenToggle, since by that point, the async should be completed.

[mister-x-ops]

are these changes mean the controls will be visible all the time in the fullscreen if the user doesn't change the focus on another element or click play/pause e.g?

[gkatsev]

Seems like this change breaks the focus visible stuff in Firefox. Seems to be fine in Chrome, though.

@o-hreshchuk I don't think it'll keep the controls up.