[D10.2 regression] Theme suggestions cannot alter variables anymore

Given it has the potential to break sites, I think it's at least major

Unassigned

Just tested this locally, and it doesn't work for me unfortunatelly

@vulcanr, it restores the behavior before 10.2.0 for us. Could you, please, add more details about your case?

Given that we've regressed here, a small test of the expected functionality would be great to have.

(crosspost with @alexpott)

Given that ThemeManager::alter() takes its second context argument ($variables) by reference does imply to me that this is intentional to some extent; if we didn't mean for that to happen we could have not done that when this code was first added. And given that the fix solves a regression and doesn't harm anything else then I think this is fine to add back in.

Perhaps as a followup we should add test coverage for this though, if we think it should actually be a supported feature?

Added some test coverage for this (unexpected) feature.

I think we should update the documentation in theme.api.php to note that $variables is passed in by reference.

Also updated the docs (and fixed the first arg to hook_theme_suggestions_HOOK_alter() while I was there)

@longwave great work on the tests and the docs. Looks great.

that this is intentional to some extent; if we didn't mean for that to happen we could have not done that when this code was first added.

When this code was first added in #678714 theme_hook_suggestions was a special variable set in hook_preprocess. At this point variables and suggestions were changeable together simply because they were not separate things yet. The hook was added in #1751194 like this Drupal::moduleHandler()->alter('theme_suggestions_' . $base_theme_hook, $suggestions, $variables);. At this point both the hook documentation and the comment at this place only mentions the suggestions as alterable: function hook_theme_suggestions_HOOK_alter(array &$suggestions, array $variables) { -- $variables , not &$variables. The comment said: // Allow suggestions to be altered via hook_theme_suggestions_HOOK_alter(). instead of "Allow suggestions and variables to be altered". I searched the issue in question for $variable and there was not a single mention of the necessity of them being changeable in suggestions. So everything suggests variables was not supposed to be alterable, it just so happened alter takes everything by reference.

As for ThemeManager::alter , that was added in #2228093 as a copy of ModuleHandler::alter. There was no special consideration, it was just copied. The issue didn't discuss any of this either.

This is just to provide a little historical background which is all I do.

Committed/pushed to 11.x and cherry-picked to 10.2.x, thanks!

Please close the mr

catch → closed merge request !5898

Automatically closed - issue fixed for 2 weeks with no activity.