Skip to content
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

Remove inline javascript to comply with some CSP #1578

Closed
mossroy opened this issue Jan 5, 2022 · 7 comments · Fixed by #1752
Closed

Remove inline javascript to comply with some CSP #1578

mossroy opened this issue Jan 5, 2022 · 7 comments · Fixed by #1752
Assignees
Milestone

Comments

@mossroy
Copy link

mossroy commented Jan 5, 2022

There is at least some pieces of inline javascript in every page, like :

 <script>
   function importScript() { return 1 } // this is to avoid the error from site.js
 </script>
 <script>let articleId = 'Wikivoyage:Offline_reader_Expedition/Europe_Home_page'</script>

the articleId is used afterwards to handle webp.

The content can be browsed anyway, at least on the few ZIM files I tested

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

@stale stale bot added the stale label Apr 16, 2022
@mossroy
Copy link
Author

mossroy commented Apr 16, 2022

The ZIM files generated by mwoffliner are probably the most used ones, at least for kiwix-js.
It seems very important to me that this is fixed, so that they work completely, with no error/warning.

@stale stale bot removed the stale label Apr 16, 2022
@kelson42 kelson42 added this to the 1.11.11 milestone Apr 16, 2022
@kelson42 kelson42 modified the milestones: 1.11.11, 1.12.0 May 7, 2022
@kelson42 kelson42 self-assigned this Jun 1, 2022
@mossroy
Copy link
Author

mossroy commented Jun 2, 2022

As discussed with @kelson42, the second code block (that sets articleId) only affects the webp polyfill.
This should have no visible user impact, because there's not a lot of browsers that support SW but do not support webp image format:

  • Firefox 44 to 64, but Firefox does not support SW in webextensions (where we have the CSP issue). And the PWA workaround does not have the CSP
  • Edge 17, but it's the old Edge, that does not support our extension

So this blocked javascript prevents the webp polyfill to load, but the browser should be able to natively handle webp.

@mossroy
Copy link
Author

mossroy commented Jun 2, 2022

The impact of the first code block still needs to be checked (but we believe it might be minor)

@stale
Copy link

stale bot commented Aug 13, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

@kelson42
Copy link
Collaborator

@Jaifroid I have launched a recipe with this fix at https://farm.openzim.org/pipeline/7fed4cc3e015578b5dc53d36. Let me know if you have any feedback.

@Jaifroid
Copy link
Collaborator

@kelson42 I downloaded the new ZIM and have been looking through it in the Chromium extension (the one affected). It all looks good: I can't find any inline JS remaining, and I don't see any related errors in console. So I think this issue is indeed fixed. Many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants