Really enable Chromium to display PDFs in the viewer iframe #930
Conversation
The previous "fix" (merged PR #924) was buggy. It not only didn't work in Chromium v90, but in more recent versions too. I verified that this fix works in Firefox (v111) and Chromium (v90): - Attempts by the ZIM content to break out of the viewer iframe are blocked. - PDFs are displayed in the viewer.
veloman-yunkan
requested review from
rgaudin,
mgautierfr,
kelson42 and
Jaifroid
There was a problem hiding this comment.
LGTM as well but I didn't test. I can't compile libzim on my box (openzim/libzim#778) and on an ubuntu one, it compiles but the test fails…
|
Mandatory to me is the approval of @Jaifroid |
|
@veloman-yunkan @mgautierfr macos CI fails! |
It's no one's fault. We install packages via brew and one of the package's dependency is python which can't be linked because it's already linked. I believe passing |
|
Another issue/PR needs then probably to be open asap. |
|
Sorry, I'm still 7 hours behind CET, will review shortly. |
There was a problem hiding this comment.
Thanks, this looks good to me with one query about the PDF MIME type which may just be over-cautious. N.B. I can't test because I can't build this on my Windows machine. I'll happily test it thoroughly once it has been merged and is available in the dev server.
|
Seems we are good. I'm merging. |
Fixes #916
The previous "fix" (merged PR #924) was buggy. It not only didn't work in Chromium v90, but in more recent versions too.
I verified that this fix works in Firefox (v111) and Chromium (v90):
Also (unrelated to the bug being addressed) removed
moz-extension:andchrome-extension:from list of valid sources in content security policy as they are not justified forkiwix-serve(as noted in @Jaifroid's review of #924)