-
Notifications
You must be signed in to change notification settings - Fork 19
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
Issues in Firefox PDF container #4
Comments
Yeah, I've also noticed the So, we have to catch the actions on the page (back, forward) and send them to the iFrame. Not 100% sure how to catch them, though. The What do you think ? |
I'm not sure if web extension API supports those operations, but I think it may be worth investigating. I'll see if I can try implement it in my free time. 🙂 |
Gosh, this is really difficult ! I tried to look up how to include it from CDN, but it seems that then we can only refer to files from the same domain: https://github.com/mozilla/pdf.js/wiki/Viewer-options#options-after-the--1 I tried to gain access to the iframe's Do you know of a more flexible CSP ? Or, if we do include it from CDN, I don't get why we are restricted to Finally, it remains to include it directly in the extension. But then we have to keep upgrading the extension every time Mozilla upgrades PDFjs, which I think is a No-Go :(. Thanks for your insight ! P.S. Nice work in RL ! Edit: It turns out that loading from a CDN actually works with arXiv documents. I have modified the extension to load the pdfJS source from CDN, and to render the article from arXiv. The downside is that if we want the full power of native The developer do not encourage re-using the same viewer as the original: mozilla/pdf.js#13928 What do you think we should do next ? |
Thanks for spending time investigating this issue! Similar to what you have mentioned, the developers have discouraged the reuse of the viewer to avoid confusion in bug reporting. (mozilla/pdf.js#8513) However, I prefer using the same viewer as the original one, since the buttons and sidebars are important features. We can still respect the developers' suggestions by slightly modifying the top bar and adding some text, such as |
Fixed in 5a624f6. The fix will be included in the v1.7.2 release. |
Super nice to see this integrated ! Thanks a bunch for doing it, and doing it so simply ! (and for the credit too 😳) I tested it yesterday and today, and it mostly works. However, I have come across a use case where it does not work: https://arxiv.org/abs/2212.14052 . It seems that the PDF link does not contain the Since the link is directly appended to the |
Thanks for testing this experimental feature and investigating the issue! (This experimental feature is made possible thanks to your in-depth investigation and practical solutions in #13) I just confirmed the issue: Clicking the extension action button on the abstract page works well. However, clicking the I don't think the 302 redirect issue is a Adding a trailing This issue should be fixed in e30fb1f. I haven't publish the fix to the firefox store yet, you can try it out by cloning the master branch. Please tell me if there are any further issues. Thank you! |
Thank you for the quick resolution ! Yes, the master branch seems to work on the papers that I have open. I haven't done in-depth tests. Thanks! |
Go Back
button to navigate back to the link's position.(Issues reported by Rick Tu)
The text was updated successfully, but these errors were encountered: