PDF-friendly book home button in the viewer #940
Conversation
|
I'm sorry but it's becoming a pain to test. I can't compile on macOS and neither can I on a |
IMO, this should better be fixed. URL should match content. |
|
@mgautierfr What has to be fixed (which ticket are we talking about) so @rgaudin can compile properly? |
|
This is kiwix/kiwix-build#599 |
|
@rgaudin You should be able to compile/test easily now. Let us know. |
|
No, I can compile on Linux now but I still can't on macOS due to the gtest issue. |
Under Firefox sandboxing of the viewer content and cross-origin restrictions (PDF-viewer is not considered same origin) prevent the viewer JS code from finding out the URL of the PDF. A partial workaround is to intercept click events for links and use that information instead but that won't work if the PDF is loaded not as a result of the user clicking on a direct link (e.g. via JS). |
|
@veloman-yunkan I'm not sure about the current status of this PR. It seems to be fixed but the URL is wrong. Do you confirm? |
|
My view is that if this PR fixes the most glaring aspect of the issue minus the URL problem, then at least you've increased usability, even if it isn't perfect (yet). Merging this to the dev server would allow testing for those of us who can't compile it. Having said that, in Kiwix JS we no longer even attempt to open PDFs in the iframe as they are considered cross-origin and therefore not only a (slight) security risk, but also are blocked by some browsers with a sandboxed iframe. We simply have to treat them as external content, like https:// URLs, OpenOffice/Word documents, and epubs. |
|
@Jaifroid thx, so if i understand right, the most obvious solutions to the url problem are either:
|
|
If I remember rightly (I'm getting confused!), the solution adopted here for showing PDFs in the iframe was not to serve sandbox headers when loading a PDF. It's probably best not to go back on that PR, or we'll really be going round in circles.... (The problem here relates to the fact that some browsers consider PDFs to be cross-origin.) Which leaves "live with it", unless someone can see a better solution. Maybe things will become clearer once this and #946 (in whatever form) are merged, and can be thoroughly tested before deployment. |
There's one partial solution that @veloman-yunkan indicated that would work for links to PDFs. I think it's worth implementing. But I'd advise we merge this and open a ticket to fix it separately (or find another solution) as @Jaifroid suggests.
Not sure we can afford that. We have thousands of Android devices with no PDF external reader accessing PDF files in the field. Maybe we should test on said devices if their browser includes PDF rendering and if it gets triggered and used the @Jaifroid way. |
Ooh, does Kiwix Serve run on Android, or does the Android app depend on it in some way? If so, it's an important consideration. My phone asks me which app I want to use to open a PDF when a Web site offers one to me: would this be any different? |
|
I am referring to Kiwix Hotspot setups composed of RaspberryPis running kiwix-serve accessed by old Android Tablets without PDF readers |
|
OK, I'm all for taking into account support for old devices! From a quick look, |
veloman-yunkan
force-pushed
the
fix_for_issue912
branch
from
33286e8
to
dd1f60a
Compare
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #940 +/- ##
=======================================
Coverage 38.88% 38.88%
=======================================
Files 56 56
Lines 3973 3973
Branches 2186 2186
=======================================
Hits 1545 1545
Misses 1097 1097
Partials 1331 1331 ☔ View full report in Codecov by Sentry. |
Yes
Done |
|
@veloman-yunkan Can you please fix the last details so we can move forward? |
In firefox, when PDF content is displayed in the viewer, changing the viewer URL in the address bar had no effect. The most prominent manifestation of this bug was the broken book home button but the same issue was present even if the fragment component of the viewer URL was edited manually. The bug was a result of 1. an optimization preventing any actions if the new content URL is the same as the old content URL (this was needed to break the infinite loop of mutual updates of the top-window and content window/iframe URLs when any one of them changes). 2. sandboxing of the iframe and inability to access the content URL in iframe because of cross-origin restrictions when the content is a PDF displayed by the builtin viewer. Now that issue is fixed. A slight remaining defect is that the addressbar URL is still not updated when a PDF file is loaded/displayed in the viewer.
veloman-yunkan
force-pushed
the
fix_for_issue912
branch
from
dd1f60a
to
c1ad65d
Compare
Fixes #912
In firefox, when PDF content is displayed in the viewer, changing the viewer URL in the address bar had no effect. The most prominent manifestation of this bug was the broken book home button but the same issue was present even if the fragment component of the viewer URL was edited manually. The bug was a result of
an optimization preventing any actions if the new content URL is the same as the old content URL (this was needed to break the infinite loop of mutual updates of the top-window and content window/iframe URLs when any one of them changes).
sandboxing of the iframe and inability to access the content URL in iframe because of cross-origin restrictions when the content is a PDF displayed by the builtin viewer.
Now that issue is fixed. A slight remaining defect is that the addressbar URL is still not updated when a PDF file is loaded/displayed in the viewer.