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 React Native createObjectURL polyfill incompatibility #1845

Merged
merged 1 commit into from
Mar 20, 2019
Merged

Fix React Native createObjectURL polyfill incompatibility #1845

merged 1 commit into from
Mar 20, 2019

Conversation

nitro404
Copy link
Contributor

React Native introduces its own polyfill for window.URL.createObjectURL which is not compatible with Shaka. Encapsulating a reference to the original function inside of the MediaSourceEngine circumvents this issue.

Issue #1842

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@nitro404
Copy link
Contributor Author

I signed the CLA.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@shaka-bot
Copy link
Collaborator

Test Failure:

Generating Closure dependencies...
Linting JavaScript...

/var/lib/jenkins/workspace/Manual PR Test (local-tests)/test/media/media_source_engine_unit.js
164:1  error  Line 164 exceeds the maximum line length of 80  max-len
179:1  error  Line 179 exceeds the maximum line length of 80  max-len

✖ 2 problems (2 errors, 0 warnings)

END-BUILD: FAILURE
Build step 'Execute shell' marked build as failure

@joeyparrish
Copy link
Member

@nitro404, can you please fix the linter errors? You can check this for yourself by running python build/all.py before pushing your commits.

@joeyparrish joeyparrish added this to the v2.5 milestone Mar 18, 2019
@joeyparrish joeyparrish added the type: enhancement New feature or request label Mar 18, 2019
@nitro404
Copy link
Contributor Author

Should be fixed! I'm unfortunately not able to run "test.py" on my current environment, the script fails with an error, likely because I have Python 3 installed.

@joeyparrish
Copy link
Member

Ah, yeah, those scripts currently require python2. If you have that, you can run python2 build/test.py.

But I'll run it through the build bot again.

@shaka-bot
Copy link
Collaborator

Test Failure:

Generating Closure dependencies...
Linting JavaScript...
Linting HTML...

Config loaded: /var/lib/jenkins/workspace/Manual PR Test (local-tests)/.htmlhintrc

Config loaded: /var/lib/jenkins/workspace/Manual PR Test (local-tests)/.htmlhintrc

Config loaded: /var/lib/jenkins/workspace/Manual PR Test (local-tests)/.htmlhintrc

Scanned 3 files, no errors found (22 ms).
Checking that the build files are complete...
Checking the tests for type errors...
/var/lib/jenkins/workspace/Manual PR Test (local-tests)/test/media/media_source_engine_unit.js:165: ERROR - Access to private property createObjectURL_ of shaka.media.MediaSourceEngine not allowed here.
shaka.media.MediaSourceEngine.createObjectURL_;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

/var/lib/jenkins/workspace/Manual PR Test (local-tests)/test/media/media_source_engine_unit.js:180: ERROR - Access to private property createObjectURL_ of shaka.media.MediaSourceEngine not allowed here.
shaka.media.MediaSourceEngine.createObjectURL_ =
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

/var/lib/jenkins/workspace/Manual PR Test (local-tests)/test/media/media_source_engine_unit.js:180: ERROR - constant property createObjectURL_ assigned a value more than once
shaka.media.MediaSourceEngine.createObjectURL_ =
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

/var/lib/jenkins/workspace/Manual PR Test (local-tests)/test/media/media_source_engine_unit.js:192: ERROR - Access to private property createObjectURL_ of shaka.media.MediaSourceEngine not allowed here.
shaka.media.MediaSourceEngine.createObjectURL_ = originalCreateObjectURL;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

/var/lib/jenkins/workspace/Manual PR Test (local-tests)/test/media/media_source_engine_unit.js:192: ERROR - constant property createObjectURL_ assigned a value more than once
shaka.media.MediaSourceEngine.createObjectURL_ = originalCreateObjectURL;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

5 error(s), 0 warning(s), 91.7% typed
Build failed
END-BUILD: FAILURE
Build step 'Execute shell' marked build as failure

@joeyparrish
Copy link
Member

For variables you reassign, use let instead of const. For the private access issue, you can make the member public (remove the @private annotation and the trailing underscore) and make it clear in the comments that it is for use by tests only.

React Native introduces its own polyfill for window.URL.createObjectURL which is not compatible with Shaka. Encapsulating a reference to the original function inside of the MediaSourceEngine circumvents this issue.

Issue #1842
@nitro404
Copy link
Contributor Author

Ahh, sorry! I've adjusted my environment and fixed all outstanding issues. Updated the function description and re-based onto master as well. Let me know if any other changes are needed, thank you!

@joeyparrish
Copy link
Member

Thanks! I've requested another test run through our build bot.

@shaka-bot
Copy link
Collaborator

All tests passed!

@joeyparrish joeyparrish merged commit 9c224c1 into shaka-project:master Mar 20, 2019
@joeyparrish
Copy link
Member

Thank you for your contribution!

kevinscroggins-youi pushed a commit to kevinscroggins-youi/shaka-player that referenced this pull request Apr 15, 2019
…ect#1845)

React Native introduces its own polyfill for window.URL.createObjectURL which is not compatible with Shaka. Encapsulating a reference to the original function inside of the MediaSourceEngine circumvents this issue.

Closes shaka-project#1842
kevinscroggins-youi pushed a commit to kevinscroggins-youi/shaka-player that referenced this pull request Apr 17, 2019
…ect#1845)

React Native introduces its own polyfill for window.URL.createObjectURL which is not compatible with Shaka. Encapsulating a reference to the original function inside of the MediaSourceEngine circumvents this issue.

Closes shaka-project#1842
kevinscroggins-youi pushed a commit to kevinscroggins-youi/shaka-player that referenced this pull request May 9, 2019
…ect#1845)

React Native introduces its own polyfill for window.URL.createObjectURL which is not compatible with Shaka. Encapsulating a reference to the original function inside of the MediaSourceEngine circumvents this issue.

Closes shaka-project#1842
kevinscroggins-youi pushed a commit to YOU-i-Labs/shaka-player that referenced this pull request May 9, 2019
…ect#1845)

React Native introduces its own polyfill for window.URL.createObjectURL which is not compatible with Shaka. Encapsulating a reference to the original function inside of the MediaSourceEngine circumvents this issue.

Closes shaka-project#1842
@nitro404 nitro404 deleted the react-native-support branch June 1, 2019 22:29
@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
status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants