Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sutharsan’s picture

clear the cache

BarisW’s picture

Too bad, for a second I was hoping that this would fix it. But no, still no luck.
I'm using the following modules:

auto_nodetitle
cck
date
filefield
front
i18n
imageapi
imagecache
imagefield
languageicons
lightbox2
markdown
mimemail
pathauto
simplemenu
simplenews
token
transliteration
views
webform
xmlsitemap

Could one of these modules be the problem?

ffej’s picture

Having the same problem here. Tried clearing the cache. I then suspected it might have to do with the Zen theme so I upgraded to the latest version of it as I noticed they had made changes to their theme registry code. I tried simplenews-newsletter-body.tpl.php and the more specific template with the term id, both with no luck. Upgraded to the dev version of simplenews. Even tried using a preprocess function in my template.php to override the body variable.

The only solution that worked for me was to drop my override directly into the preprocess function of the module. Not pretty but it works for now.

Any ideas would be appreciated. Other than this problem simplenews has been working really well.

rzegwaard’s picture

Had the same problem. Updating Drupal and Simplenews to the latest versions did the trick...

Robert

BarisW’s picture

I did as well, still not working.
Also the title override is not working.

This should be working, isn't it?

/**
 * Theme the newsletter email subject.
 */
function phptemplate_simplenews_newsletter_subject($name, $title, $language) {
  return  $title;
}
pieterbezuijen’s picture

I got stuck with the same issue. When sending my test mails, templates are working fine. As a last step, I tried sending the newsletter in de develop-environment to the complete list (with cron) and... no template was applied. After that, I tried sending without cron. This works like a charm.

@bariswanschers: This overide should be working, and is working in my setup, though without cron.

I found another post with template-issues in combination with cron: #364301: Cron does not use template files.
Maybe that will help?

Sutharsan’s picture

Status: Active » Closed (duplicate)
BarisW’s picture

Status: Closed (duplicate) » Active

Hi guys,

I'm not talking about errors while using the cronjobs. Even with test messages, no templates are being used.
Only the mail.css is added by Mimemail, but neither one of the template files is being used as well as the title override.

Any ideas?

Sutharsan’s picture

Status: Active » Closed (duplicate)

You probably experience the same problem when creating other template overrides (e.g. Views). At lease I do and I have not found a solution other than clearing cache and using the rebuild registry function of Zen theme (which clears the registry cache on each page load). This is a core thing and is by design. If you want to continue this discussion please use the issue in #4.

pixelenvy’s picture

I have the same problem.
Using latest versions of Drupal 6, Simplenews and mimemail and the templates in my theme directory are simply being ignored. I'm not using Zen and all other custom templates are working fine, just simplenews ones that aren't.

intrafusion’s picture

I have the same problem as #10, subscribing.

sonictruth’s picture

I can confirm this issue too. I have latest version of Drupal, simplenews, and mimemail.

What I have discovered is that simplenews IS using the tpl files in the module directory but ignoring the ones in my theme directory when I copy them there....and yes I have cleared the cache to rebuild the theme registry.

Im not sure what it is that forces Drupal to use the version of the tpl file in the theme instead of the one if the module but obviously this functionality is broken.

or it could be something completely different, but that's why we love drupal :)

BarisW’s picture

Version: 6.x-1.0-rc5 » 6.x-1.0-rc6
Priority: Normal » Critical

Any updates so far? It is still not working with the latest release..

This is the function I have in template.php, to remove the taxonomy-tag:

function looks_simplenews_newsletter_subject($name, $title, $language) {
  return $title;
}

Where looks is the name of my template. phptemplate_simplenems_newsletter_subject() doesn't work as well.

- I cleared the cache
- I re-actived my template.

Still nothing. And my issues regarding the usage of the .tpl files (eg simplenews-newsletter-body.tpl.php) still stands. They are not being called at all.

Please help, I really need this to work soon!

BarisW’s picture

Status: Closed (duplicate) » Active

Not a duplicate, please look into it

askibinski’s picture

I can confirm this problem. Neither clearing the cache nor using drupal_rebuild_theme_registry() helps.

BarisW’s picture

*kick*

Sutharsan’s picture

Another suggestion is to truncate the cache* tables (clear the table contents, not delete the tables). This is the most fundamental way to clear all caches.

apaderno’s picture

Title: Template files not working » Template files do not work
BarisW’s picture

Come on guys, does nobody knows how to solve this?
I issued this 2 months ago and still no fix. I wsh I knew how to fix this myself, but I don't.

Please help, I really need this to finish my project..

pieterbezuijen’s picture

I had issues with cron, and this was solved by adding the template files to the admin-theme. Maybe this will help?

kenorb’s picture

The same problem.
simplenews-newsletter-footer.tpl.php is working.
simplenews-newsletter-body.tpl.php doesn't
All those files are in theme dir.

djmabo’s picture

The same problem, but it's works for me when administration theme is not used for content editing (Use administration theme for content editing has to be unchecked). Drupal (or Simplenews module) is looking for the template files in the directory of the actual theme and it's in that time Administration theme.

NikLP’s picture

[subscribing]

BarisW’s picture

Hmmm, that could be the issue. I have Garland as the Admin interface, also for content editing and my own template for the design. I don't like to edit the Garland theme however, isn't this just a bug in the module?

gdkt11’s picture

Under Site Configuration > Administration Theme, I unchecked the box for "Use administration theme for content editing" and the template file didn't appear.

Then I went back and selected my theme instead of Garland as the Administration Theme. I sent the simplenews newsletter and my template file worked!

Thanks for the tips!

Katie

gunzip’s picture

ok but is there a way to use administration theme for content editing (ie. garland) AND have the simplenews templates in my custom theme at the same time ?

ccshannon’s picture

Exactly. An HTML newsletter should use the site's theme, NOT the admin theme. After all, who is the newsletter going to ... editors? No, it's going to the site's customers.

I'm about to develop a template for a Simplenews newsletter. If I find that it won't use the template in the site theme directory, I will hack Simplenews to get it to look in the proper directory, and report back here, unless, of course, a bug fix is issued before that.

ccshannon’s picture

Ha, turned out the newsletter design I received has its own layout, and doesn't need to inherit any theme elements from the site theme, so no need to worry about which theme folder the newsletter body/footer templates go into. I put them in admin theme folder and they work fine.

Still, it's good to know that newsletter templates should go into the admin theme folder. Once I did that, worked like a charm!

Jackinloadup’s picture

yeah i figured this out myself a while back.

Should we put forth the effort to make simplenews look the the "default" themes directory for the template?

ccshannon’s picture

It would be easier in terms of keeping templates organized, for sure. Not sure how it helps in terms of consistency of theming, given you probably want to theme a newsletter as self-standing HTML (no css or js or meta, etc) even when you do want it to look like the rest of the site.

Jackinloadup’s picture

@ccshannon

Thats a good point. Maybe newsletter templates should be kept in a separate location? This would make it easy to have multiple templates. If newsletter templates could be setup like themes it would be a double bonus.
Possibly in a location like sites/all/newstemplates or something. Using ".info" files to store the template information. I would love to expand on this idea.

Pros:
Templates would be easily maintainable and shareable when not mixed with the theme files.
Much easier to have multiple themes.

Cons:
Many code changes. Lots of patches.
possibly more difficult when you only need one template ever? is this common?

I have a multiple use-cases for something like this and would love to get my hands dirty and help get to a solution.

----------

I have tried the 6.x port of simplenews_template to solve this issue a while back, but it didnt work as expected. I thought templates would be file based but they were not.
#243567: New Drupal 6 release of Simplenews Template

kwinters’s picture

I can confirm it is also not working for me with the following configuration:

* D6.13
* Simplenews rc6
* Mimemail is disabled
* simplenews-newsletter-body.tpl.php customized and placed in the front-end theme
* Admin theme is Garland
* Preview works just fine
* Cron send uses the default template from the module folder

I suspect that this is a core bug since simplenews isn't doing any crazy theme hacks that I could find, but this is reported here because most modules don't use theme info during the cron.

