If create menu block for example with name "test" and try to theming it, with file name block--menu--menu-test.tpl.php nothing happens. I mean specifically menu block theming not work.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mdupont’s picture

Confirmed. Custom menus will have a delta beggining with "menu-" (with a hyphen).

Due to this hyphen and the fact that the theme layer switches several times hyphens and underscores, the theme layer can't use the custom template. May be somewhat similar to #992376: Paths containing a hyphen aren't compatible with page theme_hook_suggestions.

After looking at includes/theme.inc and drupal_find_theme_templates() in particular, it appears that if we create a menu named "test" and a template named block--menu--menu-test.tpl.php, the theme layer will look for a block created by the menu module and with a delta of "menu_test" (block__menu__menu_test), whereas the menu block can effectively be found at block__menu__menu-test (notice the hyphen instead of the underscore). Therefore, the custom template isn't used. Embarrassing.

mdupont’s picture

Title: Block Theming » Blocks for custom menus are impossible to theme
Version: 7.0 » 7.x-dev
Component: block.module » theme system

Requalifying the issue for better description. Can a Drupal guru confirm if this is a serious DrupalWTF?

larowlan’s picture

Subscribe

larowlan’s picture

Status: Active » Needs review
FileSize
1.63 KB

Here's a patch for review

jhodgdon’s picture

Status: Needs review » Needs work

Can I suggest a different wording comment/doc portion of this patch, and also (a) don't use @see and (b) always put () after function names? My suggested doc:

Hyphens and underscores play a special role in theme suggestions. Basically, theme suggestions should only contain underscores, because within drupal_find_theme_templates(), underscores are converted to hyphens to match template file names, and then converted back to hyphens to match pre-processing and other function names. So if your theme suggestion contains a hyphen, it will end up as an underscore after this conversion, and your function names won't be recognized. So, we need to convert hyphens to underscores in block deltas for the theme suggestions.

And should we document this general rule somewhere else as well?

larowlan’s picture

Status: Needs work » Needs review
Issue tags: +Needs tests
FileSize
1.59 KB

Here it is again with Jennifer's comment
I think this should go in the converting drupal 6.x modules to 7.x doco.
Coder_upgrade can also test for this too.
Marking this as needs test too, we should be able to test for these as with the page one #992376: Paths containing a hyphen aren't compatible with page theme_hook_suggestions

markabur’s picture

Status: Needs review » Needs work

This should use strtr() -- see @sun's comment #2 here (similar issue): #890884: Targeted overrides for theme_menu_link() and theme_menu_tree() fail for menus with a hyphen

catch’s picture

Component: theme system » block.module

Also, nitpicking but can we remove "Basically, " from the second sentence, it's not necessary there.

vitok-dupe’s picture

patch from #6 works, thx!

larowlan’s picture

Status: Needs work » Needs review
FileSize
1.58 KB

And again with feedback from comments 7 and 8

jhodgdon’s picture

+1 on the doc portion of the patch anyway.

Regarding documenting this on the 6/7 module upgrade page, we already had:
http://drupal.org/update/modules/6/7#theme_hook_suggestions_1
http://drupal.org/update/modules/6/7#theme_hook_suggestions_2

I added an extra note similar to what's in this patch to the second section about the importance of using no hyphens in the pattern.

I also filed #1025528: Need documentation for the theme process/preprocess functions as a follow-up issue, because we need to have this documented on api.drupal.org as well, I think, and there's currently no place to document it.

markabur’s picture

#10 looks good, thanks.

pixelite’s picture

Status: Needs review » Reviewed & tested by the community

Have the same issue with "block--menu--primary-links.tpl.php".

This has nothing to do with menus, so perhaps the title of the issue should be "Blocks theme_hook_suggestions fails to clean-out dashes".

webchick’s picture

Status: Reviewed & tested by the community » Needs work

Let's add a test for this, so we don't break it again.

vitok-dupe’s picture

Excuse me, I maybe wrong to post it here but anyway. If in core D7 exist body_classes and other good stuff for theming, why not include page__type_..._.tpl.php templates? Now if need to theming pages specifically node type need to create mythemename__preprocess_page function with 2 line code in template.php:

function mythemename_preprocess_page(&$variables, $hook) {
	if (isset($variables['node'])) { 
    $variables['theme_hook_suggestions'][] = 'page__type__'. $variables['node']->type;
  }
}

why don't include this in core?

vitok-dupe’s picture

in core I think it's 1 line code in the theme.inc at the 2269 line.

  if ($node = menu_get_object()) {
    $variables['node'] = $node;
+    $variables['theme_hook_suggestions'][] = 'page__type__'. $variables['node']->type;
  }

  // Populate the page template suggestions.
  if ($suggestions = theme_get_suggestions(arg(), 'page')) {
+    $variables['theme_hook_suggestions'] += $suggestions;
  }
webchick’s picture

vitok, that sounds like it should be moved to a separate issue of its own. This issue is about fixing a bug specifically with custom menu block theming.

vitok-dupe’s picture

webchick: "vitok, that sounds like it should be moved to a separate issue of its own. This issue is about fixing a bug specifically with custom menu block theming."

Yes I know, I just think if you one of those people who have weight in drupal develop process, then I one of those people who gives you idea :)

devenderdagar’s picture

Status: Needs work » Needs review

#4: issue-992376.patch queued for re-testing.

DerekAhmedzai’s picture

I had the same problem with block templates for views.

I couldn't get block--views--view_name-block_name.tpl.php
or block--views--view_name_block_name.tpl.php to work.

Applying the patch from #10 fixed the problem.

jhodgdon’s picture

Status: Needs review » Needs work

The patch in #10 is apparently waiting for a test to be added, as per #14. Otherwise it is RTBC. The status got inadvertently changed by #19.

khiminrm’s picture

The patch from #10 fixed my problem with block--views--news_block_block_1.tpl.php.

star-szr’s picture

Subscribing. Wasted half a day trying to figure out why I couldn't override a views block.

Bill Pair’s picture

Subscribed as well! Damn, Drupal 7 is hard to theme if you're not a veteran...

birdonwire’s picture

had the same problem and i don't know how to apply the patch

Jo_’s picture

Subscribing - can't override views block...

Steven.Pescador’s picture

Applied the patch from #10 and is now working for me...

wojtha’s picture

Version: 7.x-dev » 8.x-dev
Issue tags: +Needs backport to D7

Bumping to 8.x

larowlan’s picture

should this be 8.x or should it stay at 7.x because it's a bug?

wojtha’s picture

It needs to be fixed in both D8 and D7, but in D8 first.

Jevedor’s picture

#10 fixes my problem in drupal 7. thanks for the patch

wojtha’s picture

Status: Needs work » Needs review

#10: issue-992376.patch queued for re-testing.

wojtha’s picture

Adding unit test.

larowlan’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Looks good to me, thanks wojtha

Jo_’s picture

#10 fixed my issue. Thanks.

dcrocks’s picture

pingers’s picture

+1 rtbc & backport it please.

404’s picture

ran into similar problem. Hope this get resolved asap in d7. it's holding back the themers.

subscribe.

sledan’s picture

How do you add the patch given in post #10 ?

jeffschuler’s picture

Patch in #34 works for me [though in D7].

webchick’s picture

Issue tags: +webchick approved

Awesome! Thanks for the test! It's really great to have coverage of these tricky theme details.

One question:

+  // drupal_find_theme_templates(), underscores are converted to hyphens to
+  // match template file names, and then converted back to hyphens to match
+  // pre-processing and other function names. So if your theme suggestion

Should "back to hyphens" not be "back to underscores"? This is something that can be fixed during commit (though a fresh patch wouldn't hurt).

wojtha’s picture

Ups. Just realized that Dries already commited it to Drupal 8. But with wrong description.

$ git show 79ebfd09
commit 79ebfd093ce29cbe73d298b2dab5be998cc1042d
Author: Dries Buytaert <dries@buytaert.net>
Date:   Sat May 21 20:10:25 2011 -0400

    - Patch #1013864 by agentrickard, JimmyAx: book navigation block fails with Node Access modules.

diff --git a/modules/block/block.module b/modules/block/block.module
index c5e229b..76311b1 100644
--- a/modules/block/block.module
+++ b/modules/block/block.module
@@ -943,7 +943,15 @@ function template_preprocess_block(&$variables) {

   $variables['theme_hook_suggestions'][] = 'block__' . $variables['block']->region;
   $variables['theme_hook_suggestions'][] = 'block__' . $variables['block']->module;
-  $variables['theme_hook_suggestions'][] = 'block__' . $variables['block']->module . '__' . $variable
+  // Hyphens (-) and underscores (_) play a special role in theme suggestions.
+  // Theme suggestions should only contain underscores, because within

etc... and at the and of the patch is finally:

@@ -279,10 +279,10 @@ function book_block_view($delta = '') {
   }
   elseif ($current_bid) {
     // Only display this block when the user is browsing a book.
-    $select = db_select('node');
-    $select->addField('node', 'title');
-    $select->condition('nid', $node->book['bid']);
-    $select->addTag('node_access');
+  $select = db_select('node', 'n')
+    ->fields('n', array('title'))
+    ->condition('nid', $node->book['bid'])
+    ->addTag('node_access');
     $title = $select->execute()->fetchField();
     // Only show the block if the user has view access for the top-level node.
     if ($title) {

It seems that Dries unwillingly merged two commits in to the one and he apparently didn't notice...

webchick’s picture

Priority: Major » Minor
Status: Reviewed & tested by the community » Needs work
Issue tags: -Needs backport to D7, -webchick approved

Nice catch!

Committed this to 7.x (with the docs typo fix) in order to keep the two in sync. This needs to go back to 8.x just for that minor typo.

larowlan’s picture

Status: Needs work » Needs review
FileSize
851 bytes

Fixes typo as suggested

wojtha’s picture

Status: Needs review » Reviewed & tested by the community

Should be commited to both D7 and D8.

m4olivei’s picture

I ran into this issue with the aggregator module in core, which names its block delta's 'feed-1', 'feed-2', etc. I can confirm this resolves the issue on the latest 7.x-dev. Thanks!

catch’s picture

Issue tags: +Needs backport to D7

tagging for backport.

larowlan’s picture

Pretty sure this is already in 7 - see 44

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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

waltf’s picture

is this supposed to be fixed or not?
I'm using drupal 7.8 and block--system--main--menu.tpl.php doesn't work.
Do i still need to apply the patch?

jeffschuler’s picture

waltf:
block--system--main-menu.tpl.php
Only one hyphen in main-menu.

marvzz’s picture

EDIT: Seems working on 7.10 but I need to name my menu block templates with underscores instead of dash for the blocks delta.

So if i have a menu named menu-my-menu, its block template should be block--menu--menu_my_menu.tpl.php

bcreeves’s picture

Version: 8.x-dev » 7.10

This is not working for me with a menu I created that has a URL path of: menu-footer-menu-2

I created a block--menu--menu_footer_menu_2.tpl.php which didn't work. I also tried it with underscores as dashes.

I'm using Drupal v7.10

Do I need to apply the patch?

apaderno’s picture

Version: 7.10 » 7.x-dev
Status: Closed (fixed) » Needs work

If it needs to be back-ported to Drupal 7, then its status should be "needs work."

star-szr’s picture

Status: Needs work » Closed (fixed)

I believe the fix has been commited to 7.x (#44) and 8.x (#50), so this can be closed.

spencerthayer’s picture

Version: 7.x-dev » 7.17
Priority: Minor » Major
Status: Closed (fixed) » Needs work

The patch is still needed with Drupal version 7.17. This issues IS NOT closed. Come on Drupal, WTF? Also the 1021270-block-suggestions-45.patch doesn't work for 7.17.

pingers’s picture

Version: 7.17 » 7.x-dev
Priority: Major » Minor
Status: Needs work » Closed (fixed)

This is already in D7 (as committed in May 2011).

@spencerthayer please read [#73179], [#73178] & [#45111].

Elijah Lynn’s picture