Problem/Motivation
We use patch https://www.drupal.org/files/issues/advagg-handle-external-same-host-js-... on advagg 2.4.0 to allow external JS to pass through the optimizer without being converted to a file. With the 3.x branch, the JS option was retained but there are two issues.
- There is a bug in the SettingsForm that the "Preserve External" checkbox configuration is not saved. The issue is that the form_state key in the #submitForm is "js.preserve_external" but in the #buildForm it is setup with "js_preserve_external" (which is the syntax the rest of the keys use).
- We need the same functionality for CSS (not a bug of course. This would be an enhancement).
Proposed resolution
Per above, solution for each item is as follows.
- Fix the key in the form_state->getValue() to use "js_preserve_external".
- Add the same logic to the CSS optimizer, SettingsForm, and default settings.
Remaining tasks
None.
User interface changes
Added the same "preserve external" checkbox to CSS options as the JS options.
API changes
None.
Data model changes
Added new css.preserve_external to settings.yml
2.4.x ticket for reference
External JS hosted by same domain as Drupal converted to file
+ the addition of
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | advagg-preserve-external-js-css-2943275-4.patch | 3.68 KB | sdewitt |
| #3 | advagg-preserve-external-js-css-2943275-2.patch | 3.68 KB | sdewitt |
Comments
Comment #2
sdewitt commentedAttached a patch that fixes #1 described in description and adds feature function for #2.
Comment #3
sdewitt commentedComment #4
sdewitt commentedSorry, numbered the first patch file wrong. No diff between patch 2 (comment #3 :-( ) and patch 4 - correctly numbered so no interdiff added.
Comment #5
nickdickinsonwildeThanks for catching that bug and the new functionality, I'm reviewing it now.
In the future, no need to take the time if your patch file is numbered wrong - unless there is a ton of comments/files/long issue, its totally fine. Also, best practice to set Status to 'needs review' and unassign yourself from issue when you've put a patch up. In this case it had no effect, but on a busy issue queue, needs review is often treated as higher priority than active - less work for more benefit usually.
Comment #7
nickdickinsonwildePatch committed, thanks.
Comment #8
nickdickinsonwildeComment #9
sdewitt commentedThanks for the best practice advice @NickWilde. I will do this in the future. And thanks for the quick turnaround on the commit!