At the moment, the best workaround appears to be pasting your custom template back in to your simplenews folder and overwriting the default file.

BarisW’s picture

Title: Template files do not work » Template files do not work when using admin theme
weseze’s picture

in reply to #30 and #31

I've made a very quick and very dirty hack to make this possible. It might completely destroy other functionality but if you only use HTML mail (MIME mail module) for SimpleNews, this hack is very effective!

Put your entire html (html tag, head tag, body tag, css, ... everything!) in simplenews-newsletter-body.tpl.php. Omit the body and html end tags and put them at the bottom of simplenews-newsletter-footer.tpl.php.

the body tpl should look like this:

<html>
<head>
<style type="text/css">
...
</style>
</head>
<body>
...

And the footer.tpl should look like this:

<?php if ($format == 'html'): ?>
  <p class="newsletter-footer"><a href="!confirm_unsubscribe_url"><?php print $unsubscribe_text ?></a></p>
<?php else: ?>
-- <?php print $unsubscribe_text ?>: !confirm_unsubscribe_url
<?php endif ?>

<?php if ($key == 'test'): ?>
- - - <?php print $test_message ?> - - -
<?php endif ?>
</body>
</html>

Then open up modules/mimemail/mimemail.module and comment out line 120:
// $body = theme('mimemail_message', $body, $mailkey);

This will make Drupal skip the entire theming proces of the node.

The result is that the content of your node will get parsed and replaced and so on, but Drupal will not theme the node as a page. Instead it will be outputted as is. And that's very good if you put the full HTML in the template.

This is a quick and dirty hack and might very well break other functionality! Use at your own risk.

Jackinloadup’s picture

+1 for not working with Simplenews and Views tpl overrides.

Note: for views in my case i was able to place the tpl file in both the admin theme and the desired theme. This worked. It just shouldnt be necessary sense any added tpl file to the admin theme could be accidentally removed with a module update.

Sutharsan’s picture

FileSize
2.62 KB

This issue is blocking the stable release of Simplenews. I need your input to find the *cause* of this problem.
In the attachment you find two patches. One to theme.inc and one to simplenews.module and simplenews.admin.inc.

theme.cron_theme.patch:
This is a debug function to verify the theorie that during cron execution the wrong theme is selected. This theory can not be confirmed from the code.

simplenews.cron_theme.patch:
This patch temporarily adds an option to admin/settings/simplenews/mail with which a cron theme can be selected.

Test plan:
1. On a test site, you create a simplenews newsletter issue. Configure simpelnews to send using cron. Save and send the newsletter, but do NOT execute cron yet.
2. Patch theme.inc with theme.cron_theme.patch.
3. Fire cron. Make sure you do not load any test website page.
4. Remove the patch or uncomment the code.
5. Check the watchdog content. It should contain one message "$theme_key set to:
". According to the theory the listed theme is the admin theme (e.g. garland). It should however be the default theme.

6. Create a newsletter template simplenews-newsletter-body.tpl.php in your site custom template. Make sure it differentiates clearly for the default template.
7. Clear the theme registry (or all caches).
8. Apply the second patch (simplenews.cron_theme.patch).
9. Select your site theme at admin/settings/simplenews/mail.
10. Create a new newsletter issue and save and send it but do NOT fire cron.
11. Reactivate the first patch (theme.cron_theme.patch).
12. Run cron.
13. De-activate the first patch.
14. Check the content of the watchdog and the newsletter format. The watchdog should list your sites theme, the newsletter should be formatted by the new template.

If you can not patch you can still contribute. Send me a PM with the login details (ftp & site admin) to enable me to test on your test-server.

Annakan’s picture

Subscribing

Sutharsan’s picture

We don't need supporters, we need testers!

Agogo’s picture

I've tested the patches. Im using Garland and a home made theme.

The results:
5) $theme_key set to: garland
9) Home made theme selected in new Cron drop down
14) $theme_key set to: home made
14) Newsletter is NOT using home made theme simplenews-newsletter-body.tpl.php

Sutharsan’s picture

So this theory seems to be invalid. Is there anyone who want to give me give me full access to a test site to debug this problem?

Agogo’s picture

Sorry Sutharsan, Ive just discovered that the installation I was testing on (a public site) had an old hacked version of the Simplenews Template module semi installed and that it caused the templates to behave iradically. So the test results I published is'nt valid.

I will try to make time to do the test again as soon as possible.

askibinski’s picture

Sutharsan> we've got this problem too, I'll look into it to see if I have a test site where you could take a look.

moritzz’s picture

@Sutharsan

I have the same Problem with the Simplenews and the Admin Module. I set the admin theme setting under admin/themes/admin to default, but choose to switch to the admin theme when editing nodes. When i send out test newsletter (not over cron) my custom theme is _not_ used, but it's used when i send over cron - unfortunately even cron seams to render the nodes as a logged in user is available, which adds some of the Admin module's overlays in the newsletter. I even tried your patches and got...

$theme_key set to: raumboerse

.. when sending over cron, which seams to prove that my custom theme is used.

If I can, I would be very happy to help.

Thanks,
Moritz

captaindav’s picture

Assigned: captaindav » Unassigned
Status: Needs review » Active
FileSize
1.6 KB

I was able to reproduce this bug on my Windows laptop (running Acquia Drupal Stack and Eclipse) as follows:

1. Install Drupal 6.14
2. Install Simplenews 6.x-1.x-dev
3. Copied Bluemarine core theme to custom theme located in /sites/all/themes/snewstest and set copied theme as default theme
4. Copied simplenews-newsletter-body.tpl from SN module to custom theme directory, and edited the heading as follows: <h2><?php print "NOT " . $title; ?></h2>.
5. Created newsletter with subject of “SN Test” and body of “BROKEN”.
6. Added my email as subscriber
7. Set Administration Theme to Bluemarine and checked “Use administration theme for content editing”
8. Bug was reproduced, as email body contained “BROKEN” rather than “NOT BROKEN” This was observed for both Test and Cron runs, in both plain and HTML format.

I traced the code and believe the problem is caused by the system setting the theme to the Administration Theme while in the node editing form from which the “Send Newsletter” functionality is found (cron runs must invoke the Admin theme as well).

This automatic setting of the admin theme occurs in system.module line 535:
global $custom_theme;
$custom_theme = variable_get('admin_theme', '0');

