-
Notifications
You must be signed in to change notification settings - Fork 10.3k
[api-minor] Stop polyfilling structuredClone
in legacy builds
#17086
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
Conversation
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.
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/ec07744f28d6bce/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/6ce0d1567eb597b/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/6ce0d1567eb597b/output.txt Total script time: 25.04 mins
Image differences available at: http://54.241.84.105:8877/6ce0d1567eb597b/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/ec07744f28d6bce/output.txt Total script time: 37.32 mins
Image differences available at: http://54.193.163.58:8877/ec07744f28d6bce/reftest-analyzer.html#web=eq.log |
There was a problem hiding this 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!
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 thetransfer
handling to prevent bugs, and it'd be nice to avoid that.Note that
structuredClone
, with transfers, is only used in two spots: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.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.