Problem/Motivation

This ticket is to target the deprecations template_preprocess discovery, including file/include callbacks.

Likely also _some_ but not all deprecate template_preprocess callbacks, such as those in core/includes/theme.inc.

Note: There is one overlap with theme engines. I've decided to remove theme engine preprocess callbacks here as well.

Note: This also removes $this->root as the first constructor argument from ThemeManager and Registry. Doing BC there would be pretty tedious as it's the first argument and we'd need to reshuffle all of them, remove types, do BC and so on. There are only very few contrib subclasses. bootstrap _will_ break, but it will need adjustments anyway as it hardcodes the constructor and does all kind of weird things: https://search.tresbien.tech/search?q=%22extends%20(ThemeManager%7CRegistry)%22&num=50&ctx=5.

Steps to reproduce

Proposed resolution

Remaining tasks

Ensure template_preprocess_views_mini_pager comments are cleaned up.
Create follow up issues:

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3575542

Command icon Show commands

Start 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:

Comments

berdir created an issue. See original summary.

berdir’s picture

Issue summary: View changes
berdir’s picture

Title: Remove deprecations related to the template_preprocess discovery » Remove deprecations related to the theme preprocess discovery and theme.inc
berdir’s picture

Issue summary: View changes

berdir’s picture

Status: Active » Needs review

So, I might have gone a bit overboard, but since we're already making changes to the Registry constructor anyway, I realized there's another issue that we can clean up properly. We already have the optional $theme_name constructor parameter, so I couldn't make keyValueStore required.

The thing is, that theme_name parameter has been bugging me for *many* years. It's only used in a few kernel tests that want to test the registry for a specific theme. But we already have a way to do thaat, by setting the current active theme either through config or an explicit API. This significantly complicates Registry, we need the themeInitialization service just for that. With this ability removed, init() is now a single line to get the current theme. There was also an initialized flag, but this flag hasn't been set to TRUE since 12 years.

The tests are updated to get the regular registry service and set the current theme. In the one test that needs multiple, all I need to do is switch out the currently active theme before calling get() on the same service.

nicxvan’s picture

Issue summary: View changes

This is so satisfying, it looks so good, I just have a few questions.

Since we're already doing so much with the constructor already should we just go all the way with property promotion, we only want to document $this->registry but we can do that on the constructor I think.

ThemeTest references template_preprocess_html which we should probably clean up.
ViewsProcessTest references template_preprocess_views_mini_pager it's probably out of scope to clean that up here.

Finally there is a comment * @todo Replace local $registry variables in methods with $this->registry. which I think we can delete.

I confirmed all of the deletions in theme.inc are only the deprecations.
I pulled it down and reviewed Registry and reviewed it.
Then I searched for template preprocess.

Those test clean ups are almost as satisfying as the theme.inc.

I added links to the three modules extending Registry so we can create follow ups once this gets in.

I think this is actually pretty close.

dcam’s picture

Status: Needs review » Needs work

I worked on reviewing this, but I didn't make it through checking all of the template preprocess functions before having to call it a night. I left a few comments on the MR.

If you run grep -rn "template_preprocess_links" core there are about 10 lingering references to it throughout Core. At least some of these seem to be referring to the docblock of the former function. These references should be updated.

berdir’s picture

Status: Needs work » Needs review

Addressed the review, did property promotion and autowire, restored the removed functions that are scheduled for D13 removal, cleaned up template_preprocess_links() references.

berdir’s picture

autowire has some unexpected side effects around the kernel, I have undone that, although I think it would be nice if we could find a better solution that doesn't require injecting the kernel here, but we already do a lot/too much for one issue.

nicxvan’s picture

Good catch in 9. I only checked we did not remove any non deprecated code I forgot to check the version.

Also the reason I didn't see those template preprocess references is because I accidentally restricted my search to *Test.php files.

I'll review the remaining functions later if dcam doesn't best me to it.

dcam’s picture

FYI, I just sat down to resume my review.

dcam’s picture

Status: Needs review » Reviewed & tested by the community

Thank you for checking those removed theme function references before I got back to them. I double-checked them and didn't find anything.

Thank you for considering my feedback. I don't have anything else to add to my review. It sounds like @nicxvan was only planning on reviewing the other theme functions. Since that's pretty straightforward and I did it I will go ahead and mark this as RTBC.

nicxvan’s picture

+1 rtbc

nicxvan’s picture

Status: Reviewed & tested by the community » Needs work

Just found something in the theme engine issue

nicxvan’s picture

Status: Needs work » Reviewed & tested by the community

Nevermind, I was looking at the wrong tab, this issue is still RTBC

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to main, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • catch committed 4eb589c0 on main
    refactor: #3575542 Remove deprecations related to the theme preprocess...

Status: Fixed » Closed (fixed)

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