I was able to work around this behavior by inserting the following lines of code into simplenews.module at line 1516 (after if (!isset($messages[$nid][$langcode]))

global $custom_theme;
$save_theme = $custom_theme;
$custom_theme = NULL;

And at line 1561 (at end of if block):
$custom_theme = $save_theme;

Although this seems to fix the problem, after tracing the theme code, I am concerned that this approach may cause some other unforeseen problem, due to the complexity of the Drupal theme system, and the fact that I am changing global variables usually set by Drupal core. Could someone please test this code, by either modifying your simplenews.module as described above, or applying the attached patch. Hopefully this will fix this issue for now. In the future, I think the idea in #31 above, whereby Simplenews templates would be located in a separate directory, e.g. /sites/all/newstemplates, and made independent of any specific theme, should be given serious consideration.

Thanks!

captaindav’s picture

Assigned: Unassigned » captaindav
captaindav’s picture

Status: Active » Needs review
kwinters’s picture

Status: Needs review » Needs work

Unfortunately, forcing the use of the front-end theme will break any sites relying on the emails to be sent via the admin theme. Although they *shouldn't* be doing that, some will because of this bug.

A better solution would be to add a SimpleNews setting that lets you choose whether to use the front-end or admin theme, defaulting to the front-end for installs but updating to admin for existing sites. Then, no existing sites should break.

Even that is really still a workaround. The best solution involves fixing it in core, but that's unlikely to happen until at least D8 now.

Sutharsan’s picture

Re #44: but these lines in system.module are only executed on node add/edit forms and admin/* urls. How does this affect the theme when printing?

captaindav’s picture

re: #47

I don't understand why we need to support having templates in the admin theme going forward. Putting templates in the admin theme was only a work around for a bug - they never should have been there in the first place - and Simplenews should not have to support their being there in the future.

Essentially, it is a bad practice to keep templates in the admin theme (as the admin theme is usually a core Drupal theme) and having an option to do so might encourage this bad practice. Rather, any templates in the admin theme should be immediately moved to a front-end theme as soon as the Simplenews template override bug is fixed.

When the bug is fixed, we should communicate that templates must be moved to the front-end theme by posting warnings on the Simplenews Project page, Handbook pages, and README.txt. That way the only users negatively effected would be those that have templates in their admin theme and neglected to read the instructions - which I would think would be very few.

captaindav’s picture

re: #48

I don't understand what you meant by printing?

I tested the patch successfully with sending a test email, sending newsletter without cron, and sending with cron, for both plain and HTML newsletters. Is there an option to print somewhere that I am not aware of? Does this also require the use of the templates?

kwinters’s picture

It's pretty easy here to avoid breaking backward compatibility, so I think we should. No matter how diligent we are about documenting the change, it's going to break some sites if we don't maintain backward compatibility. It doesn't help that it's a pain to test, and sometimes there is more involved than just copying templates.

We don't have to support it for long, and can / should specifically say that it's temporary and admins should change it. The whole issue should be temporary, really, since it needs to be handled in core at some point.

kwinters’s picture

Re: #48 - It changes the theme, sends the email, and then immediately changes it back. Only actions that happen during the email send would try to use the altered theme, unless the state gets hosed by the theme change.

System.module changes the theme during hook_init but that may not have to be true here. I can't tell if there will be other consequences.

captaindav’s picture

re: 51

Would you be willing to create a patch for this?

You would have to auto-detect if Simplenews templates exist in the admin theme at install time, and have a setting like "Override Template Location" set to either "Admin Theme" (templates exist) or "Default Theme". And I am not sure how to handle the situation where Simplenews templates are found in both the admin and default theme?

I assume the ui for the setting would be located on /admin/settings/simplenews/general?

You would also need to document the setting, communicating that having templates in the admin theme is only a temporary feature due to a prior bug that is now fixed. And the setting would have to be tested for combination of: templates in admin theme, templates in default theme, cron/no cron, plain/HTML, and send test newsletter.

My concern is that due to the increased complexity of having this temporary feature, that a bug could result, creating more pain for the user than if he had been forced to move his themes to the default theme directory in the first place. And, the bug could effect all users, not just ones with templates in the admin theme. And, even if all of this is implemented bug free, none of the work benefits the project going forward as it is only a temporary feature that will have to be removed from the code and documentation in a future release.

kwinters’s picture

I can create a patch if it comes down to it, but I'm also working on a ton of other patches. :)

We don't actually need any template detection logic. Using a .install update hook, it can be set appropriately for upgrading users and differently by default. The setting is really just "yes, do the workaround" or not. If it is set, then it uses the new code added by your patch. If not, don't change the theme at all there.

Really only the "yes, do the workaround" option has to be fully supported. The goal is to give the site admin time to fix it the right way (and to notice that it's a problem).

There is also a hook to display a note on the site status report, which could say it's using a deprecated setting. Or if you don't want to maintain backward compatibility, then you could also just implement that hook and check the admin theme for a simplenews template. That's still better than no in-site warning at all.

I'm not worried about difficulties removing the code later, since it will be fairly isolated.

captaindav’s picture

Why don't we let Sutharsan decide if he wants to wait on your patch which would provide backwards compatibility or implement my patch which would force upgrading users to move their templates.

Sutharsan’s picture

Ok, I followed captaindav's description and was able to reproduce the problem (pew, at last!). The theme which is going to be used is determined in init_theme() in this code

  // Allow modules to override the present theme... only select custom theme
  // if it is available in the list of installed themes.
  $theme = $custom_theme && $themes[$custom_theme] ? $custom_theme : $theme;

Core sets $custom_theme to the admin theme as per "Administration theme" settings. Because the email is sent while the admin theme is the current theme the email is formatted using the admin theme. So the problem only occurs if the emails are sent 1. upon saving the node (no cron) and 2. when the admin theme is not the default theme. We could or force the use of cron, forbit the use of admin theme or set the theme as in #44. You choose ;)

Captiaindev's code should be extended with a simplenews admin option to select the theme. Which is by default the site's default theme.

Backwards compatibility is not an issue. We put some code in to detect the situation and give an error on the Status report page.

@captaindev: I meant 'mailing' not 'printing'. Many thanks for your push to get this moving again.

kwinters’s picture

Sutha,

Are you sure it works on the cron even with the admin theme set? That wasn't my experience, but I could be wrong.

The only difference between my suggestion and the extension proposed in #56 is an update function. It sets sites that were already using simplenews prior to the fix to use the admin theme (which gives an easy upgrade path, nothing breaks). Fresh installs should still default to the front-end theme.

If simplenews has been installed fresh, the install can set the setting to the default theme. On update, if the setting does exist at all then make it admin.

Sutharsan’s picture

I set $custom_theme to the theme I want to theme the newsletters with and all looks Ok. But it is subject to crowd source testing.
I don't want to treat the sites with a work-around differently. There can be two kinds of work arounds: those with a custom template in the module directory and those with custom template in the admin theme. It is not simplenews's task to watch over peoples work-arounds.

Sutharsan’s picture

Status: Needs work » Needs review
FileSize
1.57 KB

Attached a slightly altered patch for test.

kwinters’s picture

Looks like there are some whitespace issues, but nothing else jumps out at me.

askibinski’s picture

patch applies fine against latest -dev, but the 2 tpl.php files in my frontend theme still aren't being used.
When I copy them back over the simplenews defaults in the module folder, everything is fine.

all caches, theme registries etc.etc. were cleared of course.
I am using a 3 themes here: one for the frontend (which is default) one admin theme, and one mobile theme.

note: I'm only testing this now with the 'send one email to the email-adres below'-function.

captaindav’s picture

re: #59

I would name the variable orig_custom_theme rather than org_cutom_theme

captaindav’s picture

Assigned: Unassigned » captaindav
Status: Active » Needs review

re: #59

When I checked out 6.x-1.0-rc6 from CVS, simplenews.module has the following version:

1.76.2.129 (this also agrees with the 6.x-1.0-rc6 tar file version)

However, the patch references this version:

1.76.2.136

Does this matter?

captaindav’s picture

re: #61

askibinski,

With 3 themes enabled, the patch worked on 6.x-1.0-rc6.

Could you try patching 6.x-1.0-rc6 instead of dev?

Thanks,

captaindav

Sutharsan’s picture

@captaindev: patches are always against the last version in the branch (i.e. -dev)

Sutharsan’s picture

askibinski gave me access to the site he had the problem with. It is caused by 'slate' which is the admin theme of the Admin module. It handles the admin theme override differently; as a comment in the code says: "Bypass the theme system entirely so we can load our own theme into place." I see no easy solution for this case. For now I like to go forward with the #59 solution and leave the Admin module case as won't fix.

Sutharsan’s picture

Status: Needs review » Fixed

Patch in #59 is now committed to HEAD and 6.x-1.x-dev.
Thanks to all who participated.

askibinski’s picture

On a side note: the admin module has gotten a 2.0 branch now, which handles the admin theme like a real standalone theme (not in a module like in 1.x). It's very likely this will fix the 'incompatibility'.

Sutharsan’s picture

Thanks for the update.

Status: Fixed » Closed (fixed)

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

-Mania-’s picture

In case anyone is having problems with simplenews templates on a custom theme and wants it to work on 6.x-1.0 here's the method I've used:

I'm using the Zen as the front end theme and Garland as the admin theme. The test emails work fine but sending out the final newsletter will ignore simplenews templates UNLESS they reside in the garland theme directory as well.

So, I just created an empty garland-folder in /themes and copy-pasted the simplenews template files there. Everything works fine after this. I believe the Patch in #59 fixes this but I won't upgrade until it reaches a final version.

Hope all that helped someone.

Sutharsan’s picture

The latest 6.x-1.x-dev release should have solved this problem. If you still want to remain with 6.x-1.0 you can also create a symlink in the garland folder pointing to a themes folder in the front end theme.

plach’s picture

Version: 6.x-1.0-rc6 » 6.x-1.0
FileSize
1.88 KB

I experienced this bug in the 6.x-1.0 release, but I confirm the issue seems to be fixed in the latest 6.x-1.x-dev release. I preferred to stick with the 6.x-1.0 version and found the attached workaround.

drupalfan2’s picture

Status: Closed (fixed) » Needs work

Problem still exists in latest dev version (2010-Apr-22).
simplenews-newsletter-body.tpl.php is ignored when sending text mails.

Please repair!

Sutharsan’s picture

Category: bug » support
Priority: Critical » Normal
Status: Needs work » Active

DrupalFan2, this does not really help you. Take your problem seriously and supply all the details to reproduce the problem.

drupalfan2’s picture

Here are all the details:
- I did'nt apply any patch till now
- I am using Garland as admin theme
- simplenews-newsletter-body.tpl.php is ignored when sending text mails

What can we do to make simplenews-newsletter-body.tpl.php work?
Thank you very much.

Sutharsan’s picture

Where have you placed the template file?
Where have you tried to place it without success?
Have you cleared the cache when you created a (any) new template file?

drupalfan2’s picture

Sorry, but these questions should only be asked if you know that someone is a drupal beginner. I am NOT a drupal beginner!

Answers:
- I placed the template file in my theme directory
- I did not work there. Other template files do not make any problem there.
- Of course I cleared cache about hundred times!
- For all this test much time is required. The error should be fixed.

Where can the error be located?

kwinters’s picture

@DrupalFan2

Sutharsan doesn't have anything against you, but he can't possibly resolve your problem until he knows what is different between your setup and those that are currently reported as working. I understand that you're busy (we all are), but if you expect someone to fix something for you for free then you should make it as easy for them as possible. In most cases, this means providing as much detail as possible about your configuration or steps to reproduce.

drupalfan2’s picture

What further details can I provide?

eff_shaped’s picture

I'm having this same issue - using the latest version - Simplenews 6.x-1.2. Newsletters are not being sent with any theme.

I've tried:

1. putting simplenews tpl files into
a) a new "bluemarine" folder in sites/all/themes/ and
b) "bluemarine-folder" in /themes. Neither worked.

2. putting code from patch simplenews-374222-73.patch into simplenews.module - also did not work for me.

3. Finally worked when I put simplenews tpl files directly into the core /themes/bluemarine folder ... which means these need replacing for every core update. Not an ideal solution.

(Subscribing)

ShadowMonster’s picture

I don't know if someone was say it but for me this body template start working when I turn off Simplenews Template module...

Regards
ShadowMonster

GBain22’s picture

I know this may have been stated already, but if you're testing your newsletter template by amending (using mimemail-message.tpl.php) and this works fine when you test send. But sending out using cron doesn't use the template. You have two options:

1. Don't use cron for sending out the mail
2. Copy your mimemail-message.tpl.php into your current front-end theme. (Clear cache afterwards)

After I did this, my full send out started using mimemail-message.tpl.php from the front end.

The only downside to this is that you have 2 copies of the same file that you'd need to edit, should you want to make a change to the template.

kwinters’s picture

@#83 - As a temporary workaround, can you just include one file from the other? That way you will only have one copy of the code.

GBain22’s picture

Do you mean, use the "include()" function or something?

All I'm saying is, when you've finished creating your template, copy it from your admin theme, to your front end theme folder. That way it exists identically in both directories.

I actually don't see too much of a problem here, just remember that if you update your template at some point when your doing test sends, remember to copy the changes into the front end file before you do your full send out.

kwinters’s picture

Include is technically a language construct (like empty) instead of a function, but that's not important here ;)

If you have two files that need to be identical, you shouldn't actually make them the same via copying. Put all the actual code in one file and have the other one reference the full version with an include statement, symlink, etc. Then you can't forget to update one of the places.

Having to do this at all is undesirable, particularly since most sites use Garland as an admin theme and modifying it would be a core hack. It is supposedly fixed in the dev release but I can't confirm.

drupalfan2’s picture

Priority: Normal » Critical

Copying
simplenews-newsletter-footer.tpl.php
and
simplenews-newsletter-body.tpl.php

to /www/themes/garland

solves this problem!

When will this problem be solved in dev version?
- There are hundreds of users who run into this problem.
- This problem is a known problem since "February 14, 2009 - 01:26"!!!!!!!!!!!!!!!!!!!!!!!

Please finish and provide a dev version that solves this problem.
Thank you very much. Hundred of users will thank you.

goldhat’s picture

I just encountered this problem in version 6.x-1.3 when using RootCandy Admin and it took 1.5 hours of debugging and searching for answers before thankfully I found this thread. Considering its been a known issue for a long time, I cannot say I'm happy that its been left unresolved, but I realize this module is going through a maintainer change. I hope the new maintainer will see fit to make this a priority in their work and ensure its resolved as quickly as possible. It's an issue that is very deceiving to a developer, I ran countless tests trying to debug it and probably never would have found the issue on my own. It also can crop up as an issue on an existing site as was the case today, we switched to RootCandy admin theme and updated the content editing settings, and all of a sudden the Newsletter system failed to apply themes.

dboulet’s picture

Version: 6.x-1.0 » 6.x-1.x-dev

I was trying to tackle this issue, but it turned out to be very challenging. It looks like the theme system is not flexible to allow 2 themes to be used on one page—for example, the theme registry is stored in a static variable, which means that it can't be altered once a theme has been initialized.

A few ideas I had to solve this problem:

  1. Set the theme in hook_init()
  2. Redirect to another path where the newsletter can be sent (such as node/%/simplenews/send)

Anyone else have any ideas?

miro_dietiker’s picture

dboulet,
Did you look into views theming system and how they handle multi-theme settings?

One we could be:
During send we should then switch to the send theme... (to be set in simplenews settings?)
We could then switch the theme for the specific reloads (batch api, cron) in hook_init.

Additionally we should redirect to avoid wrong theme display after process complete (batch?).

A second way could also be to simply declare this issue a documentation issue.
If we define that simplenews customizations should be part of ALL themes used on a specific page, there's no issue if we're sending under the wrong theme... Additionally you should make sure to have a mail.css (for mimemail.css) in all themes, or you will get the wrong css bundled.

dboulet’s picture

@miro_dietiker, thanks for the suggestions, where exactly should I be looking for the views multi-theme settings?

In my opinion, this issue absolutely needs to be fixed. Having to sync templates across all active themes doesn't seem acceptable to me, seems like an updating nightmare!

sgabe’s picture

This issue concerns Mime Mail too and I can see this is not easy at all. Till we find a real solution I suggest everyone to place customizations (both Simplenews and Mime Mail) in all themes used on their site. I am doing this too. I can agree it is not comfy but solves the problem for now.

Lukas von Blarer’s picture

Version: 6.x-1.x-dev » 6.x-2.x-dev

hi all

i think i found a problem with the 2.x version.

in simplenews.module on line 2328:

    'simplenews_newsletter_body' => array(
      'template' => 'simplenews-newsletter-body',
      'arguments' => array('node' => NULL, 'language' => NULL),
      'pattern' => 'simplenews_newsletter_body__',
    ),

should be:

    'simplenews_newsletter_body' => array(
      'template' => 'simplenews-newsletter-body',
      'arguments' => array('node' => NULL, 'key' => NULL, 'language' => NULL),
      'pattern' => 'simplenews_newsletter_body__',
    ),

because of that the file simplenews-newsletter-body.tpl.php in the themes directory has no affect. only simplenews-newsletter-body--[tid].tpl.php.

for now i put the template files in the garland theme directory.

additionally i found out that MIME Mail solved this problem. mail.css in my frontend theme's folder works. they can do:

  $theme = variable_get('theme_default', NULL);
  $mailstyle = drupal_get_path('theme', $theme) .'/mail.css';

is something like this possible for .tpl files as well?

ehy is this issue for 1.x? shouldnt this get backported from 2.x?

thanks for solving this issue asap.

lukas

miro_dietiker’s picture

elluca, your suggestion is not correct.
The hook_theme announces the exact parameters of the theme functions - see

./simplenews.module:1550:        $body = theme(array('simplenews_newsletter_body__'. $context['node']->simplenews['tid'], 'simplenews_newsletter_body'), $node, $message['language']);

^^ see the parameters after the theme key: $node, $message['language']

Note that the theme function is not implemented but the .tpl.php file representation.
Further matches / relations:

./simplenews.module:2155:function template_preprocess_simplenews_newsletter_body(&$variables) {
...
./simplenews-newsletter-body.tpl.php:18: * @see template_preprocess_simplenews_newsletter_body()
attheshow’s picture

subscribing

Edgar Saumell’s picture

Version: 6.x-2.x-dev » 6.x-1.x-dev
Assigned: captaindav » Unassigned
Category: support » bug

Same here sending test newsletter without cron.
Template files on custom theme (zen based) folder.
Tried with generic ones and with newsletter term's id, always clearing cache. No luck.

I'm using Garland as administrative theme. I unchecked the box for "Use administration theme for content editing" and i got the templates working.
Not tried with real newsletter.

websites-development.com’s picture

Issue tags: -Needs documentation

I'm using Garland as administrative theme.
when sending a test newsletter (aka no cron) it uses Garland as the theme
when sending via cron, it uses the custom front-end theme (zen based)

miro_dietiker’s picture

Please

instead of repeating the issue description periodically:

Would someone please investigate how other projects fixed this and what approaches have been chosen?
We need inputs, starting points - finally solutions. This case can be reproduced clear enough.

Thank you! :-)

plach’s picture

On my sites I'm using the following PHP snippet to make Mimemail and HTML Mail work with the default theme templates:

<?php

/**
 * Implementation of hook_theme_registry_alter().
 */
function custom_module_theme_registry_alter(&$theme_registry) {
  custom_module_theme_reroute_hook($theme_registry, 'mimemail_message');
  custom_module_theme_reroute_hook($theme_registry, 'htmlmail');
}

/**
 * Reroute a hook to be output through a template belonging to another theme.
 */
function custom_module_theme_reroute_hook(&$theme_registry, $hook, $source_theme = NULL, $target_theme = NULL) {
  global $theme;
  
  if (empty($source_theme)) {
    $source_theme = variable_get('admin_theme', '');
  }

  if (empty($target_theme)) {
    $target_theme = variable_get('theme_default', '');
  }

  if ($source_theme != $target_theme && $theme == $source_theme) {
    $themes = list_themes();
    $target_theme = $themes[$target_theme];

    $target_path = dirname($target_theme->filename);
    $existing = array($hook => array());
    $engine_callback = "{$target_theme->engine}_theme";
    $templates = $engine_callback($existing, 'theme', $target_theme->name, $target_path);

    // @todo consider subthemes.
    if (!empty($templates)) {
      $path = $templates[$hook]['path'];
      $theme_registry[$hook]['template'] = basename($theme_registry[$hook]['template']);
      $theme_registry[$hook]['path'] = $path;
      $theme_registry[$hook]['theme path'] = $target_path;
      $theme_registry[$hook]['theme paths'][] = $path;
      $include_file = $target_path .'/template.php';
      $preprocess = FALSE;

      @include_once $include_file;
      foreach (array($target_theme->engine, $target_theme->name) as $prefix) {
        $function = $prefix .'_preprocess_'. $hook;
        if (function_exists($function)) {
          $preprocess = TRUE;
          $theme_registry[$hook]['preprocess functions'][] = $function;
        }
      }

      if ($preprocess) {
        $theme_registry[$hook]['include files'][] = $include_file;
        $theme_registry[$hook]['preprocess functions'] = array_unique($theme_registry[$hook]['preprocess functions']);
      }
    }
  }
}
?>
miro_dietiker’s picture

Cool. I see the workaround you're introducing. This is already very (unexpectedly) clean from my perspective. I first shought i'd find many contras... but don't.
Still i'm looking a more simple solution - git currently i doubt we'll find one...

The biggest issue we're introducing is the partial double-loading of two themes at the same time - resulting in potential namespace clash.

Note that this workaround loads multiple template.php files at the same time. This will break several previously valid setups where override function names use "phptemplate_xxx" stale like phptemplate_preprocess_page as it results in double definition. However we can deny this and we won't loose much.

A different solution might be to put all rendering code into (ajaxy, ...) callbacks - executed from cron and/or user foreground sending (test and live send). During those callbacks we could switch the template completely (hook_init) and thus the solution has no other potential issues. Views might does it also that way since the theme information displayed shows uns a callback.

Do you see a different way to go?

Carlitus’s picture

Any news on this issue?

Simon Georges’s picture

@Carlitus, did you test the solution proposed in #99 ?

organicwire’s picture

I tried the solution proposed in #99 and it worked nicely. In order to make it work with my simplenews installation I had to change it slightly:

function custom_module_theme_registry_alter(&$theme_registry) {
  custom_module_theme_reroute_hook($theme_registry, 'mimemail_message');
  custom_module_theme_reroute_hook($theme_registry, 'simplenews_newsletter_body');
  custom_module_theme_reroute_hook($theme_registry, 'simplenews_newsletter_footer');
  // Uncomment this to apply your node templates when sending the newsletter
  // custom_module_theme_reroute_hook($theme_registry, 'node');
}
miro_dietiker’s picture

Issue tags: +Needs documentation

Needs documentation!

valderama’s picture

solution from comment #99 did also work for me..

best,
walter

checker’s picture

Here is an easy solution to prevent using admin theme on cron run without changing code.

Install admin_theme module and set 2 disallow pathes:
*run-cron
and
*cron.php

foolofatook’s picture

After much headache and thread searching, I think this is where I shall place my burden..

I believe I have a unique problem (as I've not seen it duplicated yet).

I'm using simplenews and mimemail to send a newsletter out. I am using panels to create a layout for which to place my newsletter content in. I chose to use panels because of its ease of use for a site moderator (with little to no html knowledge) to quickly and easily go in and alter or add to future newsletters as well as change the way they are layed out. I've set up styles for the panels and they all display rather nicely on the site.

In using simplenews, I went to admin/settings/simplenews/general and enabled 'panel' as a content type to draw from in sending my newsletter. When I view the resulting newsletter in the email I get, the css doesn't get referenced and thus displays no formatting.

I've run myself in circles looking for a solution and am pretty sure I've exhausted all issues in the simplenews and mimemail module pages. Please! If anyone could steer me in a proper direction, I may choose not to kill myself lol.

ps- I definitely flush the caches everytime I try and test new theories on this... just fyi
well, thanks in advance (i hope)

Lukas von Blarer’s picture

Issue tags: +Needs documentation

@foolofatook:

open a new issue.

liquidcms’s picture

confirming that solution in #106 works well, not sure what the * are for in the paths; don't think they are required.

liquidcms’s picture

uhhh.. nope..my bad.. #106 is right on.. do need the * in the paths..

and the last tip for this mess... use mail.css in theme folder to get custom css for newsletters.. finally

russelljgarner’s picture

Let me be totally clear: this issue is commically rediculous.

Is this module a practical joke or something? This is known behavior for the last TWO YEARS and nobody even bothered to document that the templates (contrary to the behavior of any other module) go in the admin theme!? Which by the way may very well be located in the core themes folder which acording to the principle "don't hack core" should remain untouched!

I spent A WHOLE WEEEK trying, in vein, to figure this out and custom coding a workaround to pull templates from the current theme folder, simply because this behavior was not put into the readme.

This is the kind of complete lack of simple documentation and rediculous volitility that I have come to expect from Drupal.

I am hereby moving to cakePHP, custom coding my own reusable components, and never looking back.

Simon Georges’s picture

Version: 6.x-1.x-dev » 6.x-2.x-dev

Changing branch. We have to work on that one day...

Pol’s picture

Here is a first version of a patch for dev version.

Please test and report.

Simon Georges’s picture

Status: Active » Needs review
Pol’s picture

Status: Needs review » Active
FileSize
1.3 KB

And here the same patch for alpha2.

Simon Georges’s picture

Status: Active » Needs review
Pol’s picture

Another solution:

/**
 * Implementation of hook_cron().
 */
function simplenews_cron() {
  $theme_default = variable_get('theme_default', '');
  $admin_theme = variable_get('admin_theme', $theme_default);
  $backup_custom_theme = $GLOBALS['custom_theme'];

  if ($admin_theme != $theme_default) {
    $backup_custom_theme = $GLOBALS['custom_theme'];
    $GLOBALS['custom_theme'] = $theme_default;
  }

  module_load_include('inc', 'simplenews', 'includes/simplenews.mail');
  simplenews_mail_spool();
  simplenews_clear_spool();
  // Update sent status for newsletter admin panel.
  simplenews_send_status_update();

  $GLOBALS['custom_theme'] = $backup_custom_theme;
}

Patch for 6.x-2.x attached.

KarimB’s picture

#117 worked like a charm in 6.x-1.3 too.

Thank you Pol :)

mikele’s picture

I've tried the patch (#115) without success...
This code did the trick for me in 6.x-2.0-alpha2:

/**
 * Implementation of hook_cron().
 */
function simplenews_cron() {

    global $theme_key;
    $theme_current = variable_get('admin_theme', 0);
    variable_set('admin_theme', $theme_key);

    module_load_include('inc', 'simplenews', 'includes/simplenews.mail');
    simplenews_mail_spool();
    simplenews_clear_spool();
    // Update sent status for newsletter admin panel.
    simplenews_send_status_update();

    variable_set('admin_theme', $theme_current);
  
}
Pol’s picture

Here's a new patch for 6.x-2.0-alpha2 and 6.x-2.x-dev.

It should send this issue back to hell ;-)

I need feedback !

** Sorry for the double post, the proxy here had a problem :( **

(the files are the same of course!)

Pol’s picture

Here's a new patch for 6.x-2.0-alpha2 and 6.x-2.x-dev.
You do not need previous patches, just this one will do the trick.

It should send this issue back to hell ;-)

I need feedback !

miro_dietiker’s picture

Pol, i'm frightened this will break other cron implementations...
(Causing permanent change for cron compared to core behaviour...)

mikele, if some other module does a theme / rendering thing in cron, it's too late to switch.
Till now i cannot see a way here how to fix this cleanly for all possible cases.

Can someone show me the light?
I'd love to fix this somehow cleanly..

Ah .. and russelljgarner,
Please do so... It's people like you moaning around, not asking for help (during a whole week), and not helping to improve things ... that sucks.

Pol’s picture

Hello @miro_dietiker,

The last patch provided doesn't change the hook_cron() in simplenews.module.
It's just in the hook_init() and executed only if the path is *run-cron or *cron.php.

I don't see why it would break the others cron implementations, can you explain further more ?

Thanks.

miro_dietiker’s picture

Changing the theme behaviour in hook_init on cron.php results in a theme switch for all cron calls.
(BTW: Did you also cover a drush cron execution?)

If some other cron uses theme calls, they'll be changed also after this switch.

Pol’s picture

Ok now I understand, It's only the variable admin_theme who is altered.

I'll make a new patch who will restore the variable at the end of the hook_cron() then.

Thanks for the report, we're almost there ! :-)

Pol’s picture

Here's a new patch for 6.x-2.x-dev

@Miro: can you review it and/or test it ?

dboulet’s picture

Pol, I’m not sure that your approach will work. I had tried it before and it didn’t work for me. From what I can remember, Drupal does not allow you to change the theme once it is set, even if you change $GLOBALS['custom_theme']. There can only be one theme set per page request. That’s what makes this problem so challenging and why it has lingered on for so long.

miro_dietiker’s picture

Status: Needs review » Needs work

Pol, a dboulet said. That's the general issue - a D6 limitation.

The clear statement "also place theming changes into your admin theme" allows it to work stable.
However, switching the theme in the hook_init or hook_cron leads to potential other issues with other modules. So that's no clean futureproof / failproof solution...

Possibly this cannot be fixed in D6 cleanly without hack, limitation or side effect.
Did someone inspect the situaiton if we can fix this for D7?

hbblogger’s picture

Version: 6.x-2.x-dev » 7.x-1.x-dev

Hi Miro,

It would seem that everyone is too scared to try DR7. Not surprising, though and sure enough, I can't get the templates to work. I have just installed Simplenews - which is great in principle, but NONE of the supporting modules are ported to DR7 almost 9 months since it came out (for most there aren't even dev versions). I am not a programmer and have never used DR6 (yes we do exist), so I would have liked to have used HTML template and a few of the other modules, but of course they are all in DR6 (and some even DR5).

I would like to set up a template for Simplenews in DR7, but can't even get past first base (placing the Template file into the template folder of the Zen sub-theme - also tried the template file of the Zen main theme and even the main directories of both the main and sub themes). All I have done to the theme file provided by simplenews is to add one HTML line to differentiate it. Also tried turning off the admin theme for editing as suggested, but to no avail. Yes, I have cleared the cache.

