Whittle away at inline <script> tags

[cpeel]

Ideally we would have no inline JS which would allow us to tighten down our Content Security Policy. That's going to be hard to remove entirely, but if we can remove all of the pages that specify <script> we can use a nonce to cover the ones included in output_header() and slim_header() via $extra_args["js_data"] (see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#unsafe_inline_script).

So we should:

• Move JS code to .js files when possible
• Any code we can't move to .js. files needs to be lifted into $extra_args["js_data"] so it can be covered under a (to-be-developed) nonce mechanism.

Closing this ticket would be having no <script> content in PHP code outside that created in html_page_common.inc and a nonce mechanism cover the header-inserted code.

[chrismiceli]

If we do ever get to disable inline scripts, be aware that I believe it may break people's extensions, which is kind of the point, but we may get some feedback in that regard.

[cpeel]

@chrismiceli - do you have thoughts or suggestions on how to reduce the number of individual .js files that we create as part of this effort? I know that it's a best practice to reduce the number of round-trips to the server and this feels like we'll be creating a bunch of individual .js files.

[chrismiceli]

@chrismiceli - do you have thoughts or suggestions on how to reduce the number of individual .js files that we create as part of this effort? I know that it's a best practice to reduce the number of round-trips to the server and this feels like we'll be creating a bunch of individual .js files.

We definitely could create 1 or more minified bundles based on usage patterns or roles, but really I'm not 100% sure it is worth the effort. With http1.1 with keep alive or http2/3, there is only 1 TCP connection per server so there isn't a huge penalty to not bundling. Even more modern sites are kind of going away with it and instead having tons of es modules with various imports, etc.

Minifying would probably be trivial to do and reduce page load size, but our client side performance according to lighthouse is already really good: https://pagespeed.web.dev/analysis/https-www-pgdp-net-c/wf00h7vgv8?form_factor=desktop. Minifying does make debugging harder, but not really that much to be honest.

If you want me to investigate what these strategies could look like for pgdp, I could, but want to confirm the goals clearly.

[cpeel]

tl;dr: Hold off on any investigation until I see if we can support http/2.

I'm less worried about making them smaller than making fewer of them. With http 1.1 the client makes up to 6 TCP concurrent TCP connections to the server per browser -- because of keepalive more than one resource will be downloaded on that connection though. We're currently hitting a resources limit on PROD with the number of concurrent connections under high load. Having clients have more files to download will only make that worse (although only slightly worse due to caching). http/2 would be much more efficient but we can't currently support it because it requires a non-prefork Apache MPM, but mod_php will only work with the prefork MPM. To move to a non-prefork MPM we have to move to php-fpm which, in theory, will work but I have some concerns with how gettext will work with php-fpm due to how it works.

I need to do some testing on TEST to see if we can move to php-fpm and then I'd rather do that and evaluate how things go before doing any major research or work on reducing the number of .js (and .css) resources our pages request.

[70ray]

The majority of remaining inline JS is in the following areas which we are intending to revise for other reasons:

• The proofreading interface (toolbox, preview, wordcheck, proofframe)
• pinc/js_newwin.inc which is used in various places. This does not now work how it was originally intended because of changes in browsers to discourage use of pop-up windows and is inconsistent in different browsers. See issue Target new Tab instead of pop-up windows #405.
• tools/project_manager/diff.php. See pull request Link "skip empty diffs" with "without formatting" #1114
• tools/authors. Was ther some discussion of removing this entirely?

[cpeel]

tools/authors. Was ther some discussion of removing this entirely?

Yes, this code is getting deleted completely, probably after the next code release but maybe before. (see https://www.pgdp.net/phpBB3/viewtopic.php?t=82401)

[70ray]

A common case is getting translated strings into javascript functions. Should we standardize the way we do this, particularly in the new Proofreading Interface. Perhaps using an API to get an array of translated strings would avoid any security issues?

[chrismiceli]

I've seen a couple of different approaches, but we could do an API or just dump in them in the DOM as some JSON on a data-attribute or something as well. That would avoid another round to the server.