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: Text tracks with src= on Safari & UITextDisplayer are sometimes rendered natively #8256

Merged
merged 7 commits into from
Mar 19, 2025

Conversation

matvp91
Copy link
Contributor

@matvp91 matvp91 commented Mar 12, 2025

This concerns src= mode. Safari loads text tracks separately, where on loadeddata, we might not have video.textTracks set to its final value (as Safari could still be processing another text track underneath).

This PR introduces an artificial wait on the addtrack and assumes, if no other track is added within the next 500ms, textTracks shall be in its final state.

Issues when we work with textTracks too early:

  • When we set mode of a textTrack to hidden (we'd do that to allow UiTextDisplayer to render text tracks), Safari might overwrite this decision (based on AUTOSELECT=YES) when a subsequent text track is parsed. This leaves us in an invalid state where textTracks are rendered natively but we have listeners bound to the wrong one. More info here: Text tracks with src= on Safari & UITextDisplayer are sometimes rendered natively. #8255
  • It could be no textTracks are available at all on loadeddata due to Safari internally defering the load for text tracks. This would break preferences.

@matvp91 matvp91 marked this pull request as draft March 12, 2025 14:59
@avelad avelad added type: bug Something isn't working correctly priority: P2 Smaller impact or easy workaround browser: Safari Issues affecting Safari or WebKit derivatives labels Mar 12, 2025
@avelad avelad added this to the v4.15 milestone Mar 12, 2025
@shaka-bot
Copy link
Collaborator

shaka-bot commented Mar 12, 2025

Incremental code coverage: 94.12%

@avelad avelad requested review from avelad and tykus160 March 19, 2025 09:19
@matvp91 matvp91 marked this pull request as ready for review March 19, 2025 09:20
@avelad avelad merged commit 306b682 into shaka-project:main Mar 19, 2025
48 of 52 checks passed
avelad added a commit that referenced this pull request Mar 19, 2025
…rendered natively (#8256)

This concerns `src=` mode. Safari loads text tracks separately, where on
`loadeddata`, we might not have `video.textTracks` set to its final
value (as Safari could still be processing another text track
underneath).

This PR introduces an artificial wait on the `addtrack` and assumes, if
no other track is added within the next 500ms, textTracks shall be in
its final state.

Issues when we work with `textTracks` too early:
- When we set mode of a textTrack to `hidden` (we'd do that to allow
UiTextDisplayer to render text tracks), Safari might overwrite this
decision (based on `AUTOSELECT=YES`) when a subsequent text track is
parsed. This leaves us in an invalid state where textTracks are rendered
natively but we have listeners bound to the wrong one. More info here:
#8255
- It could be no textTracks are available at all on `loadeddata` due to
Safari internally defering the load for text tracks. This would break
preferences.

---------

Co-authored-by: Álvaro Velad Galván <[email protected]>
avelad added a commit that referenced this pull request Mar 19, 2025
…rendered natively (#8256)

This concerns `src=` mode. Safari loads text tracks separately, where on
`loadeddata`, we might not have `video.textTracks` set to its final
value (as Safari could still be processing another text track
underneath).

This PR introduces an artificial wait on the `addtrack` and assumes, if
no other track is added within the next 500ms, textTracks shall be in
its final state.

Issues when we work with `textTracks` too early:
- When we set mode of a textTrack to `hidden` (we'd do that to allow
UiTextDisplayer to render text tracks), Safari might overwrite this
decision (based on `AUTOSELECT=YES`) when a subsequent text track is
parsed. This leaves us in an invalid state where textTracks are rendered
natively but we have listeners bound to the wrong one. More info here:
#8255
- It could be no textTracks are available at all on `loadeddata` due to
Safari internally defering the load for text tracks. This would break
preferences.

---------

Co-authored-by: Álvaro Velad Galván <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser: Safari Issues affecting Safari or WebKit derivatives priority: P2 Smaller impact or easy workaround type: bug Something isn't working correctly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Text tracks with src= on Safari & UITextDisplayer are sometimes rendered natively.
4 participants