Theme function called incorrectly

webchick - July 24, 2009 - 21:02
Project:Google Ad Manager
Version:6.x-1.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed
Description

hook_block op view looks like this:

<?php
        $block
= array(
         
'subject' => '',
         
'content' => theme_google_admanager_block($id, $ad_slot),
        );
?>

When you call a theme function directly, rather than going through theme('...'), it cannot be overridden through the theme system.

This line should be changed to:

<?php
         
'content' => theme('google_admanager_block', $id, $ad_slot),
?>

#1

webchick - July 24, 2009 - 21:03
Status:active» needs review

Here's a patch.

AttachmentSize
google_admanager-fix-theme-function-530300-1.patch 765 bytes

#2

frankcarey - October 15, 2009 - 04:57
Status:needs review» reviewed & tested by the community

For some reason i got a rejection on this patch. Probably because i already patched it with the block cache patch webchick also submitted. I hand edited the file with the same change and it works correctly. marking RTBC

#3

frankcarey - October 15, 2009 - 05:07

correction, this is wrong. the module doesn't implement hook_theme, so though they are called theme_google_admanager_block(), they aren't going through the theme system. @webchick, do you have a patch that includes the theme function registrations?

#4

frankcarey - October 15, 2009 - 05:07
Status:reviewed & tested by the community» needs work

oops, changing the status needs work

#5

toemaz - October 26, 2009 - 09:35
Status:needs work» needs review

Find attached a new patch implementing the missing hook_theme. (update module against 6.x-1.x first before applying)

AttachmentSize
google_admanager-theme.patch 1013 bytes

#6

toemaz - October 31, 2009 - 17:27

Patch applied on 6-1 branch. Review requested with the 6.x-1.x-dev release.

#7

jaydub - November 12, 2009 - 00:29
Status:needs review» reviewed & tested by the community

patch looks good here.

#8

toemaz - November 30, 2009 - 11:46
Status:reviewed & tested by the community» fixed

Ok, I'll consider it fixed then since the patch was already applied to the CVS. If it's not ok yet, don't hesitate to reopen this issue.

#9

System Message - December 14, 2009 - 11:50
Status:fixed» closed

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

 
 

Drupal is a registered trademark of Dries Buytaert.