hook_block op view looks like this:

        $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:

          'content' => theme('google_admanager_block', $id, $ad_slot),
CommentFileSizeAuthor
#5 google_admanager-theme.patch1013 bytestoemaz
#1 google_admanager-fix-theme-function-530300-1.patch765 byteswebchick

Comments

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Active » Needs review
StatusFileSize
new google_admanager-fix-theme-function-530300-1.patch765 bytes

Here's a patch.

frankcarey’s picture

frankcarey commented
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

frankcarey’s picture

frankcarey commented

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?

frankcarey’s picture

frankcarey commented
Status: Reviewed & tested by the community » Needs work

oops, changing the status needs work

toemaz’s picture

toemaz commented
Status: Needs work » Needs review
StatusFileSize
new google_admanager-theme.patch1013 bytes

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

toemaz’s picture

toemaz commented

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

jaydub’s picture

jaydub commented
Status: Needs review » Reviewed & tested by the community

patch looks good here.

toemaz’s picture

toemaz commented
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.

Status: Fixed » Closed (fixed)

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