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

[api-minor] Stop polyfilling structuredClone in legacy builds #17086

Merged

Conversation

Snuffleupagus
Copy link
Collaborator

Comparing the currently supported browsers/environments, see the FAQ and the MDN compatibility data, the structuredClone polyfill is only needed in Google Chrome versions < 98. Because of some limitations in the core-js polyfill we're currently forced to special-case the transfer handling to prevent bugs, and it'd be nice to avoid that.

Note that structuredClone, with transfers, is only used in two spots:

  • The LoopbackPort class, which is only used with fake workers. Given that fake workers should never be used in browsers, breaking that edge-case in older Google Chrome versions seem fine.
  • The AnnotationStorage class, when Stamp-annotations have been added to the document. Given that Google Chrome isn't the main focus of development, breaking part of the editing-functionality in older Google Chrome versions should hopefully be acceptable.

Comparing the currently supported browsers/environments, see [the FAQ](https://github.com/mozilla/pdf.js/wiki/Frequently-Asked-Questions#faq-support) and the [MDN compatibility data](https://developer.mozilla.org/en-US/docs/Web/API/structuredClone#browser_compatibility), the `structuredClone` polyfill is *only* needed in Google Chrome versions < 98. Because of some limitations in the core-js polyfill we're currently forced to special-case the `transfer` handling to prevent bugs, and it'd be nice to avoid that.

Note that `structuredClone`, with transfers, is only used in two spots:
 - The `LoopbackPort` class, which is only used with fake workers. Given that fake workers should *never* be used in browsers, breaking that edge-case in older Google Chrome versions seem fine.
 - The `AnnotationStorage` class, when Stamp-annotations have been added to the document. Given that Google Chrome isn't the main focus of development, breaking *part* of the editing-functionality in older Google Chrome versions should hopefully be acceptable.
@Snuffleupagus
Copy link
Collaborator Author

/botio test

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/ec07744f28d6bce/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/6ce0d1567eb597b/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/6ce0d1567eb597b/output.txt

Total script time: 25.04 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED
  different ref/snapshot: 18
  different first/second rendering: 1

Image differences available at: http://54.241.84.105:8877/6ce0d1567eb597b/reftest-analyzer.html#web=eq.log

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/ec07744f28d6bce/output.txt

Total script time: 37.32 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED
  different ref/snapshot: 26

Image differences available at: http://54.193.163.58:8877/ec07744f28d6bce/reftest-analyzer.html#web=eq.log

Copy link
Contributor

@timvandermeij timvandermeij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, also because Chrome 98 is from February 2022 which is over 1.5 years old and using such an old browser is unsafe anyway. Thanks!

@timvandermeij timvandermeij merged commit f2c9b64 into mozilla:master Oct 7, 2023
3 checks passed
@Snuffleupagus Snuffleupagus deleted the rm-structuredClone-polyfill branch October 7, 2023 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants