Problem/Motivation

Theme functions template_preprocess_menu_local_task() and template_preprocess_menu_local_action() defined in menu.inc. After deprecations of the menu.inc functions would be logically to store template_* functions in the theme.inc file untill we will find better place or replacement solution for templeate_* functions.

Proposed resolution

Move this preprocess functions to the core/includes/theme.inc

Remaining tasks

patch, discuss, commit

User interface changes

no

API changes

no

Data model changes

no

Release notes snippet

no idea

CommentFileSizeAuthor
#15 3021788-15.patch5.01 KBsuresh prabhu parkala
#13 3021788-13.patch5.01 KBvoleger
#11 3021788-11.patch5.01 KBvoleger
#9 3021788-9.patch4.89 KBvoleger
#3 3021788-2.patch5.05 KBandypost

Comments

andypost created an issue. See original summary.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue tags: +Kill includes
Parent issue: » #2999961: [META] Deprecate contents of menu.inc
andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review
StatusFileSize
new 3021788-2.patch5.05 KB
voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Status: Needs review » Reviewed & tested by the community

Nice, let's move them.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue summary: View changes
Status: Reviewed & tested by the community » Needs work

Yeah but their theme hook is defined in drupal_common_theme() which is in theme.inc.

I think rather than doing this piecemeal we should come up with a general rule of how core theme implementations should be registered and the templates and preprocess functions are located.

If we do this patch we're just going to be shifting a lot of code into system.module for all the theme implementations in drupal_common_theme() and that feels wrong.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue tags: +Needs frontend framework manager review
andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review

@alexpott Except this 2 functions and menu_list_system_menus() all other functions are deprecated in menu.inc

So I think it's fine to move this 2 preprocesses to system module to get rid of include instead of looking for "general" solution

PS: maybe better move them to theme.inc for a while?

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
StatusFileSize
new 3021788-9.patch4.89 KB

Move the template_preprocess_* functions from menu.inc to theme.inc.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Version: 8.9.x-dev » 9.0.x-dev
StatusFileSize
new 3021788-11.patch5.01 KB

Here the patch for 9.0.x

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Title: Move template_preprocess_menu_local_task() and template_preprocess_menu_local_action() to system module » Move template_preprocess_menu_local_task() and template_preprocess_menu_local_action() to the core/includes/theme.inc
Issue summary: View changes
Issue tags: -Needs frontend framework manager review

Update IS.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
StatusFileSize
new 3021788-13.patch5.01 KB

Just rerolled

xjm’s picture

xjm
she/her
Primary language English
commented
Version: 9.0.x-dev » 9.1.x-dev
Status: Needs review » Needs work

This would be a minor-only change. Since 8.9.x and 9.0.x are now in beta, I'm moving this to 9.1.x. Thanks!

suresh prabhu parkala’s picture

suresh prabhu parkala commented
Status: Needs work » Needs review
StatusFileSize
new 3021788-15.patch5.01 KB

Please review.

daffie’s picture

daffie commented
Status: Needs review » Reviewed & tested by the community

The code changes look good to me.
For me it is RTBC.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Needs frontend framework manager review

  • lauriii committed 0585f27 on 9.1.x 
    Issue #3021788 by voleger, andypost, xjm, alexpott: Move...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs frontend framework manager review

Confirmed that there aren't any code in the patch that would rely on something that was defined in between the current location and the previous location. There isn't many lines of code in between since these files are loaded after one another in DrupalKernel::loadLegacyIncludes().

Committed 0585f27 and pushed to 9.1.x. Thanks!

Status: Fixed » Closed (fixed)

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