Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
"hook_process()? What's that?"
That's what you get when template_process() was added to the theme system. This function, combined with the fact that modules may implement preprocess hooks, means that some unsuspecting modules will now be doing awful, awful things to the template variables.
template_process() is implied to exist at http://drupal.org/node/254940#variables-processor, but there is no indication that module maintainers need to check their function names for the word "process".
Comment | File | Size | Author |
---|---|---|---|
#15 | 736326-otherfix.patch | 16.04 KB | jhodgdon |
#11 | 736326-smallfix.patch | 16.04 KB | jhodgdon |
#9 | 736326_8.patch | 16.06 KB | aspilicious |
#6 | 736326.patch | 16.06 KB | jhodgdon |
Comments
Comment #1
jhodgdonWow. Good catch!
Looking at http://api.drupal.org/api/function/theme/7, it looks like for modules there is hook_process() and hook_preprocess(). Neither of these is documented in Drupal 7, and both should be.
I've just added these to the critical catch all issue for hooks in D7: #675046: Make sure all hooks in D7 have documentation
Comment #2
jhodgdonSomeone needs to look through theme() and make sure that the doc at the top of it is really correct, as well as adding doc for hook_process() and hook_preprocess() if those are actually invoked. The code is not actually clear to me...
Comment #3
JohnAlbinAgreed. modules/system/theme.api.php needs to add hook_preprocess, hook_preprocess_HOOK, hook_process, hook_process_HOOK docs.
And it also needs to be added to the module upgrade docs.
Comment #4
jhodgdonI'm taking this on...
Comment #5
jhodgdonHere's a patch. It fixes up the theme() documentation, and adds the four needed hook docs.
Comment #6
jhodgdonIt might help if I attached the patch.
Comment #7
jhodgdonNote: If accepted, we also need to add something to the module update docs. JohnAlbin: I'm not sure what we need to add though? What of this is new to D7, just these process/preprocess hooks?
Comment #8
aspilicious CreditAttribution: aspilicious commentedtrailing white space
Powered by Dreditor.
Comment #9
aspilicious CreditAttribution: aspilicious commentedfixed the trailing white space
Comment #10
aspilicious CreditAttribution: aspilicious commentedI think something is wrong with the intendation
Maybe a newline between the else ad the if statement?
No other remarks
90 critical left. Go review some!
Comment #11
jhodgdonI don't see a problem with the indentation in the first part there. That second line you referenced is inside an if() {}
???
Added the newline in your second spot.
Comment #12
jhodgdonComment #13
aspilicious CreditAttribution: aspilicious commentedI replaced spaces with '-'.
As you can see there is one space before the keyword 'if' and two before '}'.
You always need two spaces.
BTW: the last sentence in my first code block sneeked into my review, wasn't supppsed to happen
You see the indentation problem better in the patch (when you see more of the function)
Comment #14
aspilicious CreditAttribution: aspilicious commentedComment #15
jhodgdonAh, missed that. Here's a fixed patch.
Comment #16
aspilicious CreditAttribution: aspilicious commentedRTBC
Bot can turn it to red if he wants ;)
Comment #17
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #19
effulgentsia CreditAttribution: effulgentsia commented#15 introduced some inaccuracies by using the phrase "all theme hooks" to refer to the execution of the non-theme-hook-specific preprocess() and process() functions, as per below:
As stated elsewhere in the PHPDoc of theme(), due to performance constraints, these only run for theme hooks implemented as templates.
We're working on a fix for this in #1333122: Fix documentation in theme() and template_preprocess(), so leaving this issue as closed. Just cross-linking here for posterity.