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.Problem/Motivation
Expected: drupal_add_js('something.js', array('scope' => 'footer')) (or the library definition equivalent) is listed in drupalSettings.ajaxPageState.js.
Result: only the JS files that are in the 'header' scope are listed in drupalSettings.ajaxPageState.js.
Proposed resolution
diff --git a/core/includes/common.inc b/core/includes/common.inc
index 10b09d6..70c0b81 100644
--- a/core/includes/common.inc
+++ b/core/includes/common.inc
@@ -3831,7 +3831,7 @@ function drupal_get_js($scope = 'header', $javascript = NULL, $skip_alter = FALS
// Provide the page with information about the individual JavaScript files
// used, information not otherwise available when aggregation is enabled.
- $setting['ajaxPageState']['js'] = array_fill_keys(array_keys($items), 1);
+ $setting['ajaxPageState']['js'] = array_fill_keys(array_keys($javascript), 1);
unset($setting['ajaxPageState']['js']['settings']);
// Provide the page with information about the individual CSS files used,
This is analogous to how the CSS page state is stored. $items only contains the items for the current scope; and since settings on most pages are added when $scope == 'header', that explains why only JS files that are in the 'header' scope get added to the page state.
Remaining tasks
Get proper test coverage. In my initial testing it turned out to be impossible to reproduce the same problem in the tests, likely because WebTestBase does not actually execute JavaScript, it merely simulates the crucial aspects.
User interface changes
None.
API changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | 1864944-1.patch | 732 bytes | Wim Leers |
Comments
Comment #1
Wim LeersComment #2
Wim LeersNow that #1824500: In-place editing for Fields has landed, this is super easy to reproduce.
drupalSettings.ajaxPageState.js.edit.jsis listed there (which has'scope' => 'footer') . It's not. This is the bug.Comment #3
nod_Works well as a stop-gap measure. It's the whole API that needs to be worked on to avoid this kind of uglyness, see #1871596: Create a PartialResponse class to encapsulate html-level data [Moving to sandbox].
In the meanwhile, this works.
Comment #4
webchickHm. Unfortunately, doesn't sound like it's possible to work #2 into a test since it's JS-only. :(
Committed and pushed to 8.x. Thanks!
Comment #5
Wim LeersIt is in fact possible to add tests for this, similarly to how I'm doing tests over at #1875632: JS settings merging behavior: preserve integer keys (allow for array literals in drupalSettings): using
json_decode().However, all of this will change in the issue that nod_ pointed to in #3 so it's not clear if it's worth the effort. I personally believe it's worth it, even if only because that'll ensure tests in #1871596: Create a PartialResponse class to encapsulate html-level data [Moving to sandbox] will test this edge case too. Thoughts?
Comment #6
webchickYeah, let's do it. That issue can always remove the test if it turns out to not be relevant.
Switching some metadata around.
Comment #7
Wim LeersI'll likely get this done tomorrow.
Comment #8
lmeurs CreditAttribution: lmeurs commentedThe patch from #1 to set scripts paths to
Drupal.settings.ajaxPageState.jsscope independently has not been backported to Drupal 7, is this something that should be done? (sorry, am not very familiar with the backporting policy)And sort of on the same subject (though D7): when the scope of the settings javascript has been altered to
footer, on AJAX calls newly attached files are not being loaded. Explanation: When the scope of the settings variable is no longerheader,drupal_get_js()does not add theDrupal.settings.ajaxPageState.jsarray,Drupal.settings.ajaxPageState.jsis not defined on page load,ajax_render()does not receive$_POST['ajax_page_state']['js']on AJAX calls,ajax_render()cannot determine which files are newly attached and thus none are appended to the head.Is there a good reason for only adding
Drupal.settings.ajaxPageState.jswhen the scope of the settings variable is set toheaderor should we create a new issue for this?BTW: Workarounds are setting these variables for JS and CSS manually on
hook_js_alter()or better to not change the scope of scripts but print the$scriptsvariable at the bottom of your page using this great little trick.Comment #23
quietone CreditAttribution: quietone at PreviousNext commentedThis was committed in Drupal 8.0 and reopened to add a test. Between then and now the function changed in this patch, drupal_get_js, was removed in #2368797: Optimize ajaxPageState to keep Drupal 8 sites fast on high-latency networks, prevent CSS/JS aggregation from taking down sites and use HTTP GET for AJAX requests. The replacement is AssetResolver with has its own tests so there no more work needed to be done here.
Therefor, I am restoring the issue meta data to the version when this was committed.