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

BigScreen.Enabled Not Correctly Set in Safari 5.1.x #18

Open
own3mall opened this issue Feb 2, 2016 · 5 comments
Open

BigScreen.Enabled Not Correctly Set in Safari 5.1.x #18

own3mall opened this issue Feb 2, 2016 · 5 comments

Comments

@own3mall
Copy link

own3mall commented Feb 2, 2016

I've noticed on my iframe content that document.webkitFullscreenEnabled is undefined when using the developer tools in Safari. Thus, using webkitFullscreenEnabled to determine if BigScreen.enabled is true will NOT work.

There may be a better way to fix it, but I would suggest just checking to see if the document.webkitIsFullScreen property exists to determine whether or not the fullscreen API is supported. Here's what I changed to the bigscreen source code to make it work properly in Safari:

Line 12: Added webkitIsFullScreen as one of the properties to check for enabled:

enabled: ['fullscreenEnabled', 'webkitFullscreenEnabled', 'mozFullScreenEnabled', 'msFullscreenEnabled', 'webkitIsFullScreen'],

Line 215: Added a check that enabled is not set to webkitIsFullScreen:

if (iframe && (document[fn.enabled] === false && fn.enabled !== "webkitIsFullScreen")) {

Under line 336, this was added:

if (fn.enabled === 'webkitIsFullScreen'){ return true; }

I've also attached my version to this post. Maybe there is a better way to check for fullscreen support in Safari. BigScreen.enabled should be true for me because 5.1.x does support it, but the document doesn't always have the webkitFullscreenEnabled property. It can be undefined (in my case with iframe content) which means BigScreen will incorrectly say that it does NOT support fullscreen when it does.
bigscreen_modified_safari_detection.zip

@own3mall
Copy link
Author

own3mall commented Feb 2, 2016

With this fix, the only way I can get iframe content to truly go into full screen is to add the "webkitAllowFullScreen" property to the iframe. "allowfullscreen" is not enough.

Example:

<iframe src="{WEBPAGE_WITH_BIGSCREEN_STUFF_HERE}" allowfullscreen webkitAllowFullScreen></iframe>

That's the only way I can get it to work. webkitAllowFullScreen must be added to the iframe.

@bdougherty
Copy link
Owner

I think this is actually by design. BigScreen.enabled should only be true if calling BigScreen.request would actually work. If you don't have webkitallowfullscreen added to the iframe, that request won't work.

If all you're interested in is doing fullscreen for a video (which is most of the reason this library exists), BigScreen.videoEnabled(video) is there to check for that. There's an older fullscreen api that works only on videos that is enabled even if the iframe lacks the fullscreen attribute.

@own3mall
Copy link
Author

own3mall commented Feb 5, 2016

No, in my case, I want the body of the entire HTML document in the iframe to go fullscreen. Without my changes, it won't go fullscreen in Safari when there is no reason for it to NOT go into fullscreen.

Basically, it should go fullscreen since it works just fine, but BigScreen.enabled is false until my changes are made due to checking a property that doesn't exist on the dom in an iframe.

@bdougherty
Copy link
Owner

Can you make a CodePen or JSFiddle that shows that it works? The fullscreen request should fail without the webkitallowfullscreen attribute on the iframe.

@own3mall
Copy link
Author

own3mall commented Feb 5, 2016

Please test the attached HTML file in all browsers. When you click on the image in each iframe, the image should go fullscreen. Try it in Safari. When you click on the first image in the iframe, the image does not go fullscreen. The bottom iframe works as expected which contains the changes I made to BigScreen.js

This issue is not related to having the "webkitAllowFullScreen" attribute present on the iframe. It is however required to get it to work with my changes.

test_in_safari.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants