Project:Google Ad Manager
Version:6.x-1.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

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),
?>

Comments

#1

Status:active» needs review

Here's a patch.

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

#2

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

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

Status:reviewed & tested by the community» needs work

oops, changing the status needs work

#5

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

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

#7

Status:needs review» reviewed & tested by the community

patch looks good here.

#8

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

Status:fixed» closed (fixed)

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

nobody click here