This is the 7.x back-port issue for #939462: Specific preprocess functions for theme hook suggestions are not invoked.

Proposed commit message:

Issue #939462 by lauriii, Antti J. Salminen, NROTC_Webmaster, mbrett5062, tim.plunkett, tostinni, rteijeiro, dvessel, barraponto, theapi, joelpittet, cilefen, tuutti, drzraf, Fabianx, markcarver, xjm, catch, jenlampton, sun, effulgentsia, Cottser, davidhernandez, kscheirer, andypost, akalata, rooby, hass, fubhy, jhodgdon, lemunet, gleroux02, mike stewart, kevinquillen, MXT, mlncn, becw, PavanL, chriscalip: Specific preprocess functions for theme hook suggestions are not invoked

General summary

  • This is not considered a beta blocker or critical issue for D8.
  • While patches exist in this thread for D7 (and may work for you), official fix will be to D8 first -- please do not switch issue metadata until we are ready to start the D7 backport.

Problem/Motivation

When you have a template suggestion available and are using that template, preprocess functions following naming conventions provided via hook_theme_suggestions_HOOK() are not being recognized, for example:

MYTHEME_preprocess_node__article()
MYTHEME_preprocess_block__search_form_block()
MYMODULE_preprocess_page__front()
MYMODULE_item_list__search_results()
etc.

Proposed resolution

Add a post-process step to the theme registry build to look for these types of preprocess functions and add them to the theme registry.

Remaining tasks

Patch needs review.

User interface changes

N/A

API changes

Removes the global drupal_group_functions_by_prefix() (moves it to a class to make things testable) which was added May 13, 2015: #2339447: Improve theme registry build performance by 85%

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug, there was similar functionality to this in Drupal 6, or at least in Views
Issue priority Major because it represents a big improvement to themer/developer experience.
Disruption Little to no disruption, just an addition that core and contrib can choose to use or not use.

CommentFileSizeAuthor
#10 d7-update-theme-registry-better-inheritance.patch3.9 KBPol
#3 2563445-preprocess-2.patch17.56 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 41,754 pass(es), 1 fail(s), and 0 exception(s). View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

markcarver created an issue. See original summary.

aspilicious’s picture

The D8 solution broke Display Suite, so please ping me before this gets committed..

tim.plunkett’s picture

Status: Active » Needs review
FileSize
17.56 KB
FAILED: [[SimpleTest]]: [MySQL] 41,754 pass(es), 1 fail(s), and 0 exception(s). View

This is my best attempt to reroll #939462-62: Specific preprocess functions for theme hook suggestions are not invoked, which was from early 2012. I did not try to backport what was committed to D8.

Status: Needs review » Needs work

The last submitted patch, 3: 2563445-preprocess-2.patch, failed testing.

The last submitted patch, 3: 2563445-preprocess-2.patch, failed testing.

stefan.r’s picture

Issue tags: +Drupal 7.60 target
Pol’s picture

Hello,

That system is already implemented in Atomium (see specific commit)

It's based on Bootstrap theme, but I improved it a bit to have a proper cascading execution of those functions.

So, you are now able to have preprocess and process functions for custom theme() calls per default.
We have included this feature in the theme, but a patch would be more than welcome.
This is, according to me, one of the best feature of the theming layer that we are missing in Drupal 7.

Examples:

theme('link__variant1__variant2', array(...));

The original hook is link and the preprocess functions that will be triggered, if they exists are:

  1. HOOK_preprocess_link()
  2. HOOK_preprocess_link__variant1()
  3. HOOK_preprocess_link__variant1__variant2()

Another example:

theme('link__variant1__variant2__variant3', array(...));
  1. HOOK_preprocess_link()
  2. HOOK_preprocess_link__variant1()
  3. HOOK_preprocess_link__variant1__variant2()
  4. HOOK_preprocess_link__variant1__variant2__variant3()
netw3rker’s picture

It looks like there are a lot of problems with this patch/reroll
1) code to properly call templates with paths was deleted, thus it is setting template paths to be docroot if they weren't explicitly specified.

-        // Prepend the current theming path when none is set.
-        if (!isset($info['path'])) {
-          $result[$hook]['template'] = $path . '/' . $info['template'];
-        }

later this becomes doubly compounded when the template is actually called:

     $template_file = $info['template'] . $extension;
-    if (isset($info['path'])) {
-      $template_file = $info['path'] . '/' . $template_file;
-    }
     if (variable_get('theme_debug', FALSE)) {
       $output = _theme_render_template_debug($render_function, $template_file, $variables, $extension);
     }

