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
Enable IE 11 compability of Angular + pdf.js #8192
Comments
Sounds like shifting blame from somewhat IE11-incomplete security check Angular logic to PDF.js polyfill... alright. Marking as a simple bug to fix this gap:
|
@yurydelendik For me this is analogous to the situation where a couple of Web APIs had to change their names because MooTools was applying partial polyfills and code on the Web started depending on it. Polyfilling is risky, IMO it should be done only with complete polyfills and only in final apps, not in libraries. If libraries need a particular API that's not available everywhere, they should wrap the native API in an utility function and use that function; that removes the whole class of possible bugs like this one. In short: library code shouldn't modify objects it doesn't own, e.g. native browser globals. |
@mgol oh, I agree. Entire PDF.js viewer application must reside in its own sandbox (I recommend iframe), but people keep using it within bigger apps. So we have to react accordingly. |
Let's take another example, without typed arrays and |
Not necessarily. You could keep your own internal set of variables that shadow globals. This may not cover all cases but at least promises should be fine. Something like: var Promise = window.Promise || PROMISE_POLYFILL; at the top of PDF.js. In that case you're not touching the global and you still can use This shouldn't affect performance in modern browsers either as it's a simple alias for them. I understand it may not be so easy in all the cases, though (e.g. if only a few methods need to be polyfilled) |
PDF.js code is transforming to use ES6 modules. The above approach can be problematic atm, unless a packager will automatically provide a polyfill. I would like not turn this issue into discussion about angular logic or pdf.js compatibility approach, it will be nice to fix a our polyfill so we will play nice with angular. A PR is welcome :) |
On the different thought, let's close this issue as won't fix. There is no confirmation that there are Angular+PDF.js+IE11 real world uses, if there is, the fix can be easy made on angular side by patching Angular.js. |
Hey there, I know this is an old topic but I'm running into the issue described above. I have a project where I'm using Angular 5 with PDF.js and I'm required to support IE11. I'm a newbie to Angular so I'm not exactly sure how I would "patch Angular.js" - can you give me some guidance as to how I can get around this defect? Thanks in advance. |
@elliotstoner this issue is about AngularJS compatibility, not Angular (2+) and only applies if you use |
@mgol Ah ok - I'll create a new issue then, thanks. |
Configuration:
Steps to reproduce the problem:
What is the expected behavior? (add screenshot)
What went wrong? (add screenshot)
Currently pdf.js defines document.currentScript but not link.origin nor link.protocol. If angular starts it checks if it is allowed to bootstrap automatically it checks for currentScript and asume that this will be enough to filter IE, meaning if currentScript is not defined we can automatically bootstrap. This check will not work in combination with pdf.js.
Libraries shouldn't modify properties of objects they don't own as it causes issues like this. If they need a particular API that is missing in some supported environments they may wrap its usage in a function that will use the shimmed version if the native one is not available.
Link to a viewer (if hosted on a site other than mozilla.github.io/pdf.js or as Firefox/Chrome extension):
http://plnkr.co/edit/YFCQM2Px0QU0KnGzsAlM?p=preview
The text was updated successfully, but these errors were encountered: