Problem/Motivation

The hook_theme_suggestions_HOOK() documentation states:

This hook allows the module implementing hook_theme() for a theme hook to provide alternative theme function or template name suggestions. This hook is only invoked for the first module implementing hook_theme() for a theme hook.

However this is the code:

// Invoke hook_theme_suggestions_HOOK().
$suggestions = $this->moduleHandler->invokeAll('theme_suggestions_' . $base_theme_hook, array($variables));

Proposed resolution

Update the documentation to indicate that this hook is invoked for all modules.

Remaining tasks

  • Patch
  • Review

User interface changes

n/a

API changes

n/a

Files: 
CommentFileSizeAuthor
#18 interdiff.txt996 byteswadmiraal
#18 drupal_hook_theme_suggestions_HOOK-documentation_2430909_18.patch1.16 KBwadmiraal
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,387 pass(es). View
#11 2430909_hook_theme_suggestions_10.patch1.54 KBharshil.maradiya
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2430909_hook_theme_suggestions_10.patch. Unable to apply patch. See the log in the details link for more information. View
#9 hook_theme_suggestion_correction-2430909-9.patch879 bytesjoshi.rohit100
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,293 pass(es). View
#7 hook_theme_suggestion_correction-2430909-7.patch739 bytesanwar_max
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,226 pass(es). View
#5 hook_theme_suggestion_correction-2430909-5.patch0 bytesanwar_max
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,224 pass(es). View
#2 hook_theme_suggestion_correction-2430909-2.patch739 bytesTheJustinRhodes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,294 pass(es). View

Comments

Berdir’s picture

TheJustinRhodes’s picture

Status: Active » Needs review
FileSize
739 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,294 pass(es). View

Here's a patch that updates the documentation:

Cottser’s picture

Status: Needs review » Needs work

Thanks @ClientGuy :)

+++ b/core/modules/system/theme.api.php
@@ -492,7 +492,7 @@ function hook_preprocess_HOOK(&$variables) {
  * This hook allows the module implementing hook_theme() for a theme hook to
  * provide alternative theme function or template name suggestions. This hook is
- * only invoked for the first module implementing hook_theme() for a theme hook.
+ * invoked for all modules implementing hook_theme() for a theme hook.

The hook is invoked for all modules, period. So this whole paragraph should be updated, something like this might do it:

"This hook allows modules to provide alternative theme function or template name suggestions."

anwar_max’s picture

Assigned: Unassigned » anwar_max
anwar_max’s picture

Status: Needs work » Needs review
FileSize
0 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,224 pass(es). View

As per #3 changes have been made to the patch.

anwar_max’s picture

Assigned: anwar_max » Unassigned
anwar_max’s picture

FileSize
739 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,226 pass(es). View

Uploaded empty patch. Re-uploading the patch.

Berdir’s picture

Status: Needs review » Needs work

That still doesn't make sense, the part hook_theme() needs to go. As @Cottser said, it is called for all modules. full stop.

joshi.rohit100’s picture

Status: Needs work » Needs review
FileSize
879 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,293 pass(es). View

updated patch.

Cottser’s picture

Status: Needs review » Needs work

Much closer, thanks!

  1. +++ b/core/modules/system/theme.api.php
    @@ -490,9 +490,8 @@ function hook_preprocess_HOOK(&$variables) {
    + * This hook allows modules to provide alternative theme function or template
    + * name suggestions. This hook is invoked for all modules.
    

    Arguably "This hook is invoked for all modules" is the default and therefore redundant, but I'm fine either way on that. Might be worth looking around at other *.api.php's as an example.

  2. +++ b/core/modules/system/theme.api.php
    @@ -490,9 +490,8 @@ function hook_preprocess_HOOK(&$variables) {
      * HOOK is the least-specific version of the hook being called. For example, if
      * '#theme' => 'node__article' is called, then node_theme_suggestions_node()
    

    This part actually should be updated too I think, because it seems to reflect the previous paragraph where only the module implementing hook_theme() would get the hook invocation.

    Here's the full couple sentences:

    HOOK is the least-specific version of the hook being called. For example, if '#theme' => 'node__article' is called, then node_theme_suggestions_node() will be invoked, not node_theme_suggestions_node__article().

harshil.maradiya’s picture

FileSize
1.54 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2430909_hook_theme_suggestions_10.patch. Unable to apply patch. See the log in the details link for more information. View

Updated

harshil.maradiya’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 11: 2430909_hook_theme_suggestions_10.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 11: 2430909_hook_theme_suggestions_10.patch, failed testing.

Cottser’s picture

Title: hook_theme_suggestions_HOOK() documentation is incorrect about how the hook is invoked » hook_theme_suggestions() hook_theme_suggestions_HOOK() documentation is incorrect about how the hook is invoked

@harshil.maradiya - Patch is corrupt according to git, missing newline at EOF I think.

All my comments from #10 still apply. I don't think we should change the docs for hook_theme_suggestions_HOOK_alter() in this issue. Can we go back to the patch from #9 please and provide interdiffs when making changes to patches? Thanks all! :)

wadmiraal’s picture

Assigned: Unassigned » wadmiraal

I'll get it.

wadmiraal’s picture

FileSize
1.16 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,387 pass(es). View
996 bytes

Updated documentation. I checked other *.api.php files: they don't mention "this is invoked for all modules" (it is kind of implicit).

wadmiraal’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Looks great to me, thanks!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed c9a38f1 and pushed to 8.0.x. Thanks!

  • alexpott committed c9a38f1 on 8.0.x
    Issue #2430909 by anwar_max, wadmiraal, harshil.maradiya, joshi.rohit100...

Status: Fixed » Closed (fixed)

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