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

safari keyboard only nav broken in mediaelement #1873

Closed
knomedia opened this issue Oct 6, 2016 · 38 comments
Closed

safari keyboard only nav broken in mediaelement #1873

knomedia opened this issue Oct 6, 2016 · 38 comments

Comments

@knomedia
Copy link

knomedia commented Oct 6, 2016

Within Safari v 10.0 (the latest Safari for my OS at the time), when using the keyboard to navigate a page with mediaelement, focus gets lost, and sent back to the <body> making it impossible to navigate the page for keyboard only users.

Steps to reproduce:

  • navigate to mediaelementjs.com
  • use <tab> to move focus around the page
    • you may need to option+tab depending on your safari / os x settings
  • note that you will eventually tab to the mejs-container, and then the video element
  • the next tab press will send focus to the <body>
  • the same behavior happens when reverse tabbing through the control bar, to play, to the video object, and then to body

Expected behavior:

  • keyboard only users should be able to tab through the mediaelement experience without loosing focus.

I'd love to put together a fix for this and have been chasing it down for a bit, but haven't come up with anything useful yet. Any guidance on where I might poke around to find the issue?

safari_version

@rafa8626
Copy link
Contributor

rafa8626 commented Oct 6, 2016

I can only say that if there's an issue related to that the first place to look at is mep-player.js and look for focus and focusout strings as a way to start. Do you have the same issue in the test/test.html file?

@knomedia
Copy link
Author

knomedia commented Oct 7, 2016

just confirmed that the issue exists for me in the test/test.html and in the demo/index.html files as well.

i'll keep digging, thanks for the potential starting place

@rafa8626
Copy link
Contributor

rafa8626 commented Oct 7, 2016

@knomedia You are welcome. Keep us posted about your findings

@rafa8626 rafa8626 added the Bug label Oct 11, 2016
@rafa8626
Copy link
Contributor

@knomedia I tested using option + tab and I was able to focus properly using 3.x-dev branch; please let me know if it works for you as well in that branch

@rafa8626
Copy link
Contributor

Closing this issue since no answer has been posted in more than 2 weeks. If you'd like to reopen this, let us know. Thanks!

@knomedia
Copy link
Author

apologies for going dark. verified that the issue is not found on the 3.x-dev branch.

@rafa8626
Copy link
Contributor

Great news! Thank you very much

@tiffwyatt
Copy link

tiffwyatt commented Feb 8, 2017

This bug seems to have reappeared? I just tested the video player on http://www.mediaelementjs.com using Safari 10.0.3 and the behaviour is exactly the same as described by knomedia in the first post on this issue.

I can't tab through the controls of the player itself, nor can I navigate to any link after the player, because I keep losing focus and getting sent back to the first Safari tab.

Can anyone else confirm this?

@rafa8626
Copy link
Contributor

rafa8626 commented Feb 8, 2017

Yeah I can see this. Odd. I'll try to check what's going on with this.

@rafa8626 rafa8626 reopened this Feb 8, 2017
@rafa8626
Copy link
Contributor

rafa8626 commented Feb 8, 2017

This seems to be an issue with Safari itself. I tested this with YouTube and it behaves the same. @tiffwyatt can you confirm this?

@tiffwyatt
Copy link

No, sorry, can't confirm. YouTube is working nicely in Safari, can navigate using only the keyboard without any problems.
Strange …

@rafa8626
Copy link
Contributor

rafa8626 commented Feb 8, 2017

OK I'll check some more. If you find anything let me know

@rafa8626
Copy link
Contributor

rafa8626 commented Feb 9, 2017

@knomedia I found out that this piece on the code causes the lost of focus in Safari:

t.container.find('.' + t.options.classPrefix + 'mediaelement').append(t.$media);

Any thoughts on how this could be affecting Safari? It seems like it doesn't like the idea of appending video/audio without losing focus for some reason, because I tried to insert a paragraph for testing purposes and it worked (obviously, adding some tabindex elements)

@rafa8626
Copy link
Contributor

@evanhuntley can you advise on this, please? You have more experience dealing with this type of quirks for accessibility than myself.

@rafa8626
Copy link
Contributor

rafa8626 commented Feb 16, 2017

@tiffwyatt can you download the latest version and use the keyboard-fix branch and test it in different browsers along with Safari, please (including mobile devices)? I think I figured out the best way to do this, but I need to make sure this is 100% tested before merging to master. I used in Safari Alt+Tab, and other browsers just Tab. Any feedback in the next few days I'd appreciate it since I'm planning to release the next version at the beginning of next week. Thanks

@rafa8626
Copy link
Contributor

@isantolin Is this something you can assist me as well please? This is for accessibility purposes so it's a matter of using keyboard properly in different browsers (the ones described on README).

