#2760659: Allow the use of callbacks instead of global functions in preprocess function callbacks removed the ability to get $info by reference in preprocess functions. In ThemeManager.php, line 287 was changed.
9.3.x: $preprocessor_function($variables, $hook, $info);
9.4.x: call_user_func_array($preprocessor_function, [&$variables, $hook, $info]);
Have a function like modulename_preprocess_input(&$variables, $hook, &$info);
In 9.3.x it works. In 9.4.x it errors with:
Warning: Parameter 3 to modulename_preprocess_input() expected to be a reference, value given in /var/www/html/web/core/lib/Drupal/Core/Theme/ThemeManager.php on line 287
Putting it back to how it was would fix it but it was done intentionally so that's probably not an option. I don't really know how to make the change without this break to backwards compatibility.
Show commandsStart 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:
changes
Comments
Comment #2
michelleComment #3
cilefen commentedDoes
modulename_preprocess_input(&$variables, $hook, $info);work?Comment #4
michelle@cilefen - I don't see why it wouldn't since the error happens when you try to get $info by reference.
Comment #5
longwaveWhy do you need to get it by reference? Should preprocess functions be able to alter the $info array?
Comment #6
michelle@longwave - In this case, I probably don't. It was inherited code and it looks like it can be changed to use hook_theme_suggestions_alter() instead. But this change broke existing code so I at least wanted it documented so it's findable to anyone else hitting the same issue and wondering why their code is suddenly breaking from a minor release.
Comment #7
jrockowitz commentedThe Dashboards with Layout Builder module is currently altering the $info to add support for the Gin admin theme.
@see https://git.drupalcode.org/project/dashboards/-/blob/2.0.x/dashboards.mo...
Is there an alternative way to dynamically set the 'theme_path'.
Comment #9
elneto commentedHi,
I just changed line 287 of ThemeManager.php so we pass $info by reference.
https://git.drupalcode.org/issue/drupal-3292967/-/tree/3292967-no-longer...
call_user_func_array($preprocessor_function, [&$variables, $hook, &$info]);
It worked for my site.
Sorry if this is not the way to contribute. This is my first time.
Comment #10
longwave@eineto Welcome - this is the right way to contribute! The next steps are to open a merge request for the
3292967-no-longer-ablebranch and set the issue status to "needs review", and your change will be considered for merging into Drupal.Although this was previously unintended behaviour I think this is a regression from 9.3.x and so it is probably worth fixing any sites that were broken by this now, and then perhaps finding a workaround or issuing a deprecation to users that are handling $info by reference.
Comment #11
elneto commentedRegressed it to how it was in Drupal 9.3. Passing info by reference.
Comment #12
elneto commentedYou are right @longwave. It is probably better not to pass it by reference on the first place. I guess I was influenced by @jrockowitz comment.
I have no strong feelings about it as long as it works well. Feel free to deny the merge depending on what's the desired way to go. If I knew how I would unrequest my merge :)
Comment #13
longwaveComment #15
longwaveWe obviously don't have any test coverage for this either way, but this is a regression so marking RTBC. Core committers can decide whether this is a trivial fix go to into the next 9.4.x, whether this needs tests, or whether this was invalid behaviour in the first place.
Comment #16
catchThe dashboards module has already been updated to do this differently in #3293693: [Drupal 9.4] Parameter 3 to dashboards_preprocess_dashboards_admin_list() expected to be a reference, value given, but there could be more examples out there.
I feel like passing $info by reference was unintended, so not really sure we should re-enable it (at least in the absence of more examples of people relying on it). But... we should add a change record for the original issue at least.
Leaving RTBC and tagging for more opinions. Another option would be to add back the pass by reference then open a new issue to remove it again, but no way to do a formal deprecation so not sure that would help anything much.
Comment #17
larowlanYes I agree that we should try not to reintroduce this, the info variable is there for reference only.
So +1 for adding a change record for the other issue and marking this as works as designed or won't fix
Comment #18
catchThanks for the second opinion. I've added a change record, and also added a release note to 9.4.0 https://www.drupal.org/project/drupal/releases/9.4.0
I think we can go ahead and close this.
Comment #19
michelleThanks! I had a feeling this wouldn't get reverted but nothing came up when I searched the error message so I at least wanted to get something documented for other people that search. A change record works great for that. :)