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 iPhone video starting with a black screen on iOS 17 iPhone #924

Merged
merged 6 commits into from
Aug 19, 2024

Conversation

BPerlakiH
Copy link
Collaborator

@BPerlakiH BPerlakiH commented Aug 8, 2024

Fixes: #916
It seems it is an iOS 17 iPhone bug:
if we have a poster attribute set, the video starts with a black screen.

Content used:

Currently in the "New" section:

Solution:

TLDR: Remove poster attribute on iPhone iOS 17.

Remove the poster attribute and having a poster displayed:

Looking deeper inside the HTML content we are using, we have the following structures:

TED "I hope" | iPhone 17.5 | BEFORE poster removal

<div id="video-wrapper">
    <div id="vjs_video_3"
        data-setup="{&quot;techOrder&quot;: [&quot;html5&quot;, &quot;ogvjs&quot;], &quot;ogvjs&quot;: {&quot;base&quot;: &quot;assets/ogvjs&quot;}, &quot;autoplay&quot;: false, &quot;preload&quot;: true, &quot;controls&quot;: true, &quot;controlBar&quot;: {&quot;pictureInPictureToggle&quot;:false}}"
        poster="videos/87396/thumbnail.webp" playsinline="true" preload="auto"
        class="video-js vjs-default-skin vjs-fill vjs-paused vjs_video_3-dimensions vjs-controls-enabled vjs-touch-enabled vjs-v7 vjs-user-active"
        tabindex="-1" lang="en-gb" role="region" aria-label="Video Player"><video class="vjs-tech" preload="true"
            playsinline="playsinline" poster="videos/87396/thumbnail.webp"
            data-setup="{&quot;techOrder&quot;: [&quot;html5&quot;, &quot;ogvjs&quot;], &quot;ogvjs&quot;: {&quot;base&quot;: &quot;assets/ogvjs&quot;}, &quot;autoplay&quot;: false, &quot;preload&quot;: true, &quot;controls&quot;: true, &quot;controlBar&quot;: {&quot;pictureInPictureToggle&quot;:false}}"
            id="vjs_video_3_html5_api" tabindex="-1" src="videos/87396/video.webm">
            <source src="videos/87396/video.webm" type="video/webm">
        </video>
        <div class="vjs-poster" aria-disabled="false"
            style="background-image: url(&quot;videos/87396/thumbnail.webp&quot;);"></div>
</div>

TED "I hope" | iPhone 17.5 | AFTER poster removal

<div id="video-wrapper">
    <div id="vjs_video_3"
        data-setup="{&quot;techOrder&quot;: [&quot;html5&quot;, &quot;ogvjs&quot;], &quot;ogvjs&quot;: {&quot;base&quot;: &quot;assets/ogvjs&quot;}, &quot;autoplay&quot;: false, &quot;preload&quot;: true, &quot;controls&quot;: true, &quot;controlBar&quot;: {&quot;pictureInPictureToggle&quot;:false}}"
        poster="videos/87396/thumbnail.webp" playsinline="true" preload="auto"
        class="video-js vjs-default-skin vjs-fill vjs-paused vjs_video_3-dimensions vjs-controls-enabled vjs-touch-enabled vjs-v7 vjs-user-active"
        tabindex="-1" lang="en-gb" role="region" aria-label="Video Player"><video class="vjs-tech" preload="true"
            playsinline="playsinline"
            data-setup="{&quot;techOrder&quot;: [&quot;html5&quot;, &quot;ogvjs&quot;], &quot;ogvjs&quot;: {&quot;base&quot;: &quot;assets/ogvjs&quot;}, &quot;autoplay&quot;: false, &quot;preload&quot;: true, &quot;controls&quot;: true, &quot;controlBar&quot;: {&quot;pictureInPictureToggle&quot;:false}}"
            id="vjs_video_3_html5_api" tabindex="-1" src="videos/87396/video.webm">
            <source src="videos/87396/video.webm" type="video/webm">
        </video>
        <div class="vjs-poster" aria-disabled="false"
            style="background-image: url(&quot;videos/87396/thumbnail.webp&quot;);"></div>
</div>

TLDR: the poster is set up in 3 different places. Therefore removing the attribute from the video element only, is not causing any side effects, the poster itself is still displayed, but we are fixing the black screen issue.

Dynamic contents:

Some of the ZIM files, such as Deus ex silicium fr also used in the other issue, creates the <video> tag dynamically, eg. via VUE.js routing and XHR requests.

To "fix" the <video> tags in such dynamic cases, we can observe how the DOM is changing, and if a <video> tag was inserted we can run the same removePoster function on that.

@codecov-commenter
Copy link

codecov-commenter commented Aug 8, 2024

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 36.43%. Comparing base (b2a2040) to head (78e3d72).

Files Patch % Lines
ViewModel/BrowserViewModel.swift 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           main     #924       +/-   ##
=========================================
+ Coverage      0   36.43%   +36.43%     
=========================================
  Files         0      108      +108     
  Lines         0     6230     +6230     
=========================================
+ Hits          0     2270     +2270     
- Misses        0     3960     +3960     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rgaudin
Copy link
Member

rgaudin commented Aug 9, 2024

I thought we agreed to only touch content (DOM) on last resort. Here there is no explanation. I suppose videojs is not incompatible with posters on iOS or we should have an upstream ticket linked.

@BPerlakiH
Copy link
Collaborator Author

@rgaudin In this case it's the other way around a bit. It seems that iOS 17 broke the video API, and on iPhones the video starts with black screen when a poster is applied.
I haven't found anything else so far that could fix this, but this fix does work (tested on device).

@BPerlakiH
Copy link
Collaborator Author

Putting it slightly another way, I am not sure what our upstream options could be in this case:

  • on iOS 16 on the iPhone the same content is working. It is also working on iOS 17 on the iPad, similarly it works on macOS (14.5) as well. Apple won't fix this in a "back-ward way".
  • on the video.js level, I am not sure if it is possible to distinguish between the current OS version and current device where the html is displayed (additional complexity is that we are loading these pages via WKWebView).

@BPerlakiH BPerlakiH force-pushed the 916-iphone-video-starts-with-a-black-screen branch from 9dd48e1 to 8887eac Compare August 13, 2024 19:22
@BPerlakiH BPerlakiH changed the title Fix iPhone video starting with a black screen Fix iPhone video starting with a black screen on iOS 17 iPhone Aug 15, 2024
@kelson42
Copy link
Contributor

@BPerlakiH I'm ready to merge this if:

  • CI is green
  • If we have a ticket to reassess/rollback this in the future

@BPerlakiH
Copy link
Collaborator Author

@kelson42 re-assess / rollback issue is here: #939

@kelson42 kelson42 merged commit 7400493 into main Aug 19, 2024
4 checks passed
@kelson42 kelson42 deleted the 916-iphone-video-starts-with-a-black-screen branch August 19, 2024 11:28
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

Successfully merging this pull request may close these issues.

iPhone video starts with a black screen
4 participants