it do not work when that's following pattern,

theme_hook_suggestions:
$variables['theme_hook_suggestions'][] = name-name__name / name_name__name / name__name-name

tpl:
name-name--name ....etc..

following may not work:
[poll.module at line 772, column 17] $variables['theme_hook_suggestions'][] = 'poll_vote__block';
[poll.module at line 875, column 17] $variables['theme_hook_suggestions'][] = 'poll_results__block';
[poll.module at line 890, column 17] $variables['theme_hook_suggestions'][] = 'poll_bar__block';
[profile.module at line 593, column 17] $variables['theme_hook_suggestions'][] = 'profile_wrapper__' . $field;
[search.pages.inc at line 79, column 15] $variables['theme_hook_suggestions'][] = 'search_results__' . $variables['type'];
[search.pages.inc at line 114, column 15] $variables['theme_hook_suggestions'][] = 'search_result__' . $variables['type'];
[taxonomy.module at line 648, column 15] $variables['theme_hook_suggestions'][] = 'taxonomy-term__' . $vocabulary_name_css;
[taxonomy.module at line 649, column 15] $variables['theme_hook_suggestions'][] = 'taxonomy-term__' . $term->tid;
[theme.inc at line 2466, column 15] $variables['theme_hook_suggestions'][] = 'region__' . $region;

also, I think we should patch it 

function template_preprocess_region(&$variables) {
  // Create the $content variable that templates expect.
  $variables['content'] = $variables['elements']['#children'];
  $variables['region'] = $variables['elements']['#region'];

  $region = drupal_region_class($variables['region']);
  $variables['classes_array'][] = $region;
  $variables['theme_hook_suggestions'][] = 'region__' . $region; // region__region-sidebar-first
}

to 

function template_preprocess_region(&$variables) {
  // Create the $content variable that templates expect.
  $variables['content'] = $variables['elements']['#children'];
  $variables['region'] = $variables['elements']['#region'];

  $region = $variables['region']);
  $variables['classes_array'][] = drupal_region_class($region);
  $variables['theme_hook_suggestions'][] = 'region__' . $region;  // region__sidebar_first
}
CommentFileSizeAuthor
#10 region.overrides.patch1.96 KBAnonymous (not verified)
#9 taxonomy_term.patch1.26 KBdroplet
#9 region.patch829 bytesdroplet

Comments

Anonymous’s picture

Anonymous (not verified) commented

can you elaborate on what you mean by 'it do not work'? modules can't override core?

droplet’s picture

droplet commented
Title: some modules templates do not work (theme_hook_suggestions) » modules theme override broken (theme_hook_suggestions)

Yes. We cannot overriding themes when using above theme_hook_suggestions pattern.

theme_hook_suggestions now only work with :
$variables['theme_hook_suggestions'][] = name__name_name / name__name__name / name__name__name_name

Anonymous’s picture

Anonymous (not verified) commented

ok, can you help us write a test case that fails? say one that creates a test module that tries to override a theme template but fails?

that will help a lot with making a patch to fix this.

droplet’s picture

droplet commented

sure if I can but do not how to write it against drupal QA.

I make a mistake above. name_name__name is works.

so any pattern with "-" and like this poll_results__block_block is not working.

effulgentsia’s picture

effulgentsia commented
Component: base system » theme system
Issue tags: +Needs tests

template_preprocess_region() is indeed broken. Values for 'theme_hook_suggestions' need to contain underscores, not hyphens. So the pattern is that $variables['theme_hook_suggestions'][] needs to be set to something like 'region__sidebar_first'. This will allow a theme to implement a 'region--sidebar-first.tpl.php' file, but the underscore to hyphen conversions happens only when converting a suggestion to a template file name. Passing the output of drupal_html_class() to a 'theme_hook_suggestions' assignment is a bug, and makes the suggestion not work, so region-specific template files aren't being used.

Writing a patch to fix the bug in template_preprocess_region() is easy. Adding a test to that patch to make sure a theme's "region--sidebar-first.tpl.php" file gets used is a little harder, but do-able. Anyone up for trying it?

effulgentsia’s picture

effulgentsia commented
Title: modules theme override broken (theme_hook_suggestions) » Some region-specific overrides of region.tpl.php don't get used.

.

effulgentsia’s picture

effulgentsia commented
Priority: Critical » Normal

Not critical, but definitely is a bug worth fixing.

droplet’s picture

droplet commented

OK. 2 quick patches.

something like 'name__name_name' require fixing from API side.

droplet’s picture

droplet commented
StatusFileSize
new region.patch829 bytes
new taxonomy_term.patch1.26 KB
Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Needs review
StatusFileSize
new region.overrides.patch1.96 KB

droplet, thanks for the patches! have a look at this link to see the best way to create them for drupal: http://drupal.org/patch

i've rerolled them into a single patch, and changed the status to 'needs review' so the testbot will pick it up.

effulgentsia’s picture

effulgentsia commented
Status: Needs review » Reviewed & tested by the community

Patch looks great. Thanks! I'm RTBC'ing it in case Dries or webchick are willing to commit this without tests. Tests are needed, but honestly, lots of things in the theme system are still missing adequate test coverage, so perhaps, they'd be willing to let theme system test coverage be handled in separate issues.

marcingy’s picture

marcingy commented
Priority: Normal » Major

Changing to major as per tag.

mustanggb’s picture

mustanggb commented

Tag update

dries’s picture

dries commented
Issue tags: -Needs tests

#10: region.overrides.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

The last submitted patch, region.overrides.patch, failed testing.

bojanz’s picture

bojanz commented
Status: Needs work » Fixed

#11 was actually committed (wanted to do a reroll and saw the changes already there).

effulgentsia’s picture

effulgentsia commented
Title: Some region-specific overrides of region.tpl.php don't get used. » Need tests for region-specific overrides of region.tpl.php
Status: Fixed » Needs work

Still needs tests though.

effulgentsia’s picture

effulgentsia commented
Priority: Major » Normal

Adding tests is always worthwhile, but not "major".

effulgentsia’s picture

effulgentsia commented
Category: bug » task

Ack. Sorry for taking 3 comments to update all the issue fields.

droplet’s picture

droplet commented

recall my memory,
my patch only fixed "name__name_name",
patterns like "poll_results__block_block" not solve yet.

I can't roll out a patch until get some more info. So what name patterns we can use, anyone could confirm it ?

Thanks.

effulgentsia’s picture

effulgentsia commented

Thanks, @droplet. Please see #5. $variables['theme_hook_suggestions'] should only ever contain "machine names" (i.e., lowercase letters, numbers, underscores). It should never contain hyphens. Underscore to hyphen conversion happens only when translating a theme hook suggestion to a template file name, but theme hooks are indexed in the theme registry by machine name, so hyphens in theme_hook_suggestions are always wrong.

effulgentsia’s picture

effulgentsia commented

By the way, I don't see poll_results__block_block anywhere in HEAD, but if it were there, it wouldn't be wrong per se, just perhaps a poor name choice simply because it wouldn't be clear what purpose the 2nd 'block' serves. poll_results__block is fine and makes sense (i.e., a variant of poll_results, specific to when viewed in a block). Wherever you see the double underscore, it means the thing is a variant of whatever comes before the double underscore, and in a few cases, you can even find multiple double underscores, like 'field__' . $element['#field_name'] . '__' . $element['#bundle'], which allows for a bundle-specific variant of 'field__' . $element['#field_name'], which itself is a field-specific variant of 'field'.

droplet’s picture

droplet commented

Im confused.

if I set that to follow should work well in expectation ??
$variables['theme_hook_suggestions'][] = 'region_sth__' . $variables['region'];

print_r($theme_hook_suggestions);
Array ( [0] => region_sth__sidebar_first )

However, I use "region-sth--sidebar-first.tpl.php" to test and FAILED

so is it the bug, right ??

./modules/taxonomy/taxonomy.module: $variables['theme_hook_suggestions'][] = 'taxonomy_term__' . $term->vocabulary_machine_name;
./modules/taxonomy/taxonomy.module: $variables['theme_hook_suggestions'][] = 'taxonomy_term__' . $term->tid;
./modules/poll/poll.module: $variables['theme_hook_suggestions'][] = 'poll_vote__block';
./modules/poll/poll.module: $variables['theme_hook_suggestions'][] = 'poll_results__block';
./modules/poll/poll.module: $variables['theme_hook_suggestions'][] = 'poll_bar__block';
./modules/search/search.pages.inc: $variables['theme_hook_suggestions'][] = 'search_results__' . $variables['module'];
./modules/search/search.pages.inc: $variables['theme_hook_suggestions'][] = 'search_result__' . $variables['module'];
...
..

effulgentsia’s picture

effulgentsia commented

Ah, this might help answer the confusion. The double-underscore is there to delimit a more specific version of a "base" theme hook. So, for example, "node__article" is a more specific version of the "node" theme hook, and therefore, "node--article.tpl.php", if it's there, gets used as the template file instead of "node.tpl.php" for article nodes.

'region_sth__' . $variables['region'] would make sense if there was a base theme hook of 'region_sth'. You could create a module that implements hook_theme() to define a 'region_sth' theme hook, but without that base theme hook being defined, 'region-sth--*.tpl.php' files don't get found by the theme system.

Note that 'taxonomy_term', 'poll_vote', 'poll_results', 'poll_bar', and 'search_results' are all examples of theme hooks defined by some module's hook_theme() implementation, so double-underscore after any of these makes sense. In fact it is precisely, because base theme hooks can and often do consist of more than one word, that in D7, we started using "__" as the "suggestion" delimiter to avoid name conflicts with "_" as merely a word delimiter. As an example, 'block-admin-display-form.tpl.php' is its own base template file for theming the admin/structure/block table, whereas, if you had a module called 'admin_display_form.module', then a theme could have a 'block--admin-display-form.tpl.php' to override block.tpl.php for those module's blocks only, and this template file name wouldn't conflict with 'block-admin-display-form.tpl.php'.

droplet’s picture

droplet commented

thanks effulgentsia, your reply is very helpful. I learn lots stuff from this issue :). thanks again.

_________’s picture

_________ commented

Just found this thread in the bug system ...

I'm having problems with some (related to this?) region-specific overrides working, and others not. I'll just mention the bug post here: http://drupal.org/node/1200498, which references my initial forum chat.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.