bdougherty / BigScreen

A simple library for using the JavaScript Full Screen API.
https://brad.is/coding/BigScreen/
MIT License
710 stars 77 forks source link

onenter and onexit events not being fired in Firefox 45.0 #20

Open terraboops opened 8 years ago

terraboops commented 8 years ago

I haven't nailed down why this is happening, but these events are not being fired. This issue affects Firefox only.

I tested this in Firefox 45.0 -- but I suspect this is happening in other versions also.

Here is a minimum test case with jQuery that will reproduce the issue:

function onEnterFullscreen() {
    alert("I don't fire in Firefox!");
}

function onExitFullscreen() {
    alert("I also don't fire in Firefox!");
}

$('#photo-demo img').on('click', function() {
    if (BigScreen.element !== this) {
        BigScreen.request(this, onEnterFullscreen, onExitFullscreen);
    }
    else {
        BigScreen.exit();
    }
}); 

I've tried to write a unit test to reproduce this issue, however the Unit Tests do not actually activate the Fullscreen API because use of the Fullscreen API requires User Interaction.

It is possible to simulate User Interaction with sendEvent in PhantomJS, but other browsers will require an some sort of event handler. I haven't found a way to tell the browser that it's okay to allow Fullscreen API requests without User Interaction.

For my purposes, I'm just going to use an onclick handler and make the test pass and use the resulting lib for my project. However, I'm wondering what solution you'd like me to implement so that the Unit Tests are actually testing the Fullscreen API. Do you use any CI tools? Is an onclick handler acceptable? Can I simulate User Interaction with PhantomJS for the CI tests if there are any?

Thanks!

terraboops commented 8 years ago

I found the issue, it's due to this block of code:

// If there's no element after 100ms, it didn't work. This check is for Safari 5.1
// which fails to fire a `webkitfullscreenerror` if the request wasn't from a user
// action.
setTimeout(function() {
    if (!document[fn.element]) {
        callOnError(iframe ? 'not_enabled' : 'not_allowed', element);
    }
}, 100);

In my own project, I've wrapped this in a User Agent test: navigator.userAgent.indexOf("Safari") > -1

bdougherty commented 8 years ago

Thanks for the detailed report! Seems like this is the same as #17. I generally dislike having to do UA detection, but I think it's the least bad way to handle it. Do you mind submitting a pull request with that change?

bdougherty commented 8 years ago

As for the tests, I'm happy to review a PR with any changes, but I want to note that I've started on a new major version that will be promise-based to match the latest spec. I definitely want to get better test coverage in actual browsers with the new version.

terraboops commented 8 years ago

Yeah, I agree on avoiding UA strings -- but for a hack I thought it might be okay. If I think of something better I'll let you know.

I'll submit a PR right away. Cool stuff about the new major version, I'll keep an eye out for it.

uc-ach commented 7 years ago

How this issue can be avoided? Because i'm also having same issue .onenter and .onexit not working on Firefox, what to do