Configuration:
Steps to reproduce the problem:
What is the expected behavior? (add screenshot)
What went wrong? (add screenshot)
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
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:
document.location.origin
must be set to new URL(document.location.href).origin if 'origin' property is absentHTMLScriptElement.prototype
shall have the origin and protocol getters with the similar logic as above.@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.
Polyfilling is risky
@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.
library code shouldn't modify objects it doesn't own, e.g. native browser globals.
Let's take another example, without typed arrays and Promise
PDF.js library would not be the same. And by not modifying global objects we would make our code unreadable and probably less performant for majority of the modern browsers.
Let's take another example, without typed arrays and Promise PDF.js library would not be the same. And by not modifying global objects we would make our code unreadable and probably less performant for majority of the modern browsers.
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 Promise
in your code without any changes.
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 ng-app
instead of manual bootstrapping. Angular 2+ doesn't even support automatic bootstrapping so this issue won't apply there.
@mgol Ah ok - I'll create a new issue then, thanks.