Hi,

me_theme_registry_alter implements regstry alter that forces drupal theming layer to use me module theme_menu_item_link function. Why exactly is that? This breaks any theme that implements it's own theme_menu_item_link function

Comments

hefox’s picture

Context does similair, but it checks who is currently controlling the theme and doesn't touch it if the theme is using it, I'd suggest the same.

donquixote’s picture

me.module now does this in me_theme_menu_item_link(),
see the patch in #419082: Seems to break DHTML Menu.

This is nice, but there are a few problems:

1. Multiple themes on one site (with sections or context).
me_variable_get('me_theme_menu_item_link')
-> this will always return the function for the default site theme, even when the current page has a different theme.

2. Theme inheritance.
function_exists($GLOBALS['custom_theme'] .'_menu_item_link')
-> This does not check for base theme implementations.

I'm currently thinking about how this can be fixed, without breaking other modules' theme_registry_alter(). Not so easy.

donquixote’s picture

Here is the solution (will correct when I find bugs).

<?php
function me_theme_registry_alter(&$theme_registry) {
  if (isset($theme_registry['menu_item_link']['function'])) {
    // The registry is different for each theme.
    $theme_registry['menu_item_link']['me_original_function'] = $theme_registry['menu_item_link']['function'];
    $theme_registry['menu_item_link']['function'] = 'me_theme_menu_item_link';
  }
}
?>

and

<?php
function me_theme_menu_item_link($link) {
  static $_functions = array();

  $theme = $GLOBALS['theme'];

  if (!isset($_functions[$theme])) {
    $registry = theme_get_registry();
    if (function_exists($candidate = $registry['menu_item_link']['me_original_function'])) {
      $function = $candidate;
    }
    else if (isset($GLOBALS['theme_info'])) {
      $themes = $GLOBALS['base_theme_info'];
      $themes[] = $GLOBALS['theme_info'];
      foreach (array_reverse($themes) as $theme_info) {
        $candidate = $theme_info->name .'_menu_item_link';
        if (function_exists($candidate)) {
          $function = $candidate;
          break;
        }
      }
    }
    if (isset($function)) {
      // do nothing
    }
    elseif (isset($GLOBALS['theme_key']) && function_exists($candidate = $GLOBALS['theme_key'] .'_menu_item_link')) {
      $function = $candidate;
    }
    elseif (isset($GLOBALS['theme_engine']) && function_exists($candidate = $GLOBALS['theme_engine'] .'_menu_item_link')) {
      $function = $candidate;
    }
    elseif (function_exists('theme_menu_item_link')) {
      $function = 'theme_menu_item_link';
    }
    $_functions[$theme] = $function;
  }

  if (isset($_functions[$theme]) && $_functions[$theme] !== __FUNCTION__) {
    _me_check_path($link);
    $function = $_functions[$theme];
    return $function($link);
  }
  else {
    // This should never happen.
    return __FUNCTION__;
  }
}
?>

And yes, I am too lazy for a real patch (again) ..

donquixote’s picture

The important parts:
- There is more than one theme registry: One for every theme.
- Instead of the variable_set / variable_get, we can store the value in the theme registry itself. We attempt to use a unique key ("me_original_function"), to avoid name collisions.
- We check for parent themes, in addition to the active theme.

donquixote’s picture

StatusFileSize
new2.59 KB
new1.75 KB

I noticed that the fallback logic has been removed, so I produced two patches: One with the fallback code ("big"), and one without ("small").

donquixote’s picture

StatusFileSize
new2.55 KB
new3.35 KB

I think I was looking at the wrong module version.
Here are the same patches against 2.x-dev.

I think "big" is the one we need.

donquixote’s picture

Status: Active » Needs review
donquixote’s picture

Any chance of getting this in?
It is quite unconvenient having to maintain a patched module.

nohup’s picture

Status: Needs review » Fixed

committed to git and released, thanks donquixote.

donquixote’s picture

Thanks!
Which version did you choose?
(it's some time ago, I don't really remember what is the difference)

nohup’s picture

6.x-2.x dev the release is 6.x-2.8. I goofed up the git commit settings so it won't show up in my commit stream but the code is there.

donquixote’s picture

Yeah, my question was rather, which version of the patch.

nohup’s picture

"big"

Status: Fixed » Closed (fixed)

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