ngx-extended-pdf-viewer
ngx-extended-pdf-viewer copied to clipboard
Remote server URLs don't work for `pdfDefaultOptions.assetsFolder`
Check this first
Sorry, but the nature of this problem means my answers to your three questions are:
- Probably not.
- Probably not.
- Um, I don't know how for this one...
Background info before I can describe the bug...
We are using your excellent PDF viewer in a multi-tenant Angular app (one app white-labelled and served separately for each of many customers). Without too much detail that's irrelevant to you, just know that our architecture causes our assets
path to be not relative to the site URL, but on a completely separate server.
To illustrate this, here are a couple of pretend URLs for a couple of sites using our app:
-
https://customer1.our-app.com
-
https://customer2.our-app.com
And a pretend assets URL shared by all sites:
-
https://library.our-app.com/assets
When we upgraded to your v13 (along with our upgrade to Angular v13), we could no longer load PDFs:
We determined that this was due to the fact that your code was now trying to load the new op-chaining-support
from a relative assets
URL, and it couldn't, since those aren't there. (The browser was trying to parse our app's error page as that JavaScript file, because of Angular default routing. THAT was confusing!)
Further research through your GitHub issue discussions taught us that we just needed to set pdfDefaultOptions.assetsFolder
, so we updated our code so that it basically has this outcome:
pdfDefaultOptions.assetsFolder = 'https://library.our-app.com/assets';
This fixed the op-chaining-support
problem.
However...
Describe the bug
We then got this message when trying to load:
I determined that we were trying to load the PDF worker via a URL like https://customer1.our-app.com/https://library.our-app.com/assets/pdf.worker-2.13.491.min.js
. (Note the double https
.)
I found this in your source code in pdf-default-options.ts
:
workerSrc: () =>
pdfDefaultOptions.needsES5
? `./${pdfDefaultOptions.assetsFolder}/pdf.worker-${getVersionSuffix(pdfDefaultOptions.assetsFolder)}-es5.js`
: `./${pdfDefaultOptions.assetsFolder}/pdf.worker-${getVersionSuffix(pdfDefaultOptions.assetsFolder)}.js`,
(Ditto for other options -- cMapUrl
, sandboxBundleSrc
, standardFontDataUrl
.)
You've made the very reasonable assumption that assets are always served via a relative URL, but this doesn't work for our particular use case.
Workaround
You've given us the ability to override these other options, so that's what I'm doing. I don't want to recalculate everything, though, so I'm doing (something like) this:
pdfDefaultOptions.assetsFolder = 'https://library.our-app.com/assets';
const newWorkerSrcReturnValue: string = pdfDefaultOptions.workerSrc().replace(/^\.\//, '');
pdfDefaultOptions.workerSrc = () => newWorkerSrcReturnValue;
// Ditto for the other options that depend on pdfDefaultOptions.assetsFolder.
And this seems to work.
Proposed solution
Make your default options functions account for a possible full remote URL in pdfDefaultOptions.assetsFolder
, and leave off the ./
in that case?
But that's up to you... Not sure if our use case is truly a one-off or not.
If you decide to just call this a "won't fix", I don't blame you. But at least this is documented in your issues, in case anyone else out there happens to have a similar setup. And our workaround should be fine for us (and future them, perhaps).
But if you do decide to support this weird use case, we'll be able to delete a bit of workaround code in the future. 🙂
And a question:
Do you know of any other complications that my workaround won't prevent?
Version info
- v13.0.0
Desktop (please complete the following information):
- Probably irrelevant, but mostly Chrome
Smartphone (please complete the following information):
- Nothing specific
To Reproduce
-
Can you provide a reproducer?
- Sorry, I don't think so.
-
If you really can't provide me a reproducer, please tell me how to reproduce the behavior:
- Hopefully the description above is sufficient.
Thank you!
- No... thank YOU! You've got a great PDF viewer here -- and Angular ready! 😍
@Snuffleupagus @calixteman @timvandermeij I'm receiving a lot of positive feedback for wrapping your work into a decent Angular library. It's time to pass the kudos to you, too. Thanks a lot for creating such a great Firefox PDF viewer and even allowing other people to build upon your work! You guys rock!
(And before you ask: please don't try to answer this particular GitHub issue - it's specific to my Angular wrapper. I just wanted to share some of the "thank you"s I get with you).
@etramell-tem To be honest, I didn't spend much thought on your scenario yet because it almost always results in a CORS issues, and most people have a hard time solving it. Plus, loading resources from a CDN is more or less illegal if you're in Europe (because of the GDPR), so I tend to discourage it.
Nonetheless, I agree it's annoying it doesn't work. Your description sounds like a corporate CDN, and there's nothing wrong with that.
I'm pretty sure your workaround works well. At the moment, I'm not sure if I can solve the root cause of the problem, or if I prefer to simply document it. Relative paths in Angular applications are always a pain in the - well, you know where. There's always a corner case I didn't think of!
Version 15.0.0-alpha.1 fixes the bug. Please run a test to confirm this.
BTW, thanks a lot for your detailed error description!
Best regards, Stephan
@stephanrauh - Thank you for the fixes! Sorry for the delayed response -- I was on vacation last week.
I'll (temporarily) try out your alpha version with our workarounds turned off and make sure I don't see any further issues.
Vacations are important, too! :) You might be interested to learn there's still something wrong with the paths. But if I'm not mistaken, it's been wrong for a very long time, and it "only" affects East-Asian fonts. ("Only" meaning I considering the bug very annoying and embarrassing, but the majority of PDF files - even including languages like Japanese, Korean, and Chinese - render without problems). Here's the pointer to the ticket: #1485
@stephanrauh - Well, I couldn't quite test it all the way. I can test only locally (because I can't push your alpha version up to our "real" servers, and the version of pdf.js you try to load from this newer version doesn't exist on the remote assets server, etc.).
But I can run your alpha version locally, comment out our workarounds, and see that the format of the pdf.js URL looks correct. (No more failure for a malformed URL -- just a failure for a non-existent JS file, as expected.)
That'll have to do it for now. We'll run fine with the workarounds for a while. We're probably weeks away from our next dependency upgrade cycle (we try to stay a quarter behind Angular's latest version, and we're trying to get to v14 soon). I don't know when your v15 will be fully released or compatible with whichever version of Angular, but whenever everything syncs, we can try it for real without the workarounds (with only the assets URL redefined), and then I can finally let you know whether we're good for sure or not. (Reality is so annoying sometimes!)
(I also had a look at the commits for your code changes linked herein, and they seem logical enough to me, for whatever it's worth, with my 30 minutes or so of experience with your codebase! 🙂)
Also, thanks for the heads up on the Asian fonts. I don't think our usage extends there quite yet, but good to know in case it does! (And I know what you mean... I consider ALL my bugs to be annoying and embarrassing. 😆)
Thanks a lot, Ed! As for the release planning: at the moment, I'm both a bit lazy and busy doing other stuff, so version 15.0.0 probably get a few weeks to get mature. A major version update is always a good opportunity to add new features, so maybe I'll add an option to enable the brand-new (und unfinished) PDF editor. I haven't seen it myself yet, apart from a very early stage, so I won't promise anything. But if I unlock it, I'm going to play around with the editor for a couple of evenings.
I'm definitely going to support Angular 15 (unless something unexpected happens), but that version is probably still months away. My best guess is it's going to be release in November - but don't take my work for it, it's just an educated guess. By the time Angular 15 lands, ngx-extended-pdf-viewer is probably at version 18. :) That's the drawback of semantic versioning - every breaking change, no matter how small or insignificant, forces me to increase the major version number, even if I don't add new features!
As for the embarrassing bugs: yeah, you're right, there are very few bugs I'm proud of. :) However, I wonder if the bug has something to do with the falling download figures. They've plateaued at 40.000 per week, which is just incredible, but with a slight tendency southwards. The other two Angular PDF viewer show similar trends, so maybe it's a general development. However, me accidentally frustrating developers in the far East is a good explanation, too.