Welcome suggestions and in return (once I work out how to get the newletter to work), I would be happy to write a basic tutorial of some sort for DR7. (It took a while to find where everything was after installation, so I'm sure others are experiencing the same problems).

tugis’s picture

I managed to use the template files in my admin theme by changing the "simplenews_theme" hook. "template" variable is not defined for "simplenews_newsletter_body" and "simplenews_newsletter_footer".

Attached a patch for 7.x-1.x-dev

hbblogger’s picture

After many hours trying to work out where exactly the problem lies, it would appear that it is not Simplenews that is the problem, but the module/s that handle the HTML. There are two modules that you may choose to use - Mime Mail and HTML mail. It appears that they have similar features (and in fact there is a discussion on Drupal as to why they don't get together - but thats another issue.

From what I have found out (and I am open to correction), Mimemail only works if you put a specific file called mail.css in the root folder (NOT the template folder) of your theme (in my case a sub-theme of Zen). In fact it needs to be in root folder of your Admin theme (as well? - not sure) so my solution was to make my sub-theme my admin theme (which didn't look good at all). For specific templates - you can put the simplenews-newsletter-body.tpl and simplenews-newsletter-footer.tpl in the template file of your (sub) theme folder.

Don't like using your main theme as an admin theme? Neither did I.

Turns out that this problem can be bypassed by using HTML mail instead of Mimemail and (with a bit of effort) it can be made to do basically the same thing. In this case, though, you need to take html--simplenews.tpl.php file and modify it by adding your header or additional information above and below the body. So far I have only go it to work by modifying this file to do what I want (time was running out!). (note the two dashes). I left the file in the module (not the sub-theme) directory and it worked for me, but there does appear to be some sort of hierachy where you can place files with different names in either the sub-theme root or the sub-theme template directory to make the results more specific (haven't got that to work yet)

One final word on this subject - I tried (in vain) to add a logo to the top of my e-mail by modifying the template files. It does not work in either mimemail or HTMLmail. When I raised these issues, Mimemail came back to me and said that it wasn't possible and that this needs to be updated in their readme file and HTMLMail came back with 'noted'. So don't expect anything to work too soon on that front.

HTML Mail has a link to the echo module which is supposed to take your website template and put it into your e-mail, but I didn't get that to work.

Hope this saves others a few hours of frustration!!

bensey’s picture

Priority: Critical » Normal

Ok, this thread is getting pretty crazy. I came here as it's a 7.x thread but it was only changed that recently at towards the end. I'm with #111 on this - this issue is becoming kinda stupid. And gosh Sutharsan - I feel your pain..!

This did of course give the lead though, to put my template files in the admin theme - worked like a charm. Can't believe that I wasted so much time without trying that! As was described way earlier, the problem is that with D7 the admin theme is part of core if you are using Seven - which I am. Dropping template files outside the sites directory is a big no-no.

It also seems a little silly to add templates to the admin theme when the templates themselves are themed to match the non-admin theme. I think that is the major issue.

Perhaps there should be a location for template files outside themes, something like sites/libraries/templates for these kind of situations that people can choose to use if they wish to include cross-theme templates..?

Or should simplenews have a setting somewhere where you can choose where it seeks templates, as the HTML Mail module does? This could be a simpler solution perhaps?

For now making sure I add some templates to the core admin theme is yet another thing that goes on the list for every time I apply core updates. *sigh*

Have denounced this from critical to normal - it's not that big, and there's way too much drama here!
Not sure if it's even a bug report too? Perhaps more something for which an eloquent solution does not yet exist.

Ron Williams’s picture

Using Seven Admin theme with Simplenews Templates

I encountered this issue and had concerns regarding the templates being in the core theme directory since I don't want to modify core, even to just add files to the directories. As mentioned in #132, adding another item to a list of to-do's is not something I want to do.

I am not sure whether this is the correct thing to do, but I have yet to have an issue caused by this setup. Add a folder called seven to your sites/all/themes directory. Add the template files for simplenews there, but leave all core files where they are and do not move them to the new directory. Be sure to clear the cache and the files will be recognized and usable while your admin theme will still be upgradable as part of core without any extra steps with the upgrade.

^I made the bold/h2 content so users could easily notice this post. If this multi-folder solution is incorrect, let me know. Generally issues with multiple locations for modules/themes are related to different files within each directory. In this case there are no duplicate files.

bensey’s picture

Thanks Ron,

have tested and confirmed that #133 does indeed work! Fantastic and simple solution.

Simplenews template files placed in sites/all/themes/seven do perform as expected, and do not need to be placed in the core seven theme directory.

jherencia’s picture

Please try #1349728: Possibility to configure the theme that will render the email, I think it may solve all this problems.

miro_dietiker’s picture

Assigned: Unassigned » Berdir

Assigning to current bugfixing cycle. Would be great to have here a clean long term fix.

GiorgosK’s picture

#135 lets you assign what theme handles the emails when you use mimemail
works great

jherencia’s picture

Status: Needs work » Closed (duplicate)
Berdir’s picture

Status: Closed (duplicate) » Needs work

Not so fast.

Want to have a look at the code there first, we might need something similar for plain text mails and/or different html mail modules as well.

nerdacus’s picture

This is NOT a duplicate.

I am currently running Simplenews 7.x-1.0-alpha1 (2011-Sep-17) and Mime Mail 7.x-1.0-alpha1 (2011-Dec-18), the latter of which has the option to select which theme I want Mime Mail to use. This works seems to work fine with e-mails other than Simplenews.

When debugging Simplenews, I found that the message is built by Simplenews with the templates of the current theme (see postscript below). Sending a test results in the use of the admin theme, but when cron is ran the default is used. The only thing being pulled by Mime Mail's specified theme is my mail.css when the newsletter is sent, but before that Simplenews does not know of Mime Mail's specified theme (nor should it).

Currently, I have duplicated templates in both my default and admin themes, but in the future this could result in stale data. I propose the addition of configurable option similar to Mime Mail's where admins can force the use of one theme over the others.

I hope this helps,
-Marcus

PS: The following code does nothing to prevent the use of the admin theme when sending the test e-mail as it claims it does.

EDIT: I just looked over the latest release of 7.x.-1.x-dev and this code is irrelevant, but I still predict the same issue happening, I'll see if I can get around to testing it.

    // Use the default theme to render the email content.
    // We temporary clear the $custom_theme to prevent the admin theme
    // from being used when the newsletter is sent from the
    // node add/edit form and the admin theme is other than the
    // default theme. When no $custom_theme is set, the
    // After theming the email $custom_theme is restored.
    global $custom_theme;
    $org_custom_theme = $custom_theme;
    $custom_theme = '';

    watchdog('DEBUG simplenews', '<pre>theme path = '.print_r(path_to_theme(),TRUE).'</pre>');

The theme path is not changed nor forced to be the default.

Watchdog returns different paths when ran as a test versus cron.

Berdir’s picture

Yeah, I removed that part of the code in 7.x-1.x-dev, because, as you found it, it's absolutely not doing anything.

The setting in mimemail only controls the theme for 'mimemail_message' as far as I can see. We would need to add something similar for our own theme functions.

Before doing that, we need to check if it's possible for two themes to implement the same function. I think support for phptemplate_* prefixes have actually been removed from core now that I think about it but still, something I need to check. If it's safe to include the template.php of two different themes at the same point, we could try this...

Berdir’s picture

Status: Needs work » Needs review
Issue tags: -Needs documentation
FileSize
8.34 KB

Ok, here is a first patch.

This is basically the same approach as done by mimemail, but for our own theme functions.

Please test.

s_leu’s picture

#142: simplenews_theme.patch queued for re-testing.

s_leu’s picture

Status: Needs review » Needs work

Applied the patch and it didn't fix the problem in my environment.

s_leu’s picture

- Patch does not work if memcache is enabled
- Description about template naming in simplenews/README.txt is wrong, view mode specific templates have to be titled like this simplenews-newsletter-body--email-html.tpl.php and not simplenews-newsletter-body--email_html.tpl.php
- category names have to be written in lowercase, spaces and underlines have to get converted into single dashes
- renaming category terms (which will probably be performed by normal endusers) leads to loss of all category specific templates. Therefore category specific templates should use the tid instead of category name in the filename.

jherencia’s picture

The probably reason is that maybe memcache has not cleared it's caches with drupal_theme_rebuild.

Do you have memcache module installed?

Berdir’s picture

Status: Needs work » Needs review

The memcache issue was due to a bug in memcache.module, which as been fixed in this very moment: #1344036: Backport tests from: Cache clear fails when using wildcards - strange behavior with wildcard_valid()

Berdir’s picture

FileSize
8.73 KB

Ok, found the problem.

It's a conflict with mimemail.module. The registry alter hook of it is called first, it then triggers a build of the configured theme and only for that theme is simplenews then called. Because the global $theme_key is just the key of the currently used theme and still points to the admin theme. So simplenews then thinks that it is altering the the admin theme, checks that off as processed in $executed and when it is finally called for the actual admin theme, it is ignored because it thinks that it was already processed.

The result of this is that we can't rely on $theme_key. The proper solution would be a fix for #1119364: hook_theme_registry_alter does not provide information about the theme in question but we can't wait for this.

So, the attached patch "solves" the problem by adding a simple semaphore recursion prevention and removes the $theme_key == $simplenews_theme check. The downside of this is that we build the configured theme twice.

The patch also adds support for theme hook suggestions.

We're probably going to try and move most of this into the mailsystem module. Because if mimemail and simplenews would actually work equally, we'd build the configured theme three times...

miro_dietiker’s picture

Status: Needs review » Reviewed & tested by the community

Looks generally good.

Note that the theme registry is thus cycled recursively ALWAYS due to the call of _theme_load_registry... This however is even a private function by definition.
After a cache clear this means, the registry is ALWAYS built TWICE as a recursion. There's potential conflict with other modules doing the same...
However this seems currently unavoidable due to drupals design. But it should be documented, why the recursion happens.

I'm so happy this theme issue can be solved for future!
Having it even cleaner solved with mailsystem module would be even better. But at least we're no more lost with the theming system.

jherencia’s picture

Doble post, sorry. DO went down.

jherencia’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
3.3 KB

I added this: #1382036: Add a common way to configure the theme that will render the email so if it finally gets in the patch I append would fit.

I cannot test it as long as I don't have a simplenews environment configured so it'd be great if anyone could.

IMPORTANT:
For those who are going to use this: RTBC is #374222-148: Template files do not work when using admin theme

jherencia’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 374222_mailsystem.patch, failed testing.

Berdir’s picture

The tests on your patch are failing because we do not (yet) depend on mailsystem. Will probably work once you add the dependency to the info file.

jherencia’s picture

Status: Needs work » Needs review
FileSize
3.59 KB

Status: Needs review » Needs work

The last submitted patch, 374222_mailsystem_2.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
10.99 KB

Ah yeah, the testbot won't find the new dependencies

Ok, what about this?

We don't add a hard dependency on mailsystem, instead, we simply integrate with it. See attached patch and the following excerpt from the updated README.txt:

Simplenews supports the mail theme setting from the
mailsystem module (http://drupal.org/project/mailsystem). Install it, choose
the mail theme and the newsletter templates from that theme will be used no
matter which other themes are enabled.

I also re-worked other parts the README.txt. That category-machine-name stuff is nonsense, there is no such thing. The only thing that's supported is the tid. And many paths and tokens were wrong.

Schnitzel’s picture

+1 for the patch at #157, worked perfectly.

Just to make sure for other ppl who wants to test: this patch http://drupal.org/files/1382036-3.patch from http://drupal.org/node/1382036 has to be applied to the MailSystem, that Simplenews uses the right theme :)

miro_dietiker’s picture

Thanks for review, Schnitzel.

Attached patch is primarily rerolled plus some minor changes.
As soon as it's committed in mailsystem i think we can go here.

Pol’s picture

Hello Miro,

By reading your patch I see that you are replacing tokens using machine name by their tid.

Shouldn't we keep that way of working ?

I mean, it's very usefull to use machine name instead of tid's when we deploy features on different instances who are using the same machine names and differents tids.

Thanks!

miro_dietiker’s picture

Pol, berdir already introduced this behaviour and argumented.
There's no machine name concept with taxonomy terms. Renaming a term (title) resulted in breach of template files. That's horrible.

miro_dietiker’s picture

Double post..

Berdir’s picture

I just removed the incorrect documentation. Neither machine names (never existed) nor titles could be used for templates, with or without this patch.

Berdir’s picture

@miro: You accidently commited your patch in commit c733ac0

I reverted the change with "$ git revert c733ac0".

rooby’s picture

The patch does not apply to the latest dev version.

Here is a re-roll of #159

It isn't working for me though. I have applied this patch and the patch for mailsystem and configured mailsystem to use my main theme but it is still loading template files from the admin theme.

It doesn't matter if I have the mail system set to use mime mail or the default, it still doesn't work.

rooby’s picture

Scratch that, I made a mistake in the last one.

This one actually works.

Berdir’s picture

Version: 7.x-1.x-dev » 6.x-2.x-dev
Status: Needs review » Patch (to be ported)

Commited, finally!

Moving this over to 6.x-2.x, the code in mailsystem has been backported but is afaik not working yet.

philippejadin’s picture

Sorry to fill even more this thread, but can a maintainer tell me when this will be part of a new (non dev) 7.x release of simplenews?

I rely on this feature for theming newsletters, and this would help :-)

miro_dietiker’s picture

It is already part of beta2.

znerol’s picture

For poeple who do not want to use the mailsystem module (for example because of #1369736: Alternative locations for "created" Class files?) an alternative is to disable the admin theme for the newsletter-tab by implementing hook_admin_paths_alter.

function mymodule_admin_paths_alter(&$paths) {
  unset($paths['node/*/simplenews']);
}

Update: This only works on Drupal 7 of course.

miro_dietiker’s picture

Note that this is not covering all usecases.
You might have rules that trigger things and have a send action on any url.
In addition, the cron also needs to be covered.

It's actually a lack of core to support those theme cases because they're needed by nature of the architecture and pluggability.

rooby’s picture

The cron case is particularly annoying.

znerol’s picture

Ok, probably I was wrong commenting on this issue because it is tagged with D6. I just rechecked the cron behaviour on Drupal 7 and I can confirm that the site theme is picked up during cron execution. Therefore in my case it is enough to remove the simplenews admin path in order to make the test-newsletter look the same as the real one. However I'm not distributing HTML-mails, I just override the simplenews template files in order to tidy up the header/footer a little bit such that infamous drupal_html_to_text does not screw up the whole thing completely but thats a different story.

I agree that a separate mail-theme is the right way to solve this problem. The solution from my comment above is just a quick way to fix my particular D7-text-only-newsletter-case withouth having to pull in another contrib module (mailsystem). ymmv.

dianacastillo’s picture

This #133 works for me (making a new directory 'seven' under my sites/all/themes )

ShaneOnABike’s picture

For what it's worth I upgraded to the latest Mailsystem and you can now choose what Theme to use (which is grabs the theme tpl files properly). Not sure if that works for others but it might help.

ShaneOnABike’s picture

I wonder if this is also relevant to the theme hooks that aren't getting called for me now? Even though I select the right theme it never seems to call them properly...

Is the patch above working sorry I can't make sense of the paper trail it's pretty long

jetwodru’s picture

Hi, thank you for your info, for a few days trying to figure out how to include the mail.css and the templates working for user emails (sign up no approval, activation, reset password and etc), I noticed that some are working and some are not and then I scratched my head till peeling while searching deadly online for solutions. Now I just realized that this is a Super Big Big Bug with this module. This bug should be documented in the Readme file otherwise many users would be suffering if they happen to use customized template files and mail.css .

This problem still persists in version 7.x-1.0-beta1, just for the info of those encountering the same problems. Thanks

MXT’s picture

Issue summary: View changes

Hi all,

I want to share my experience on simplenews templates override.

I'm using:

  • Simplenews 7.x-1.1
  • Mime Mail 7.x-1.0-beta3
  • A custom theme for users
  • A custom theme for admin

Notice last 2 points: so I have 2 custom theme folders in my sites/all/themes directory: a folder for the front-end theme and a folder for back-end theme.

Results:

  • putting simplenews-newsletter-body.tpl.php in my front-end theme folder: does not work
  • putting simplenews-newsletter-body.tpl.php in my back-end theme folder: works

Is this the correct behaviour?

If yes, how my overrides in simplenews-newsletter-body.tpl.php in admin theme can be passed to front-end theme also? Need this because newsletters received by users contains a link to see the online version too, and this online version doesn't reflects overrides.

Anyway I think all this should be better documented in readme.txt

UPDATING:

Thanks to #180 I was now able to choose what theme to use to format newsletter, regardless if these are sent by automatic cron (from front-end theme active) or manual cron (manually launched from back-end theme).

So, to get all thing works correctly using simplenews and mimemail, you have to:

  1. Put your templates override (e.g. simplenews-newsletter-body.tpl.php) in your front-end theme folder
  2. Go to /admin/config/system/mailsystem and choose your front-end theme from "Theme to render the emails" select options
  3. Put your mail.css in your front-end theme folder
  4. That's all

ONE MINOR ISSUE: I'm also embedding some views in newsletter body through hook_preprocess_simplenews_newsletter_body() in my template.php with relative views template-overrides. Well these views template overrides are ignored if you send your newsletter with manual cron launched from back-end theme. Instead, they are correctly picked up if newsletter are sent by automatic cron (from front-end theme active).
I've resolved putting these views template overrides in both folders (back-end theme and fron-end theme).

Hope this help

  • Sutharsan committed 49bd58b on 8.x-1.x
    #374222 by sutharsan: Fixed Template files do not work when using admin...
  • Sutharsan committed cb1d229 on 8.x-1.x
    #374222 by Sutharsan: Reapplied, accidently lost. #412234 by Sutharsan:...
  • Berdir committed 73b311d on 8.x-1.x
    Issue #374222 by Berdir, jherencia, rooby, miro_dietiker: Fixed template...
candelas’s picture

This problem still persists in 7.x-1.1.dev

kenorb’s picture

Version: 6.x-2.x-dev » 7.x-2.x-dev
Assigned: Berdir » Unassigned
Status: Patch (to be ported) » Needs review
kenorb’s picture

Version: 7.x-2.x-dev » 7.x-1.x-dev