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.
Split off from #1279226: jQuery and Drupal JavaScript libraries and settings are output even when no JS is added to the page:Needs to check if $javascript isn't empty before trying to group.
Comment | File | Size | Author |
---|---|---|---|
#4 | i1554122.patch | 587 bytes | attiks |
#1 | i1554122.patch | 4.43 KB | attiks |
Comments
Comment #1
attiks CreditAttribution: attiks commentedComment #2
nod_all good
Comment #3
tim.plunkettSimilarly to #1554142: drupal_array_merge_deep_array assumes $array is actually an array, I would think this was the caller's responsibility. The function is documented as taking an array, and doesn't not provide any defaults, so it shouldn't need to check for an array.
Furthermore, if it were to perform the check itself, it might as well as do
if (empty($javascript)) { return $groups; }
right at the top and not needlessly indent (see this for more on guard clauses).Comment #4
attiks CreditAttribution: attiks commentedNew patch attached
drupal_group-jss is defined at http://drupalcode.org/project/drupal.git/blob/refs/heads/8.x:/core/modul...
Comment #5
tim.plunkettMaybe I'm just confused, but under what conditions is $javascript empty? And what is the value of $javascripts?
drupal_get_js returns a render array with #items set to at least an empty array, which is passed to drupal_group_js. If the array is empty, the foreach is smart enough to not loop through it.
The only reason this would be needed would be if its not an array at all, which would mean a bug elsewhere.
Comment #6
attiks CreditAttribution: attiks commented@Tim drupal_get_js can also return an empty string:
but the current code uses drupal_add_js to get the javascript.
I gonna postpone this until there's a decision in #1279226: jQuery and Drupal JavaScript libraries and settings are output even when no JS is added to the page
Comment #7
tim.plunkettAFAICS, that return value should be one of the #items, not the only one.
Comment #8
sunAdding array type-hinting for $javascript can't hurt either.
$javascript == #items, as passed from drupal_pre_render_scripts, which defaults to an array in system_element_info(), but that's nowhere enforced.
Comment #9
nod_The issue is solved another way for D8, relevant if we want to fix the other issue for D7.
Comment #10
mgiffordOk, then it needs a D7 patch. Not sure if this is still needed though.