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

Change the minimum "supported" version of the Safari-browser to Safari 10 #12737

Merged
merged 1 commit into from
Dec 15, 2020

Conversation

Snuffleupagus
Copy link
Collaborator

According to https://github.com/mozilla/pdf.js/wiki/Frequently-Asked-Questions#faq-support, Safari 9 is still listed as "mostly supported".

Given that the last release from the Safari 9 branch was on September 1, 2016, it's questionable at least to me if it actually makes sense for us to even pretend to "support" such an old browser.
Especially when the first release from the Safari 10 branch was on September 20, 2016, which is now over four years ago.

Based on the MDN compatibility data, this patch thus removes the following polyfills:

@timvandermeij
Copy link
Contributor

I fully agree with this, and anything higher than Safari 9 (which is way too old in my opinion as well) seems reasonable to me. Let's indeed stick with >= 10 for now, and we can always increase the number later on if the need arises, but this seems like a small change that will give us some clean-up.

…i 10

According to https://github.com/mozilla/pdf.js/wiki/Frequently-Asked-Questions#faq-support, Safari 9 is still listed as "mostly supported".

Given that the *last* release from the Safari 9 branch was on [September 1, 2016](https://en.wikipedia.org/wiki/Safari_version_history#Safari_9), it's questionable at least to me if it actually makes sense for us to even pretend to "support" such an old browser.
Especially when the *first* release from the Safari 10 branch was on [September 20, 2016](https://en.wikipedia.org/wiki/Safari_version_history#Safari_10), which is now over four years ago.

Based on the MDN compatibility data, this patch thus removes the following polyfills:
 - `TypedArray.prototype.slice()`, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/slice#Browser_compatibility
 - `String.prototype.codePointAt()`, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/codePointAt#Browser_compatibility
 - `String.fromCodePoint()`, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/fromCodePoint#Browser_compatibility
@Snuffleupagus Snuffleupagus marked this pull request as ready for review December 15, 2020 09:00
@Snuffleupagus
Copy link
Collaborator Author

Snuffleupagus commented Dec 15, 2020

Something else, which I just spotted the other day, is that the update to Webpack 5 changed some default values. Note how there's now an Arrow function being used as part of the Webpack-header even in our es5-files, see e.g.

return /******/ (() => { // webpackBootstrap

This can obviously be fixed by tweaking some configuration options, however looking at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions#Browser_compatibility I'm not convinced that we need to do that if we accept this patch since all browsers that we support should then have native Arrow function support.

@Snuffleupagus
Copy link
Collaborator Author

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/983a5d04b6bd381/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/983a5d04b6bd381/output.txt

Total script time: 4.17 mins

Published

@timvandermeij timvandermeij merged commit 0655f50 into mozilla:master Dec 15, 2020
@timvandermeij
Copy link
Contributor

Looks good to me. I'll update the FAQ page.

@Snuffleupagus Snuffleupagus deleted the Safari-10 branch December 16, 2020 10:32
@Snuffleupagus
Copy link
Collaborator Author

I'll update the FAQ page.

Thanks for doing this!

Looking more closely on that list, I do wonder if we actually need to keep the "Safari 9 and below"-line since it honestly seems redundant now; I'd suggest just removing it.
Also, the "Android 4 and below"-line seems useless as well given that e.g. Android 5 is way too old to be supported; hence I'd suggest just removing this one too.

@timvandermeij
Copy link
Contributor

I agree; it's removed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants