Download & Extend

hook_registry_alter breaks themes

Project:me aliases
Version:6.x-2.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

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

#1

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.

#2

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.

#3

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) ..

#4

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.

#5

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

AttachmentSize
me.menu_item_link.small_.patch 1.75 KB
me.menu_item_link.big_.patch 2.59 KB

#6

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.

AttachmentSize
me.menu_item_link.small_.patch 2.55 KB
me.menu_item_link.big_.patch 3.35 KB

#7

Status:active» needs review

#8

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

#9

Status:needs review» fixed

committed to git and released, thanks donquixote.

#10

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

#11

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.

#12

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

#13

"big"

#14

Status:fixed» closed (fixed)

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

nobody click here