Problem/Motivation
JavaScript files are only aggregated from AJAX requests, when the maintenance mode constant is set. This was a type in a previous bugfix where files were aggregated even in maintenance mode.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | Screenshot 2024-06-06 at 10.28.10 AM.png | 82.05 KB | punamshelke |
| #6 | Screenshot 2024-06-06 at 10.29.05 AM.png | 96.85 KB | punamshelke |
| #2 | 3452672-optimized-for-normal-mode.patch | 787 bytes | punamshelke |
Issue fork drupal-3452672
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3452672-aggregate-javascript-files
changes, plain diff MR !9204
Comments
Comment #2
punamshelkeComment #3
punamshelkeComment #4
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue.
While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)
Comment #5
cilefen commentedI think that is intentional but the comment may be wrong. Can you verify by reading over the original issue?
Comment #6
punamshelkeHi @cilefen
MIght be intentional but I have tested this aggregation and its not working properly and behaviour is not constant.
After analysis I have found this piece of code and I have compared with with 10.0 then looks like mistake might be I am wrong but want other also test this case before closing....
10.0

10.1 onwards

Comment #7
cilefen commentedIt’s understood that #3373328: ^10.1 CSS aggregation breaks during maintenance mode changed the code.
Comment #8
mmillford commentedThe issue seems to be that it removed a `!` from the line declaring $optimize_js, so now js is only optimized in maintenance mode. #3373328 was focused on the css aggregation, so its understandable that the missing exclamation point for js optimization would be overlooked.
Comment #10
catchWas really confused by this issue because aggregation in general is working fine, then realised this affects JavaScript added via AJAX responses only.
I'm not sure if we have an existing test for this that can be adapted. Since it's a one character fix for what was essentially a typo committed to 10.1 already, moving to needs review.
Comment #12
catchComment #13
smustgrave commentedCan the issue summary be updated.
This is a small change but should tests be a follow up?
Comment #14
catchUpdated the issue summary. Given this was a typo and it's a regression in released versions, I personally think tests should be a follow-up here. Would be good to add to performance test coverage but we'll need an AJAX request that adds at least two JavaScript files at once, and I'm not sure where to find one of those yet.
Comment #15
smustgrave commentedThanks. Opened #3468754: Add performance test coverage for AJAX requests for the tests.
Comment #20
nod_Since this is an issue in 10.x I backported to 10.3.x
Committed and pushed 07abb808af to 11.x and b6ea46a9e1 to 11.0.x and 1bcb65983f to 10.4.x and 379efd0a0b to 10.3.x. Thanks!