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

fix: enter fullscreen from fullWindow does not trigger exitFullWindow #7939

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tapornanan
Copy link

@tapornanan tapornanan commented Sep 24, 2022

Description

Fix #7705

Specific Changes proposed

When the player is playing in full window mode enterFullWindow() and then calls requestFullscreen() but the full window state not get reset

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Change has been verified in an actual browser (Chrome, Firefox, IE)
    • Unit Tests updated or fixed
    • Docs/guides updated
    • Example created (starter template on JSBin)
  • Reviewed by Two Core Contributors

@welcome
Copy link

welcome bot commented Sep 24, 2022

💖 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.

@gkatsev
Copy link
Member

gkatsev commented Oct 6, 2022

This does make sense on the surface, but fullWindow was originally created as a fallback for lack of native fullscreen support and it's still there at the end of a bunch of if/else statements

video.js/src/js/player.js

Lines 2929 to 2932 in b58a220

// fullscreen isn't supported so we'll just stretch the video element to
// fill the viewport
this.enterFullWindow();
}

So, a bit weird to exit it only to maybe enter it immediately after.

@tapornanan
Copy link
Author

@gkatsev Thanks you

In my case, There was a requirement that we need to load videojs and play the video in fullscreen whenever the user clicked on UI. It works fine on high-end/fast devices but in some case, there is some low-end/slow device that is failed to run requestFullscreen() within valid user gesture time therefore I add an extra check to go full window if the fullscreen is failed. Then we wait for the user to click on the play button in the video we will again trigger requestFullscreen which this time is usually successful because all scripts are already loaded. But the state of videojs is still shown that isFullWindow still true 😢

@mister-ben mister-ben added the needs: discussion Needs a broader discussion label Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: discussion Needs a broader discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Going fullscreen from full window does not trigger exitFullWindow
3 participants