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

Add a polyfill for classList.{add, remove} with more than one parameter #10527

Merged
merged 1 commit into from
Feb 8, 2019

Conversation

Snuffleupagus
Copy link
Collaborator

Unsurprisingly IE11 doesn't support this, so a polyfill is needed since otherwise the sidebar can no longer be opened.

Also, simplifies the existing classList.toggle polyfill.

…eter

Unsurprisingly IE11 doesn't support this, so a polyfill is needed since otherwise the sidebar can no longer be opened.

Also, simplifies the existing `classList.toggle` polyfill.
@kristopher776
Copy link

Thnx

@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

pdfjsbot commented Feb 8, 2019

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/c8cf1978d6b7883/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 8, 2019

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/c8cf1978d6b7883/output.txt

Total script time: 1.68 mins

Published

@timvandermeij timvandermeij merged commit 2b6e636 into mozilla:master Feb 8, 2019
@timvandermeij
Copy link
Contributor

IE11 keeps on being a problem. Hopefully we can drop support for it in a new major version. For now, thanks for patching this!

@Snuffleupagus Snuffleupagus deleted the classList-add-remove branch February 8, 2019 21:57
@Snuffleupagus
Copy link
Collaborator Author

Snuffleupagus commented Apr 3, 2019

IE11 keeps on being a problem. Hopefully we can drop support for it in a new major version.

Lately I've been wondering if it would be possible to change to wording in the FAQ, to more clearly outline the "actual" current state of IE support.

There's essentially a few different tiers of problems with IE, as far as I see it:

  1. Features that cannot be ever be fixed or polyfilled correctly, no matter how/where the code is placed; one example that comes to mind are Blend modes.
  2. Features that could perhaps theoretically be supported, but would require really ugly hacks not limited to compatibility.js and would generally result in code which is an unreadable/unmaintainable mess; one such example could be the sidebar resizing in the default viewer (where feature detection is used instead).
  3. General slowness of the browser itself, which obviously cannot be fixed here.
  4. Various edge-case issues, maybe related to specific platforms/drivers/whatever, affecting an old and no longer officially supported browser.

Looking back through the history of PDF.js, the last couple of years have essentially only seen IE specific changes being made in compatibility.js (by adding polyfills such that the library/viewer at least runs without breaking errors).
Furthermore, the likelihood of any IE specific changes being accepted anywhere in the code-base except for compatibility.js has always been extremely low, at this point in time that probability is most likely zero.

Hence we're currently in a situation where most (or all?) reported IE-specific issues are either impossible to fix, or could only be fixed partially, and/or a solution would be rejected for adding unwanted complexity.
Also, any time a contributor wastes hacking on IE-specific issues, is time that could have been used to improve general aspects of the library itself (it's also very rare that those affected by e.g. a missing polyfill actually provides a patch themselves).
The end result is that there's a fair number of issues left (perpetually) open, that either cannot or will not ever be fixed, which could very well give users false hope that a solution will eventually be provided.

To summarize: In preparation for a future PDF.js version that explicitly removes IE-support, I'm proposing that the FAQ is updated to more clearly agree with the reality of the support status (which should also save time for contributors). For example, update https://github.com/mozilla/pdf.js/wiki/Frequently-Asked-Questions#faq-support with, something along these lines:

While the library, and the default viewer, should still work in IE 11 for the time being, some functionality/features may not be available and the performance will be worse compared to modern browsers.
Going forward, only bugs which *completely* prevents the library, and/or the default viewer, from running will be accepted.

Obviously this could be seen as changing the support status "out of the blue", but looking at the commit history it would merely put into writing what's been happening for the past few years.
This would also allow closing most (all?) issues tagged with https://github.com/mozilla/pdf.js/labels/4-ie11-specific


Somewhat related here is IE Edge, which is now in the process of becoming a Chromium browser. Given these changes it's questionable how many of the issues in https://github.com/mozilla/pdf.js/labels/4-ie-edge-specific will still be relevant, and I cannot see why PDF.js contributors should spend time supporting a browser version when the vendor has essentially given up on it; hence I'd suggest treating IE Edge the same as above.

@timvandermeij
Copy link
Contributor

I have updated the FAQ section to describe the support status. I agree with the above and it's essentially what we've been doing for a long time now.

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.

4 participants