Follow-up from #241570-68: Theme preprocess functions do not get retained when using patterns.
hook_theme() allows for specifying a 'file' key, so that a default theme function and preprocess functions can reside in a separate include file. For example, user_theme() does this for 'user_profile', allowing template_preprocess_user_profile() to reside in user.pages.inc.
We also allow for theme hook suggestions to be specified as a #theme value. For example, a module or theme can implement hook_user_view_alter(), and change $build['#theme'] from 'user_profile' to 'user_profile__' . $account->uid
. Doing so would, for example, allow a theme to implement a user-profile--1.tpl.php
in order to customize the display of the root user's profile.
In #241570: Theme preprocess functions do not get retained when using patterns, we made sure that the preprocess functions for the base hook (e.g., 'user_profile') are still called, even when the theme implements a specific suggestion override (e.g., user-profile--1.tpl.php).
However, there is currently a bug in theme() where the file in which those base preprocess functions are located isn't included, thereby, the preprocess functions fail to get called unless that file happened to be included elsewhere.
#4 contains a test that demonstrates this by changing the original test from #241570: Theme preprocess functions do not get retained when using patterns from testing 'breadcrumb' to testing a new test theme hook.
Comment | File | Size | Author |
---|---|---|---|
#20 | 1430300-20.patch | 5.33 KB | NROTC_Webmaster |
#16 | 1430300-16.patch | 4.75 KB | no_commit_credit |
#16 | interdiff.txt | 992 bytes | no_commit_credit |
#13 | 1430300-10.patch | 4.75 KB | no_commit_credit |
#13 | interdiff.txt | 1.13 KB | no_commit_credit |
Comments
Comment #1
mradcliffeThis works for me in D7. I don't have a D8 install to test on.
Comment #2
mradcliffeOh, dangit. I didn't format patch on the right branch :(
Here's just a standard git diff patch for d7.
Comment #3
mradcliffeNope. I am an idiot sometimes. Sorry for spam.
Comment #4
effulgentsia CreditAttribution: effulgentsia commentedThanks for the find and fix! Here's a test-only patch that demonstrates the bug, and a patch with that test plus the fix in #1 with a minor code comment tweak. Adding the backport tag so we remember to commit this to 7.x too, though the actual backport work might just be to remove 'core/' from the patch file.
Comment #5
effulgentsia CreditAttribution: effulgentsia commentedComment #5.0
effulgentsia CreditAttribution: effulgentsia commentedUpdated issue summary.
Comment #6
sunThanks, looks good to me.
Comment #7
Lars Toomre CreditAttribution: Lars Toomre commentedThis documentation block needs work.
The first line of a docblock should start with an active verb tense and the whole line should be less than 80 characters. If necessary include more in an additional comment (after a blank line).
Comment #8
sunAs of now, that doesn't really apply to test methods in test cases. Before we apply the phpDoc summary there, I'd like to see the existing phpDoc summaries getting fixed.
Comment #9
xjmWell, I wasn't assertive enough to NW a patch sun reviewed over a docs nit, but I did think the same thing and considered rerolling it before an incident distracted me yesterday. :) And having it over 80 characters is already inconsistent with all our docblocks everywhere.
Here is the existing issue to clean up the docblocks in this file:
#1431664: Clean up API docs for system/tests, J-Z, including subdirectories
The more correct we can make all patches currently going in, the less the cleanup patches will be reroll-inducing, baby-eating nightmares. So I'd disagree with postponing writing proper docs to cleanup issues.
Comment #10
xjmOh, I'll just do it. :)
Comment #11
mradcliffeThank you for the tests, effulgentsia. Those look good to me as well.
Comment #12
Lars Toomre CreditAttribution: Lars Toomre commentedThank you @xjm.
The comment in #8 left my nose a bit out of joint. Since when do we ever knowingly allow docblock lines to exceed 80 characters?? I can recall many patch comments where @sun himself would note a docblock line exceeded 80 characters.
Comment #13
no_commit_credit CreditAttribution: no_commit_credit commentedComment #14
xjm#11#13 just tweaks the docblocks a little. Trivial nitpicky comment change, so leaving the issue at RTBC. :) Thanks everyone!Comment #15
xjmComment #16
no_commit_credit CreditAttribution: no_commit_credit commentedComment #17
xjmThe above fixes a code style issue webchick spotted and also removes t() from the assertion message. References: http://drupal.org/simpletest-tutorial-drupal7#t and #500866: [META] remove t() from assert message.
Comment #18
catchThanks!
Committed/pushed to 8.x, moving back to 7.x for backport.
Comment #19
catchComment #20
NROTC_Webmaster CreditAttribution: NROTC_Webmaster commentedHere is the port
Comment #21
xjmBackport checks out.
Comment #22
webchickI initially got confused here about the fact that the theme.inc hunk references 'includes' but the test function adds 'file', however it looks like this is just the theme system being inconsistent with itself. xjm pointed out that http://qa.drupal.org/pifr/test/229313 shows the test failing without the patch.
Committed and pushed to 7.x. Thanks!
Comment #24
cweagansUpdating tags per http://drupal.org/node/1517250
Comment #24.0
cweagansUpdated issue summary.