I was working on a theme (quite a pain for a not-quite theme developer) and basing that theme in zen. Zen uses the block_zebra and block_id variables defined in phptemplate.engine for setting special CSS classes for the blocks in regions. In particular, they set a counter class and zebra class for blocks within region plus one globally among all regions.
I was noticing that the classes were not properly set and I wondered why. The actual problem was that the count between regions was the same as the global one. While looking into phptemplate.engine code, I noticed that there's a global variable $sidebar_indicator that wasn't being set, so the code that increments the counter per region was always incrementing an unset region.
Attached is a patch that sets that global variable properly before calling theme('blocks, $region) function. Looking a little bit more on the file I am almost sure that some code just below that set this variable is not used at all, so probably left in some improvement to this file. I just added one line of code in the patch.
Hopefully this works and gets through 5.10?
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 292538-sidebar-indicator.patch | 1.47 KB | damien tournoud |
| phptemplate.engine.patch | 431 bytes | hanoii |
Comments
Comment #1
hanoiiWhile reading #281042: regression in 5.8 from 5.7 theme('blocks','all') broken - so no header alteration possible I noticed that this problem was introduced by the patch posted in that issue so leaving the relationship on this comment.
My patch should be fairly simple and inline with the what was fixed on the previous patch committed.
Comment #2
hanoiiComment #3
damien tournoud commentedThis is indeed a regression introduced by #281042: regression in 5.8 from 5.7 theme('blocks','all') broken - so no header alteration possible.
Here is a better patch, that keeps the previous behavior ($sidebar_indicator is only used for the left and right sidebars).
Comment #4
damien tournoud commentedAnd that's better with a patch.
Comment #5
hanoiiI saw your modified patch and everything looks right except why only pass $region if is only left or right?
I would like and I would think that's proper to have the block_id and block_zebra variables properly set on any region, why only limit $sidebar_indicator for left or right? Now, i would think this might be a semantic issue, rather than name it $sidebar_indication would be better to be $region_indicator. I have checked the phptemplate.engine file and that variable is only used for the block_id and block_zebra variables, and those variables should be available on every region.
Comment #6
damien tournoud commented@hanoii, because that's the behavior of all Drupal 5.x before 5.9 [1]
Drupal 5.x is feature frozen, so changing its behavior is completely out of question. The patch in #4 only reverts that previous behavior.
[1] The CVS history doesn't lie: http://cvs.drupal.org/viewvc.py/drupal/drupal/themes/engines/phptemplate...
Comment #7
hanoiiI see. It's a very tiny little feature anyway :).
So far I only need this for my 'left' sidebar, so I'll be able to stick to 5.10 release when it's out and if this patch gets committed.
Thanks.
Comment #8
damien tournoud commented@hanoii: Agreed, but it's because a small "feature" got overlooked that this issue appeared on the first place. I don't want someone else to complain that the behavior changed.
Comment #9
drummCommitted to 5.x.
Comment #10
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.
Comment #11
vtemor commentedI just upgraded from 5.9-5.10, and I still had to use the phptemplate.engine file from version 5.8 because the blocks on the home page didn't show up. Any one else still encountering this problem? Why didn't this patch get included in 5.10?
Comment #12
drumm#4 was included in 5.10, http://cvs.drupal.org/viewvc.py/drupal/drupal/themes/engines/phptemplate.... Can you paste the $Id line of your themes/engines/phptemplate.engine on the second line of the file.
Comment #13
dvessel commented