@rafa8626 rafa8626 removed the Bug label Feb 16, 2017
@tiffwyatt
Copy link

@Ron666 Yes, will test this later today and let you know if it works. Thanks for looking into this!

@rafa8626
Copy link
Contributor

@tiffwyatt You are welcome. Keep me posted

@rafa8626
Copy link
Contributor

Thanks @isantolin

@rafa8626
Copy link
Contributor

Any updates on this?

@tiffwyatt
Copy link

Hi, sorry, took me a while to get round to testing …
But have done so now and it's looking good! Keyboard-only navigation in Safari is working well. Also tested in Firefox (51.0.1) and Chrome (56.0.2924.87), no problems.

@rafa8626
Copy link
Contributor

rafa8626 commented Feb 20, 2017

@tiffwyatt How about IE? Have you been able to try MS Edge, or any version between 9 and 11?

@rafa8626
Copy link
Contributor

If you confirm no issues at all also with IE, I'll merge this fix

@tiffwyatt
Copy link

@Ron666 As I am on a Mac I tested IE 9, 10, 11 and Edge using https://crossbrowsertesting.com. Didn't encounter any problems while navigating keyboard-only. Will test Edge on a Windows machine at my office tomorrow morning and let you know – but as far as I can see your fix is working well.

@rafa8626
Copy link
Contributor

Great! Well let me know ASAP so I can put the fix in place for the next release. Thanks for testing this

@tiffwyatt
Copy link

@Ron666 Just tested with Edge and Chrome on Windows 7 – working well! Thanks a lot for fixing this.

@rafa8626
Copy link
Contributor

Perfect. Great news. The fix has been integrated now and will be available in the CDN for next release. Closing this ticket now

@tiffwyatt
Copy link

tiffwyatt commented Mar 8, 2017

@Ron666 Really sorry, but I just noticed the Safari keyboard navigation bug isn't completely resolved after all. Before your fix, the behaviour was:

"I can't tab through the controls of the player itself, nor can I navigate to any link after the player, because I keep losing focus and getting sent back to the first Safari tab."

Now, after the fix, I can tab through the controls of the player itself – that part works perfectly. The trouble is, after tabbing through the last control the focus still gets lost and is sent back to the first Safari window tab. Because of this it is impossible to ever navigate past the video/audio element to any content further down on the page – effectively a keyboard trap.

I didn't realize this while testing, as the video was the only item on my test page.

Would be brilliant if you could find a fix for this! Will gladly test anything you might come up with.

@rafa8626
Copy link
Contributor

rafa8626 commented Mar 8, 2017

I see. I'll try to check this ASAP and keep you posted. Thanks

@rafa8626 rafa8626 reopened this Mar 8, 2017
@rafa8626
Copy link
Contributor

rafa8626 commented Mar 9, 2017

Do you have a URL I can use to replicate the issue?

@tiffwyatt
Copy link

You can test this on http://www.mediaelementjs.com:

Start tabbing from the top of the page and keep going until you reach the last control in the "Big Buck Bunny" Video (the fullscreen control).
Now, if you press tab once more, focus should move on to the select element "Try other media sources" under the video. But it doesn't, instead focus gets lost. Pressing tab again moves focus up to the first Safari browser tab.

(This is assuming your Safari browser is configured to Full Keyboard Access – http://www.weba11y.com/blog/2014/07/07/keyboard-navigation-in-mac-browsers/)

@rafa8626
Copy link
Contributor

@tiffwyatt this is an issue with Safari, so I have submitted a bug report for this. Let's wait and see what they say

@tiffwyatt
Copy link

@Ron666 Okay, thanks!

@rafa8626
Copy link
Contributor

@tiffwyatt I have some news for you: I think the problem with Safari is that when using appendChild it doesn't append the children elements of it, and they get lost for some reason, but not it's focus. So I'll be working on something to make sure we cover this scenario and I'll keep you posted

@rafa8626
Copy link
Contributor

rafa8626 commented Apr 5, 2017

@knomedia and @tiffwyatt I have a new branch (keyboard-fix) that solves this issue, but I'll need a very robust test on OSX and Windows. If you can assist me on testing to make sure everything is working as expected I'd appreciate it. I had to remove any children from the original video/audio tag and save them in 3 different categories and then, if destroyed, recreate them. Please let me know ASAP. Thanks

@rafa8626
Copy link
Contributor

rafa8626 commented Apr 5, 2017

@tiffwyatt I tested on Mac and so far it has been no issues at all. Once you confirm with Safari on your side, as well as IE9+ and Edge I think I'll be good to merge this

@rafa8626
Copy link
Contributor

rafa8626 commented Apr 6, 2017

@tiffwyatt I just restricted this for Safari desktop. So I tested this and it's working fine. It has been merged now to the master branch. Thanks.

@rafa8626 rafa8626 closed this as completed Apr 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants