By calling theme inside the theming functions, the preprocess functions is called twice, resulting in things like duplicate classes.

Attached patch borrows menu_item_containers way of doing it.

CommentFileSizeAuthor
#6 special_menu_items-call_theme_once-1447864-6.patch600 bytesmarcvangend
#2 1447864-2-preprocess_called_two_time.patch524 bytesgagarine
special_menu_items-theme.patch1.47 KBXen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gagarine’s picture

gagarine CreditAttribution: gagarine commented
Version: 7.x-1.0 » 7.x-2.x-dev
Status: Active » Postponed (maintainer needs more info)

I don't have duplicate classes. Can you please explain how to reproduce?

gagarine’s picture

gagarine CreditAttribution: gagarine commented
FileSize
1447864-2-preprocess_called_two_time.patch524 bytes

Reroll. But before commit this, I want to be sure it's necessary.

Xen’s picture

Xen CreditAttribution: Xen commented

As far as I remember (it's two finished projects in the past):

If you have a theme or module that imlements hook_preprocess_menu_link that adds classes to links like so:

$vars['element']['#attributes']['class'][] = 'another-class';

You'll end up with duplicate classes, but that's only one possible side effect, other modules might do other things that results in misbehavior when the preprocess functions is called multiple times. Apart from the fact it's just messy and unperforming to run the same code multiple times, even if it is idempotent.

gagarine’s picture

gagarine CreditAttribution: gagarine commented
Status: Postponed (maintainer needs more info) » Active
mvc’s picture

mvc
they
Primary language English
Location Montréal, CA
CreditAttribution: mvc commented

uh, shouldn't the patch in #2 remove the line return theme('special_menu_items_link_default', $variables); before trying to return something else?

marcvangend’s picture

marcvangend
he/him
Primary language Dutch
Location Amsterdam
CreditAttribution: marcvangend commented
Status: Active » Needs review
FileSize
special_menu_items-call_theme_once-1447864-6.patch600 bytes

mvc, you're right.

gagarine, steps to reproduce:
1) Add a preprocess function to your theme:

function THEMENAME_preprocess_link(&$vars) {
  $vars['options']['attributes']['class'][] = 'foo';
}

2) Clear cache and refresh the page.
3) Check your output and see the 'foo' class was added twice.

Here is a reroll which includes the fix for #5. Works great for me.

rossb89’s picture

rossb89 CreditAttribution: rossb89 at ComputerMinds commented
Issue summary: View changes

Just found this issue... a 5 year old patch still works, brilliant!
This should definitely be committed. I ended up with some classes added 6 times due to multiple contrib modules and a theme function preprocessing the link.

rossb89’s picture

rossb89 CreditAttribution: rossb89 at ComputerMinds commented
Status: Needs review » Reviewed & tested by the community

  • gagarine committed bc41aa9 on 7.x-2.x 
    Fix #1447864 by marcvangend, Xen: Nested call to theme() causes...
gagarine’s picture

gagarine CreditAttribution: gagarine as a volunteer commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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