-
-
Notifications
You must be signed in to change notification settings - Fork 76
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
Relocate inline <body> scripts to their own tag in the <head> #2110
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2110 +/- ##
=======================================
Coverage 75.43% 75.44%
=======================================
Files 41 41
Lines 3188 3205 +17
Branches 703 707 +4
=======================================
+ Hits 2405 2418 +13
- Misses 666 670 +4
Partials 117 117 ☔ View full report in Codecov by Sentry. |
@audiodude Just to clarify, the idea is that they are collected and put into a separate js file which is then called on |
I'll take a look at the code asap. |
Nope that's not it. I guess I misunderstood the requirements, so thanks for looking! This collects all of the inline EDIT: See the test for the example of what happens: https://github.com/openzim/mwoffliner/pull/2110/files#diff-3e8990a262b08f8ee68d62817c1c36e46727c30f9068b3468ec3c8d802cbe8b7 |
Ah OK. It used to be the case that mwOffliner could include its own script files (overrides). It would probably be complicated to do. If these scripts are not doing anything useful now that all sections are open anyway (their function was to open the sections), then removing them would be the simpler option. |
mwoffliner can, and still does, that. However, it's for generic scripts that apply to all pages. We would have to assume every page has
Yes, as I said on the bug, the README for these scripts say they are for lazy loading images, "themeing" (whatever that is), and obviously for unhiding sections (though the README doesn't mention that). |
Closing this, as it doesn't meet requirements. Will try again with a version that only strips inline scripts, but doesn't relocate them. |
Fixes #2096
In the abstract Renderer, we create a method, collectInlineJs, which grabs any
<script>
tags that are in the body of the article, takes the actual JavaScript inside, and collects it into a string. The original<body>
<script>
tags are removed in the process. This string is passed to the callback ofprocessHtml
, where the concrete Renderers convert the intermediate<body>
HTML into a full HTML document, and put in the<head>
.We introduce a new template,
inline_js_to_head_wrapper.html
, which contains a single script tag, with the body scripts wrapped in aDOMContentLoaded
listener. This means that the scripts will still execute, in supported environments, after the page has loaded. It is unclear the proper function of these scripts, but the thought is that we might as well try to run them since we're already including scripts such aspcs.js
in the page.