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

Investigate root cause of overwritten MCap polyfill on Safari #3696

Closed
joeyparrish opened this issue Oct 12, 2021 · 4 comments · Fixed by #4009
Closed

Investigate root cause of overwritten MCap polyfill on Safari #3696

joeyparrish opened this issue Oct 12, 2021 · 4 comments · Fixed by #4009
Assignees
Labels
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
Milestone

Comments

@joeyparrish
Copy link
Member

In #3530, the MCap polyfill is being overwritten on Safari. It's not clear what is causing this. @joeyparrish could not reproduce the issue, but @theodab could.

PR #3668 gave us a workaround, but we still haven't addressed the root cause. It would be preferable to find the root cause of the issue, revert the PR, and merge a fix that addresses the root cause.

@joeyparrish joeyparrish added type: code health A code health issue priority: P3 Useful but not urgent labels Oct 12, 2021
@shaka-bot shaka-bot added this to the Backlog milestone Oct 12, 2021
joeyparrish pushed a commit that referenced this issue Oct 12, 2021
…sers (#3668)

See also #3696

Closes #3530

Change-Id: Ied2e644f8a5d170ef70386dc2a39b51fc95a691f
@joeyparrish joeyparrish added priority: P1 Big impact or workaround impractical; resolve before feature release type: bug Something isn't working correctly and removed type: code health A code health issue priority: P3 Useful but not urgent labels Jan 10, 2022
@joeyparrish joeyparrish self-assigned this Jan 10, 2022
@joeyparrish
Copy link
Member Author

I'm able to reproduce the original issue on Safari 15.1. Previously, I was unable to reproduce it in Safari 14.1.

@joeyparrish
Copy link
Member Author

The issue is timing-related. With breakpoints enabled in the polyfill installation, the problem vanishes.

@joeyparrish
Copy link
Member Author

Delaying the polyfill by 5 seconds seems to work around the issue. With that, content shared with us privately via #3530 gets past the original 6010 error (and instead fails a license request).

@joeyparrish
Copy link
Member Author

I was trying to determine if/when/how the property for decodingInfo was being overwritten, and I may have stumbled onto a good clue.

I first changed the polyfill to:

  1. Set decodingInfo
  2. Use Object.defineProperty to make the property non-writeable
  3. Check 5 seconds later if the value was still what I set it to

After 5 seconds, the value had been reset every time (presumably by the platform). But I couldn't be sure if it was decodingInfo or the mediaCapabilities parent of decodingInfo which was being reset.

So then I changed the polyfill to:

  1. Set decodingInfo
  2. Make a local copy of the value of mediaCapabilities
  3. Use Object.defineProperty as before
  4. Check both the values of decodingInfo and mediaCapabilities after 5 seconds

And magically... both were the same now! And consistently when reloading the page to try again.

So here's my theory. It may be that navigator.mediaCapabilities is a getter which returns a new MediaCapabilities object on-demand, which is later garbage-collected when not referenced. Creating a local copy that is used later in a closure seems to keep our modifications in the polyfill from being undone.

I'll experiment with this more and see if this could be a viable fix.

@joeyparrish joeyparrish modified the milestones: Backlog, v3.3 Jan 11, 2022
joeyparrish added a commit that referenced this issue Jan 11, 2022
Fixes #3696
Fixes #3530

Change-Id: I2f2e248c9001e10013eb4b03af6b9ef49f28dc6c
joeyparrish added a commit that referenced this issue Jan 11, 2022
Fixes #3696
Fixes #3530

Change-Id: I2f2e248c9001e10013eb4b03af6b9ef49f28dc6c
joeyparrish added a commit that referenced this issue Jan 28, 2022
Fixes #3696
Fixes #3530

Change-Id: I2f2e248c9001e10013eb4b03af6b9ef49f28dc6c
tecteun pushed a commit to tecteun/shaka-player that referenced this issue Feb 10, 2022
tecteun pushed a commit to tecteun/shaka-player that referenced this issue Feb 10, 2022
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Mar 12, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 12, 2022
@avelad avelad modified the milestones: v3.3, v4.0 May 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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
3 participants