(Source: https://npm-stat.com/charts.html?package=ng2-pdf-viewer&package=ngx-extended-pdf-viewer&package=ng2-pdfjs-viewer&from=2019-01-01&to=2022-08-15)
It's really hard (for me, anyway) to attribute any specific cause to these kind of trends. The shape of a similar plot for @angular/core
has a similar shape to yours (although at a larger scale 😉).
Less Angular downloading means less need for Angular PDF viewers? Just summer vacation time? Corporate layoffs? Total coincidence? 🤷
I have the same scenario. Thanks @etramell-tem for an incredibly well-documented issue, and @stephanrauh for a phenomenal library!
@Jadamae77 First of all, thanks for all the kind words! You've said you've got the same scenario. Does the latest alpha version of my library solve your issue? Or are you still suffering from the bug?
Yes, the latest version fixes the issue! I wasn't sure on Friday, but we have the files uploaded to our internal CDN and it all works as expected. Thanks again!
@stephanrauh - Hello again! Sorry to have to bother you again, but I'm working on our Angular v14 upgrade this week. I've updated to your v15.0.5, and I think there's still a residual bug or two with this absolute path stuff.
As we discussed before, I can't see what your updates will do until after we deploy them. So I did some sanity checking to see if it was safe to remove our workaround code.
I put this temporary code in:
pdfDefaultOptions.assetsFolder = 'https://some.url';
console.log(pdfDefaultOptions.cMapUrl());
console.log(pdfDefaultOptions.sandboxBundleSrc());
console.log(pdfDefaultOptions.workerSrc());
console.log(pdfDefaultOptions.standardFontDataUrl());
and I was surprised that I didn't get what I was expecting.
So I went back to your v13, and to the alpha version you had me look at a couple of months ago, and these were the results:
v13.0.0 (before your fix):
./https://some.url/cmaps/
./https://some.url/pdf.sandbox-2.13.491.js
./https://some.url/pdf.worker-2.13.491.js
../https://some.url/standard_fonts/
v15.0.0-alpha.1 (just after your fix):
https://some.url/cmaps/
https://some.url/pdf.sandbox-2.15.671.js
https://some.url/pdf.worker-2.15.671.js
../https://some.url/standard_fonts/
v15.0.5 (latest):
../https://some.url/cmaps/
https://some.url/pdf.sandbox-2.16.446.js
https://some.url/pdf.worker-2.16.446.js
../https://some.url/standard_fonts/
-
standard_fonts
makes sense, given how it was changed in this commit. (I guess I didn't look at that one in my quick test in August.) -
cmaps
looks like it was right (for the absolute case) in that commit (which is confirmed by the alpha version output above), but apparently you had to change it again later for some reason.
Still no urgency at all on this end... just thought I'd give you an update, and I'll simply keep our workaround code.
If you happen to publish a patch in the next few days, great -- I'll be working on this upgrade for a while yet because I still have other dependencies to update. (Logically enough, I'm in the N's at the moment. 😉)
Otherwise, I can try this check again at some later date, perhaps in our Angular v15 upgrade in six months or so.
And if you'd prefer a new bug report, please let me know. At first, I couldn't decide whether to do that or just comment here. I ended up erring on the side of proximity to the original info.
Thanks! Ed
Wait, what? Now that's really unexpected. But don't worry about bothering me - quite the contrary, I feel honored. Knowing that people care enough about my library to report bugs, even adding a lot of insight - that's just awesome!
I'm afraid I'll have to postpone work on your bug until the end of October. In the meantime, I'm busy doing other things. If you don't hear from me at the end of this month, please nudge me.
No worries -- we're scheduled for our Angular v15 upgrade sometime in Q1 2023 (which might drift into Q2 a bit), so we probably won't be ready to try to remove the workarounds again until then. Thanks!
@stephanrauh - Unfortunately, I've lost my job due to a restructuring layoff (as so many developers have lately). We hadn't yet gotten around to our product's Angular v15 and other scheduled Q1 dependency upgrades.
I'll forward this URL to one of my former teammates so that they'll know to take a look at your status whenever they get around to the next batch of upgrades.
I enjoyed our brief consultation on this issue. If I'm lucky enough to work on another Angular product, and we have use for a PDF viewer, I'll once again be a grateful user of your work.
Best regards, Ed
Oh no! I'm sorry to hear that! My company is looking for new developers, but I'm afraid you don't live in Germany, and employing people abroad is probably too difficult for us (and for you). However, if you want to give it a try, just ping me. Of course, I can't promise anything beyond giving you the opportunity. But that's better than nothing!
In any case, I'm sure you'll find a new job you'll love soon!
Good luck Stephan
Finally, the bugfix has landed with version 17.4.1.