these blocks help with 2 cases: 1) the theme element has a template named, and a path provided, 2) the theme element has a template with the path listed in it. (for example modules/toolbar/toolbar.tpl.php)

2) it is looking in the defined functions namespace and attempting to derive available pre/process function names. This is problematic because many theme elements include their preprocess functions in separate include files (for example bootstrap theme's bootstrap_preprocess_html() located in bootstrap/includes/html.vars.php. If this is to work correctly, all include files for all theme elements would need to be included before this line could run:

+function _theme_post_process_registry(&$cache, $theme, $base_theme, $theme_engine) {
+
+  // Get all user defined functions.
+  list(, $user_func) = array_values(get_defined_functions());
+  $user_func = array_combine($user_func, $user_func);
+
+  // Gather prefixes. This will be used to limit the found functions to the

I think this needs a harder look to ensure it's being correctly handled.

Pol’s picture

I think the whole registry generation should be revisited.

Lately, I've spend some hours on Atomium and I've updated the way the registry is altered.

I ended up rewriting each hooks... here's how I did:

/**
 * Implements hook_theme_registry_alter().
 */
function atomium_theme_registry_alter(&$registry) {
  // Retrieve the active theme names.
  $themes = _atomium_get_base_themes(NULL, TRUE);

  // Return the theme registry unaltered if it is not Atomium based.
  if (!in_array('atomium', $themes)) {
    return;
  }

  // Get all themes.
  $all_themes = list_themes();
  $current_theme_engine = $all_themes[$GLOBALS['theme']]->engine . '_engine';

  // Process registered hooks in the theme registry.
  _atomium_process_theme_registry($registry, $themes);

  // Process registered hooks in the theme registry to add necessary theme hook
  // suggestion phased function invocations. This must be run after separately
  // and after all includes have been loaded.
  _atomium_process_theme_registry_suggestions($registry, $themes);

  // Compile a list of prefixes.
  // The order of this is very important, see the doc at:
  // https://api.drupal.org/api/drupal/includes!theme.inc/function/theme/7.x
  $prefixes = array('template' => 'template')
    + module_list()
    + array($current_theme_engine => $current_theme_engine)
    + array_combine($themes, $themes);

  // Alter each hook and compile a complete list of preprocess/process functions
  // in the right order.
  foreach ($registry as $hook => &$info) {
    $variable_process_phases = [
      'preprocess functions' => 'preprocess',
      'process functions' => 'process',
    ];

    $stack = array($hook);
    while ($pos = strrpos(current($stack), '__')) {
      $stack[] = drupal_substr(current($stack), 0, $pos);
      next($stack);
    }

    foreach ($variable_process_phases as $phase_key => $phase) {
      $info[$phase_key] = array();
      foreach ($prefixes as $prefix) {
        $info[$phase_key][] = $prefix . '_' . $phase;

        // This is the code that ensure preprocess/process inheritance.
        array_map(function ($hook) use (&$info, $phase, $phase_key, $prefix) {
          $info[$phase_key][] = $prefix . '_' . $phase . '_' . $hook;
        }, array_reverse($stack));
      }

      // Ensure uniqueness of functions.
      $info[$phase_key] = array_unique($info[$phase_key]);

      // Filter out functions that does not exist.
      $info[$phase_key] = array_filter($info[$phase_key], function ($function) {
        return function_exists($function);
      });

      // Sadly we have to remove preprocess and process for hooks that provides
      // a theme function. (like the date module with date_display_single).
      // We could have this in Atomium but some modules badly implementing
      // attributes handling would fail.
      if (isset($info['function']) && function_exists($info['function'])) {
        $info[$phase_key] = array_filter($info[$phase_key], function ($function) use ($phase, $hook) {
          return (FALSE !== strpos($function, $phase . '_' . $hook));
        });
      }
    }

    $info['includes'] = array_unique(
      array_merge(
        (array) $registry[atomium_get_base_hook($hook)]['includes'],
        (array) $info['includes']
      )
    );

    // Ensure "theme path" is set.
    $info += array(
      'theme path' => $GLOBALS['theme_path'],
    );

    // Remove this member so each hook is independent and doesn't depend or
    // inherit of it's parent hook.
    // This prevent many situations where the preprocess/process calls orders
    // are not triggered in the right order.
    unset($info['base hook']);
  }
}
Pol’s picture

Status: Needs work » Needs review
FileSize
3.9 KB

I worked on a new patch.

It's very simple and I'd like to have feedback.