Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
For some reason, i don't want my javascript files to be served from CDN.
So, I added *.js in Exception for the blacklisted file path.
But, my js files are not aggregating even if I enabled " Aggregate JavaScript files." on performance page.
Comment | File | Size | Author |
---|---|---|---|
#20 | 2684289-20.patch | 1006 bytes | Wim Leers |
Comments
Comment #2
nileshlohar CreditAttribution: nileshlohar commentedHere is the patch for it
Comment #3
nileshlohar CreditAttribution: nileshlohar commentedIssue appears only when I add "*.js" in exception ie when I don't want any of my js files to be loaded from CDN.
if I add any other path specific exception it works well.
my patch resolves the issue and works in both conditions.
Comment #5
nileshlohar CreditAttribution: nileshlohar commentedUpdated patch for failed testing.
Comment #6
nileshlohar CreditAttribution: nileshlohar commentedMore correct version.
Comment #7
Wim LeersPlease provide steps to reproduce, I don't yet understand how JS aggregation is supposedly broken in version 2.7 and how this would fix it.
Comment #8
Wim LeersComment #9
nileshlohar CreditAttribution: nileshlohar commentedSteps to Reproduce:
Result:
JS files are not aggregated.
Reason:
This code makes sure that CDN-blacklisted JS files are not aggregated.
but as I am blacklisting all my js file (*.js)
It's blocking aggregation of all JS files and as a result, No js files are aggregated.
Resolution:
I am adding a condition to check if all JS files are blacklisted then further code doesn't gets executed to block all JS files aggregation.
Hope this helps....
Comment #10
nileshlohar CreditAttribution: nileshlohar commentedComment #11
Wim LeersI see now. Thanks for the explanation!
This bug was introduced by #2499967: CSS/JS Aggregation and blacklisting of individual CSS/JS files within the aggregates.
And in fact, the same problem exists for CSS aggregates: if you blacklist
*.css
. But nobody is probably doing that.Comment #12
Wim LeersMarked it
because you have to blacklist all JS files to hit this edge case.Comment #13
Wim LeersCan you please test this patch?
Comment #14
nileshlohar CreditAttribution: nileshlohar commentedWorking fine with the small update.
I'm attaching updated patch with:
Comment #16
nileshlohar CreditAttribution: nileshlohar commentedUpdated Patch after solving Errors.
Comment #18
nileshlohar CreditAttribution: nileshlohar commentedAgain... Updated Patch.
Comment #19
nileshlohar CreditAttribution: nileshlohar commentedComment #20
Wim LeersYou're right in #14 in that the patch in #13 indeed has
*.css
where it should have*.js
. But instead of fixing that, you changed the patch completely.So, bringing back #13, because the code in there is cleaner/simpler. And fixing the bug you noticed. Can you please test this patch? :) Thanks!
Comment #21
nileshlohar CreditAttribution: nileshlohar commentedTested and it is fixing the Bug.
Comment #23
Wim LeersThanks! Credited you :)
Comment #24
nileshlohar CreditAttribution: nileshlohar commentedThanks :)
Comment #25
Wim LeersThank you! :)