Skip to content

[api-minor] Remove support for browsers/environments without fully working URL.createObjectURL implementations #14515

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

Merged

Conversation

Snuffleupagus
Copy link
Collaborator

This disableCreateObjectURL option was originally introduced all the way back in PR #4103 (eight years ago), in order to work-around URL.createObjectURL()-bugs specific to Internet Explorer.
In PR #8081 (five years ago) the disableCreateObjectURL option was extended to cover Google Chrome on iOS-devices as well, since that configuration apparently also suffered from URL.createObjectURL()-bugs.[1]

At this point in time, I thus think that it makes sense to re-evaluate if we should still keep the disableCreateObjectURL option.

  • For Internet Explorer, support was explicitly removed in PDF.js version 2.7.570 which was released one year ago and all IE-specific compatibility code (and polyfills) have since been removed.

  • For Google Chrome on iOS-devices, while we still "support" such configurations, it's not the focus of any development and platform-specific bugs are thus often closed as WONTFIX.

Note here that at this point in time, the disableCreateObjectURL option is only being used in the viewer and any URL.createObjectURL()-bugs browser/platform bugs will thus not affect the main PDF.js library. Furthermore, given where the disableCreateObjectURL option is being used in the viewer the basic functionality should also remain unaffected by these changes.[2]
Furthermore, it's also possible that the URL.createObjectURL()-bugs have been fixed in browser itself since PR #8081 was submitted.[3]

Obviously you could argue that this isn't a lot of code, w.r.t. number of lines, and you'd be technically correct. However, it does add additional complexity in a few different viewer components which thus add overhead when reading and working with this code.
Finally, assuming the URL.createObjectURL()-bugs are still present in Google Chrome on iOS-devices, I think that we should ask ourselves if it's reasonable for the PDF.js project (and its contributors) to keep attempting to support a configuration if the browser developers still haven't fixed these kind of bugs!?


[1] According to https://groups.google.com/a/chromium.org/forum/#!topic/chromium-html5/RKQ0ZJIj7c4, which is linked in PR #8081, that bug was mentioned/reported as early as the 2014 (eight years ago).

[2] Viewer functionality such as e.g. downloading and printing may be affected.

[3] I don't have access to any iOS-devices to test with.

…rking `URL.createObjectURL` implementations

This `disableCreateObjectURL` option was originally introduced all the way back in PR 4103 (eight years ago), in order to work-around `URL.createObjectURL()`-bugs specific to Internet Explorer.
In PR 8081 (five years ago) the `disableCreateObjectURL` option was extended to cover Google Chrome on iOS-devices as well, since that configuration apparently also suffered from `URL.createObjectURL()`-bugs.[1]

At this point in time, I thus think that it makes sense to re-evaluate if we should still keep the `disableCreateObjectURL` option.

 - For Internet Explorer, support was explicitly removed in PDF.js version `2.7.570` which was released one year ago and all IE-specific compatibility code (and polyfills) have since been removed.

 - For Google Chrome on iOS-devices, while we still "support" such configurations, it's *not* the focus of any development and platform-specific bugs are thus often closed as WONTFIX.

Note here that at this point in time, the `disableCreateObjectURL` option is *only* being used in the viewer and any `URL.createObjectURL()`-bugs browser/platform bugs will thus not affect the main PDF.js library. Furthermore, given where the `disableCreateObjectURL` option is being used in the viewer the basic functionality should also remain unaffected by these changes.[2]
Furthermore, it's also possible that the `URL.createObjectURL()`-bugs have been fixed in *browser* itself since PR 8081 was submitted.[3]

Obviously you could argue that this isn't a lot of code, w.r.t. number of lines, and you'd be technically correct. However, it does add additional complexity in a few different viewer components which thus add overhead when reading and working with this code.
Finally, assuming the `URL.createObjectURL()`-bugs are still present in Google Chrome on iOS-devices, I think that we should ask ourselves if it's reasonable for the PDF.js project (and its contributors) to keep attempting to support a configuration if the *browser* developers still haven't fixed these kind of bugs!?

---
[1] According to https://groups.google.com/a/chromium.org/forum/#!topic/chromium-html5/RKQ0ZJIj7c4, which is linked in PR 8081, that bug was mentioned/reported as early as the 2014 (eight years ago).

[2] Viewer functionality such as e.g. downloading and printing may be affected.

[3] I don't have access to any iOS-devices to test with.
@Snuffleupagus
Copy link
Collaborator Author

/botio integrationtest

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

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

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/be033965c211c72/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/be033965c211c72/output.txt

Total script time: 4.95 mins

  • Integration Tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

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

Total script time: 7.90 mins

  • Integration Tests: Passed

@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

pdfjsbot commented Feb 2, 2022

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/586919f4036fb61/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 2, 2022

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/586919f4036fb61/output.txt

Total script time: 4.31 mins

Published

@timvandermeij timvandermeij merged commit 48c8831 into mozilla:master Feb 2, 2022
@timvandermeij
Copy link
Contributor

I agree with the reasons to do this; thanks!

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