There seem to be some wrong declarations in simplenews' hook_theme implementation.
In 2394 - 2397 the path to the file containing the theme function is wrong. For this one I attach a patch.
The resulting warning that I saw only when trying to run update.php:

warning: include_once(sites/all/modules/simplenews/theme/includes/simplenews.admin.inc) [function.include-once]: failed to open stream: No such file or directory in /Users/ralf/Sites/Interactum/gk.interactum.de/includes/theme.inc on line 286.

In line 2374 - 2375 the simplenews_status theme uses a template and a file. I'm not sure if that is wrong, but it seems strange to me.
I post this for the dev version, but I see the problem also in the alpha2 version that I just tried to install.

CommentFileSizeAuthor
simplenews-6.x-theme-declarations.patch496 bytesberliner

Comments

simon georges’s picture

simon georges commented
Status: Active » Needs review

Just changing the status, as there is a patch attached.

simon georges’s picture

simon georges commented

@berliner, did you try to clear your theme registry ? There has been changes since the 6.x-1.x branch regarding the files of the theme.

berliner’s picture

berliner commented

Nothing to do with the cache IMO. If you look at the declaration for theme_simplenews_subscription_list:

'simplenews_subscription_list' => array(
      'file' => 'includes/simplenews.admin.inc',
      'arguments' => array('form' => NULL),
      'path' => $path . '/theme',
    ),

then you can see the the included file is simplenews.admin.inc which is located in simplenews/includes, but the given path for the inclusion is simplenews/theme, which results in simplenews/theme/includes/simplenews.admin.inc which generates the warning of my original post. I don't know though why this only happens when running update.php (at least I only saw it at that moment), but the declaration in itself seems wrong.

Another thing concerns the theme declaration for theme_simplenews_status. No path has been given where the template file should be searched making it probably harder for themes to use their own copy of the template. I'm not sure about that (and I have not the time to test unfortunately) but my experience tells me to always use the path attribute when you want a maximum of flexibility.

simon georges’s picture

simon georges commented

Can somebody else confirm this?

joel_guesclin’s picture

joel_guesclin commented

I get this error when installing the module

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Change looks good to me.

This theme function does not exist in 7.x-1.x but here is a similar one from the 7.x-1.x branch:

    'simplenews_status' => array(
      'file' => 'simplenews.admin.inc',
      'path' => $path  . '/includes',
      'variables' => array(
        'source' => NULL,
        'status' => NULL,
      ),
    ),
simon georges’s picture

simon georges commented
Status: Reviewed & tested by the community » Fixed

Fixed in 6.x-2.x-dev. Thanks!

Status: Fixed » Closed (fixed)

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