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

Fullscreen in Safari 10 not working #34

Closed
peteresser opened this issue Dec 8, 2016 · 3 comments
Closed

Fullscreen in Safari 10 not working #34

peteresser opened this issue Dec 8, 2016 · 3 comments

Comments

@peteresser
Copy link

@peteresser peteresser commented Dec 8, 2016

Hi There,

First of all; great work guys, really like your elements and components. We hope to contribute in the near future. A customer was reporting issues with Safari 10 and fullscreen. After debugging I also came across this drupal website, where a user has found the root cause and solution.

https://www.drupal.org/node/2805311

The problem is the detection of the Safari version. As they now have version 10 (2 digits), your regular expression is only taking 1 digit, so it sees it as Safari < 7 and no real full screen is enabled. But is also not enabling the semi full screen modus (as @paalj there is asking), thus the button is not doing anything.

Version 8 and 9 of Safari are working fine, as I just checked it with Browserstack.
Can you please fix this in your main h5p.org distribution as embeds from H5P are not working with full screen in Safari 10?

If I can assist in any way, let me know.

Peter

@peteresser

This comment has been minimized.

Copy link
Author

@peteresser peteresser commented Dec 8, 2016

Fix found and made by lestt added as reference:

h5p_safari_fullscrean_bug.patch.txt

H5P.safariBrowser = navigator.userAgent.match(/version/([.\d]+)/i);

@fnoks

This comment has been minimized.

Copy link
Member

@fnoks fnoks commented Dec 12, 2016

Thank you for your investigations, and for providing a patch. We will add it ASAP.

@thomasmars

This comment has been minimized.

Copy link
Member

@thomasmars thomasmars commented Dec 12, 2016

This has been fixed and commited to 'master' branch of library, and should be included in the upcoming release.

@thomasmars thomasmars closed this Dec 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.