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

Re-assess / rollback JS fixes for videos #939

Open
BPerlakiH opened this issue Aug 18, 2024 · 13 comments
Open

Re-assess / rollback JS fixes for videos #939

BPerlakiH opened this issue Aug 18, 2024 · 13 comments
Assignees
Milestone

Comments

@BPerlakiH
Copy link
Collaborator

BPerlakiH commented Aug 18, 2024

Previously we were fixing reader side the following attributes in the content <video> html tag (for iPhones on iOS 17 only):

If there's a content side way to fix this, we should remove these hacks.

@benoit74
Copy link

benoit74 commented Sep 2, 2024

Is there any issue opened somewhere (in video.JS repository?) to track the fact that this is not working properly and should be fixed upstream? I don't mind to open the issue(s) if needed, but you probably have more context + these issues should be linked here so that we know when / if the issue is fixed upstream.

@rgaudin
Copy link
Member

rgaudin commented Sep 2, 2024

It's not a video.js bug ; see #916 (comment)

@benoit74
Copy link

benoit74 commented Sep 2, 2024

These issues could be "worked-around" at video.JS level because all video.JS users with an iPhone 17 have the problem.

Video.JS could probably detect that condition and remove the poster / add the playsinline ; or do I miss something?

@rgaudin
Copy link
Member

rgaudin commented Sep 2, 2024

Possibly, I think all the selection criteria are available in User-Agent and video format

Here's my UA in Kiwix 3.5.1 on macOS 14.6.1 for instance

Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko)

Same on Safari

Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.6 Safari/605.1.15

Now on affected iPhone

Mozilla/5.0 (iPhone; CPU iPhone OS 17_5_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148

And on Safari

Mozilla/5.0 (iPhone; CPU iPhone OS 17_5_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.5 Mobile/15E148 Safari/604.1

@kelson42
Copy link
Contributor

@benoit74 I would like to move this issue in openzim/youtube or openzim/python-scraperlib. Is that OK for you?

@benoit74
Copy link

This problem is specific to apple platforms, so I don't think it really helps to move the issue.

The real problem is in these apple platforms / readers, the scraper is producing valid HTML5 code, only apple platforms are not capable to process it correctly.

And once problem is solved in video.JS, we still need to rollback JS fixes in kiwix-apple.

What is your intention by moving this issue? (if the goal is that I work on this issue rather than @BPerlakiH, then just assign me this issue, I don't mind to work on kiwix-apple issue ^^)

@kelson42 kelson42 assigned benoit74 and unassigned BPerlakiH Oct 10, 2024
@kelson42
Copy link
Contributor

I have assigned the issue to @benoit74. I will postponed the issue to the next milestone. At this stage the point is to move the issue where the root cause is and will be fixed, do we already know where it will be?

@kelson42 kelson42 modified the milestones: 3.6.0, 3.7.0 Oct 10, 2024
@benoit74
Copy link

I need help to reproduce the issue.

I tried to setup a basic HTML page with "what looks to be the bug condition": a video.js player, with a poster attribute and without the playsinline attribute. It is accessible online at https://www.oviles.info/video-js-ios-17.html (it is just a basic HTML page, you can have a look at the "beautiful" source code to check).

When I open the page on iOS 17 iPhone on browserstack with Safari, I do not get the bug. Is it because the bug only happens with our web view ? Do you have a chance to test this ?

I do not own any iPhone and I do not have a simulator, so pretty hard for me to go further. And without a simple repro, it will be hard to get video.js folks attention.

Btw, is it correct the web view component we are using is named WKWebView?

@rgaudin
Copy link
Member

rgaudin commented Oct 11, 2024

@benoit74 please read my previous comment here pointing to this other comment of mine

@benoit74
Copy link

Thank you, I missed that! Are you able to quickly check it is still a problem with latest iOS 17 then? (I would prefer to not open an issue if bug has been fixed by Apple)

@rgaudin
Copy link
Member

rgaudin commented Oct 14, 2024

@benoit74 you mean iOS18 I guess? I just tested and it's still not working properly but symptom is not reliable: sometimes I get correct video playback but no sound, sometime, I have both video and sound but it's jerky.

@benoit74
Copy link

I meant latest iOS 17 in case they've patched it since the previous tests. But iOS 18 is fine as well. Thank you anyway!

@benoit74
Copy link

Issue opened upstream: videojs/video.js#8895

I did not spoke about playsinline since this is something different from my PoV, more linked to the way we want the webpage to behave and in such a situation I feel like it is our responsibility to properly add playsinline attribute. Or did I missed something?

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

No branches or pull requests

4 participants