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

When Iphone & set "playsinline" attribute - need to show fullscreen button #3832

Closed
byungjuJin opened this issue Dec 30, 2021 · 5 comments · Fixed by #3853 or #4009
Closed

When Iphone & set "playsinline" attribute - need to show fullscreen button #3832

byungjuJin opened this issue Dec 30, 2021 · 5 comments · Fixed by #3853 or #4009
Labels
status: archived Archived and locked; will not be updated type: enhancement New feature or request
Milestone

Comments

@byungjuJin
Copy link

Have you read the FAQ and checked for duplicate open issues?
YES

Is your feature request related to a problem? Please describe.

Use Iphone.
Normally Safari only allow fullscreen when click play.
But with set "playsinline" attribute on

Describe the solution you'd like

  • Shaka check if "playsinline" attribute or not, show fullscreen button (or always show fullscreen button. almost all go to fullscreen, button is not matter)
  • OR make a API "playsinline" and show fullscreen button by API.

Describe alternatives you've considered

Before new feature, I should make a fullscreen button as a custom button.

Additional context

I know why someone write this : safari is the new IE

@byungjuJin byungjuJin added the type: enhancement New feature or request label Dec 30, 2021
@shaka-bot shaka-bot added this to the Backlog milestone Dec 30, 2021
@avelad
Copy link
Collaborator

avelad commented Jan 11, 2022

The ideal would be to use the standard and if it is not supported check if the webkit methods are supported:
webkitSupportsFullscreen
webkitEnterFullscreen
webkitExitFullscreen
webkitDisplayingFullscreen

Mote info in: https://developer.apple.com/documentation/webkitjs/htmlvideoelement/

@avelad
Copy link
Collaborator

avelad commented Jan 11, 2022

I can send a PR for this, if anyone is interested.

@joeyparrish
Copy link
Member

@avelad, I'm not certain what you are proposing. It sounds different from what @byungjuJin wrote. Can you elaborate? We already have a fullscreen polyfill to use the webkit methods when they are the only ones available.

It sounded to me like what @byungjuJin was saying was this:

Solution 1: If on iOS, and playsinline attribute is not set, hide/disable fullscreen button in UI.
Solution 2: If on iOS, and playsinline attribute is not set, fullscreen button should add playsinline attribute during UI initialization.

@avelad
Copy link
Collaborator

avelad commented Jan 11, 2022

On iOS, I don’t see the full screen button

A21E3463-2661-4BCD-A3D6-590C393E5204

@joeyparrish
Copy link
Member

And this is in spite of having the playsinline attribute on the video element in our demo.

We have this in the fullscreen button's constructor:

    // Don't show the button if fullscreen is not supported
    if (!document.fullscreenEnabled) {
      this.button_.classList.add('shaka-hidden');
    }

And we polyfill document.fullscreenEnabled based on prefixed methods on the document, but iOS has its methods on the video element. Now I understand the issue. I have some ideas on this:

Solution 1: Make the fullscreen polyfill more in-depth, using something like document.querySelectorAll('video') inside the polyfill methods to query video elements on iOS.
Solution 2: Make the fullscreen UI button call different methods on iOS.
Solution 3: Solution 2 + add playsinline if missing.

Solution 2 is simplest. Solution 1 benefits apps that don't use our UI, but also don't have iOS-specific UI code, at the expense of more complexity.

I'm leaning toward solution 2 or 3. I think if you've built your own UI, and you support iOS, you probably already have code for this in your UI. Our UI has more context than a polyfill (is associated with a specific video element), and so it can do a good job with less code, even if that code has platform-specific details in it.

theodab pushed a commit that referenced this issue Jan 12, 2022
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Mar 13, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 13, 2022
@avelad avelad modified the milestones: Backlog, v4.0 May 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
4 participants