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: Polyfill missing AbortController on Tizen #4707

Merged
merged 1 commit into from
Nov 15, 2022

Conversation

joeyparrish
Copy link
Member

We started using AbortController in HlsParser in 777c27e, but that change was made while the Tizen TV in our lab was offline. This restores functionality to Tizen.

This polyfill uses getters, a feature of ES6 that the compiler won't transpile into ES3, so this also changes the output language to ES5. This should not be an issue, since ES5 has been well supported in all browsers since IE10.

We started using AbortController in HlsParser in 777c27e, but that
change was made while the Tizen TV in our lab was offline.  This
restores functionality to Tizen.

This polyfill uses getters, a feature of ES6 that the compiler won't
transpile into ES3, so this also changes the output language to ES5.
This should not be an issue, since ES5 has been well supported in all
browsers since IE10.
@joeyparrish
Copy link
Member Author

This should unblock PR #4698.

@avelad
Copy link
Member

avelad commented Nov 14, 2022

@joeyparrish I'm a bit scared about this. What happens if a platform supports fetch and not AbortController. Will this polyfill be able to abort the current request?

See: https://github.com/shaka-project/shaka-player/blob/main/lib/net/http_fetch_plugin.js#L233

AbortController --> https://caniuse.com/abortcontroller --> Since Chrome 66
Fetch --> https://caniuse.com/fetch --> Since Chrome 42
ReadableStream --> https://caniuse.com/mdn-api_readablestream --> Since Chrome 43

The problem occurs between Chrome 43 and Chrome 65 --> Tizen 3 for example https://developer.samsung.com/smarttv/develop/specifications/web-engine-specifications.html

@avelad
Copy link
Member

avelad commented Nov 14, 2022

It seems that fetch signal is not supported until Chrome 66 https://developer.mozilla.org/en-US/docs/Web/API/fetch

@github-actions
Copy link
Contributor

Incremental code coverage: 59.77%

@joeyparrish
Copy link
Member Author

joeyparrish commented Nov 14, 2022

@avelad, I'll get Chrome 65 and find out what happens. Though I doubt it's still in use on desktops, it could be in CE devices.

@joeyparrish
Copy link
Member Author

joeyparrish commented Nov 14, 2022

@avelad, I found that fetch() from Chrome 65 does not respect the non-native AbortController, because fetch implementations from that era did not know about the signal option (an AbortSignal instance). So if you try to use a very old fetch in combination with a polyfilled AbortSignal, aborting the request is not possible.

However, I also found that shaka.net.HttpFetchPlugin captures window.AbortController before our built-in polyfills are installed. So the app ends up using XHR anyway.

@avelad
Copy link
Member

avelad commented Nov 15, 2022

I am going to test on several Tizen and WebOS devices, if it works correctly I will approve the PR.

@avelad
Copy link
Member

avelad commented Nov 15, 2022

Tested in Tizen 3.0, Tizen 4.0 and Tizen 5.0 and it works! (and uses xhr)

@avelad avelad added type: bug Something isn't working correctly platform: WebOS Issues affecting WebOS platform: Tizen Issues affecting Tizen priority: P1 Big impact or workaround impractical; resolve before feature release labels Nov 15, 2022
@avelad avelad added this to the v4.4 milestone Nov 15, 2022
@avelad
Copy link
Member

avelad commented Nov 15, 2022

Tested in WebOS 3.0, WebOS 3.5 and WebOS 4.5 and it works! (and uses xhr)

@avelad avelad merged commit 75ef975 into shaka-project:main Nov 15, 2022
@joeyparrish joeyparrish deleted the fix-tizen-tests branch November 15, 2022 15:33
joeyparrish added a commit that referenced this pull request Dec 8, 2022
We started using AbortController in HlsParser in 777c27e, but that
change was made while the Tizen TV in our lab was offline. This restores
functionality to Tizen.

This polyfill uses getters, a feature of ES6 that the compiler won't
transpile into ES3, so this also changes the output language to ES5.
This should not be an issue, since ES5 has been well supported in all
browsers since IE10.
joeyparrish added a commit that referenced this pull request Dec 8, 2022
We started using AbortController in HlsParser in 777c27e, but that
change was made while the Tizen TV in our lab was offline. This restores
functionality to Tizen.

This polyfill uses getters, a feature of ES6 that the compiler won't
transpile into ES3, so this also changes the output language to ES5.
This should not be an issue, since ES5 has been well supported in all
browsers since IE10.
@pahonski
Copy link

pahonski commented Apr 3, 2023

@joeyparrish I'm not sure if it is regarding this issue but I found a problem on Playstation4 (WebMaf 3.1.1). It appeared from the 4.3.1 version of shaka. When I start the playback I receive an error TypeError: DOMExceptionConstructor is not a constructor (evaluating 'new DOMException("signal is aborted without reason","AbortError")) and then PlayStation crashes.

@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
platform: Tizen Issues affecting Tizen platform: WebOS Issues affecting WebOS priority: P1 Big impact or workaround impractical; resolve before feature release status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants