In a module we're building we need to switch theme's during code execution to send emails, in D6 we did this as follows, the code is called by pressing a button on node/edit page

global $custom_theme;
$orig_theme = $custom_theme;
$custom_theme = variable_get('theme_to_use_for_email', $orig_theme);
/* load node, render page, send email */
$custom_theme = $orig_theme;

In D7 $custom_theme isn't available anymore and the only two options available to switch themes are:
1/ hook_custom_theme (http://api.drupal.org/api/drupal/modules--system--system.api.php/functio...), which is called way to early and changes the theme for everything
2/ using theme callback on menu item, but this will also change the theme for the page

   'theme callback' => 'variable_get',    
   'theme arguments' => array('theme_default'),

See also #539022: Batch API should use the current theme to run the batches and #553944: Define hook_menu_get_item_alter() as a reliable hook that runs before the page is doomed

The only possible solution I can think of right now is to setup a custom menu item with a theme callback and do an extra httprequest to get the rendered content with the right theme, but it isn't a clean solution.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

attiks’s picture

I had a look at the code in drupal_theme_initialize() so I tried the following which works in my scenario. in summary I call the code inside drupal_theme_initialize() with my custom theme before I render my mail body and after I'm done I do the same with the current theme.

Be warned: ugly code coming up

I'm waiting for feedback if this is an acceptable solution, if so I'll make a patch

Load custom theme

drupal_theme_rebuild();
$themes = list_themes();

global $theme_info, $base_theme_info, $theme_engine, $theme_path;
unset($theme);
unset($theme_info);
unset($base_theme_info);
unset($theme_engine);
unset($theme_path);

global $theme_key;
$origtheme = $theme_key;
$custom_theme = variable_get('newsletter_used_theme', $theme_key);

$theme = !empty($custom_theme) ? $custom_theme : $theme;

// Store the identifier for retrieving theme settings with.
$theme_key = $theme;

// Find all our ancestor themes and put them in an array.
$base_theme = array();
$ancestor = $theme;
while ($ancestor && isset($themes[$ancestor]->base_theme)) {
$ancestor = $themes[$ancestor]->base_theme;
$base_theme[] = $themes[$ancestor];
}
_drupal_theme_initialize($themes[$theme], array_reverse($base_theme));

// Themes can have alter functions, so reset the drupal_alter() cache.
drupal_static_reset('drupal_alter');
// my custom code calling drupal_render

Reset the theme

drupal_theme_rebuild();
unset($theme);
unset($theme_info);
unset($base_theme_info);
unset($theme_engine);
unset($theme_path);
$theme = $custom_theme = $origtheme;

$theme = !empty($custom_theme) ? $custom_theme : $theme;

// Store the identifier for retrieving theme settings with.
$theme_key = $theme;

// Find all our ancestor themes and put them in an array.
$base_theme = array();
$ancestor = $theme;
while ($ancestor && isset($themes[$ancestor]->base_theme)) {
$ancestor = $themes[$ancestor]->base_theme;
$base_theme[] = $themes[$ancestor];
}
_drupal_theme_initialize($themes[$theme], array_reverse($base_theme));

// Themes can have alter functions, so reset the drupal_alter() cache.
drupal_static_reset('drupal_alter');

attiks’s picture

Status: Active » Needs review
FileSize
1.08 KB

Included a patch that changes drupal_theme_initialize() to drupal_theme_initialize($force_theme = '')

attiks’s picture

Status: Needs review » Needs work

This is more complex, patch in #2 doesn't always works, because
1/ function theme uses a static declaration
2/ function theme_get_registry uses a static declaration
3/ function _theme_registry_callback uses a static declaration

Is it possible to use http://api.drupal.org/api/drupal/includes--bootstrap.inc/function/drupal..., so those statics can be reset?

Jelle_S’s picture

Status: Needs work » Needs review
FileSize
2.86 KB

Added patch based on comment #3

Status: Needs review » Needs work

The last submitted patch, 981654-4.patch, failed testing.

Jelle_S’s picture

Status: Needs work » Needs review
FileSize
4.66 KB

previous patch failed because in theme_get_registry the function _theme_registry_callback would return a negative callback. This patch checks that. Also added drupal_theme_initialize('test_theme') to make sure all static variables are filled with the correct values.

Status: Needs review » Needs work

The last submitted patch, 981654-5.patch, failed testing.

Jelle_S’s picture

Status: Needs work » Needs review
FileSize
4.86 KB

The Simpletest frameworks resets static cache from time to time (drupal_static_reset()), so a check was added to ensure static cache isn't empty.

Status: Needs review » Needs work

The last submitted patch, 981654-6.patch, failed testing.

attiks’s picture

Status: Needs work » Needs review

testbot try again

attiks’s picture

#8: 981654-6.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 981654-6.patch, failed testing.

attiks’s picture

Status: Needs work » Needs review
FileSize
3.32 KB

Same patch without test, fingers crossed

Status: Needs review » Needs work

The last submitted patch, 981654-13.patch, failed testing.

attiks’s picture

Status: Needs work » Needs review

I'm baffled, testbot can't install this, on my local dev server i can install all three profiles without any problem ...

attiks’s picture

#8: 981654-6.patch queued for re-testing.

attiks’s picture

FYI, error is
Encountered error on [install], details:
array (
'@reason' => 'failed to set database information',
)

Jelle_S’s picture

FileSize
5.09 KB

from post #8 :

The Simpletest frameworks resets static cache from time to time (drupal_static_reset()), so a check was added to ensure static cache isn't empty

this check must always return true when installing, otherwise the system tries to go to the database, while there is no database specified yet. Fixed in this patch:

Jelle_S’s picture

FileSize
5.19 KB

forgot 'global $install_state'... This one really should work...

Status: Needs review » Needs work

The last submitted patch, 981654-19.patch, failed testing.

Jelle_S’s picture

FileSize
5.19 KB

From post #18:

from post #8 :

The Simpletest frameworks resets static cache from time to time (drupal_static_reset()), so a check was added to ensure static cache isn't empty

this check must always return true when installing, otherwise the system tries to go to the database, while there is no database specified yet. Fixed in this patch:

Correction: this check must always return true when installing AND updating. Fixed in this patch:

Jelle_S’s picture

Status: Needs work » Needs review
attiks’s picture

+++ includes/theme.inc
@@ -67,14 +67,34 @@ function _drupal_theme_access($theme) {
+  global $theme, $user, $theme_key, $install_state;

remove $install_state

+++ includes/theme.inc
@@ -67,14 +67,34 @@ function _drupal_theme_access($theme) {
+
+  }

remove blank line

+++ includes/theme.inc
@@ -67,14 +67,34 @@ function _drupal_theme_access($theme) {
+  else{

add space before {

+++ includes/theme.inc
@@ -67,14 +67,34 @@ function _drupal_theme_access($theme) {
+  elseif($force_theme != ''){

add spaces

+++ includes/theme.inc
@@ -242,11 +262,13 @@ function _drupal_theme_initialize($theme, $base_theme = array(), $registry_callb
+    if(isset($callback)){

add spaces

Powered by Dreditor.

attiks’s picture

Status: Needs review » Reviewed & tested by the community

I'm setting this to RTBC so we can get some feedback from others

Jelle_S’s picture

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

Cleaned up patch based on #23.

Status: Needs review » Needs work

The last submitted patch, 981654-25.patch, failed testing.

Jelle_S’s picture

FileSize
5.05 KB

Again, cleaned up patch based on #23.

Jelle_S’s picture

Status: Needs work » Needs review
attiks’s picture

Status: Needs review » Reviewed & tested by the community

I'm looking for feedback from anybody who wants to use theme switching in code

David_Rothstein’s picture

Title: $custom_theme is not usable anymore - regression » Theme system is initialized before some modules have the information they need to decide what theme to use
Status: Reviewed & tested by the community » Needs work

I don't think this issue is described correctly, or at least the patch and issue don't directly correspond.

The patch adds a feature where you can use two themes on the same page request. That is not fixing a regression - you could never do that in Drupal 6 either. Sure, you could switch the $custom_theme global all you want, but once you actually used it - i.e., by calling theme() - that is the theme you were stuck with.

Looking at the patch itself, this seems problematic at first glance:

+    drupal_static_reset('drupal_add_css');
+    drupal_static_reset('drupal_add_js');

Won't that erase any module-provided CSS or JS that was added to page previously? Given the number of places in core (not to mention contrib) that might be assuming there is only one theme used per page, this seems like more of a Drupal 8 feature.

I agree that the original use case in this issue is important to support. But I don't think it's that hard to do as a separate page request. During the form submission, you could put the data you need in $_SESSION and then redirect to a different menu callback which (after checking the data it expects is in $_SESSION) uses it to send the email. It's not super-pretty, but it should work. I could try to write up some sample code for that, but don't have time at the moment.

Keep in mind also that although it is less pretty, it is also more foolproof. The Drupal 6 code in the above example is fragile when combined with other modules. All you need is another module that happens to initialize the theme system before you do and the code will stop working. (See, for example, #219910: Calling theme function from hook_init() interferes with administration theme.)

David_Rothstein’s picture

I think if we wanted to look at a limited solution for Drupal 7 that would make this easier, we need to look at this code that happens during the _drupal_bootstrap_full() step:

    // Prior to invoking hook_init(), initialize the theme (potentially a custom
    // one for this page), so that:
    // - Modules with hook_init() implementations that call theme() or
    //   theme_get_registry() don't initialize the incorrect theme.
    // - The theme can have hook_*_alter() implementations affect page building
    //   (e.g., hook_form_alter(), hook_node_view_alter(), hook_page_alter()),
    //   ahead of when rendering starts.
    menu_set_custom_theme();
    drupal_theme_initialize();
    module_invoke_all('init');

Perhaps we could provide some simple way for modules to indicate that on a particular page request they want to "opt out" of the drupal_theme_initialize() call that happens there (it could even be as dumb as setting a global variable).

However, in doing that, the module would also have to accept the two consequences described in the above code comment. So I'm not sure if it's worth it, compared to writing a bit more code to use a separate request which avoids these problems. On the other hand, I can see how particular implementations (e.g., site-specific code) might not care, and would be happy to opt-in to something more like the "Drupal 6 way".

attiks’s picture

Status: Needs work » Needs review

David, thanks for the feedback and sorry for the late reaction.

There are actually 2 use cases for us:
1. As described above, switching the theme on a page request, this works as long as you take into account the order of execution, we build the email completely before trying to output anything
2. During cron runs, where hook_custom_theme doesn't work since we can not assume which other modules are using a certain theme. Your proposal in #31 won't help in this case either.

+ drupal_static_reset('drupal_add_css');
+ drupal_static_reset('drupal_add_js');

The is indeed pretty drastic, but needed because otherwise we will end up with css/js from another theme in the output of our emails.

The latest patch doesn't change anything to how D7 is working now, so as long as it isn't needed all works like expected, but I'm open to all feedback/help to solve this.

Damien Tournoud’s picture

Title: Theme system is initialized before some modules have the information they need to decide what theme to use » Use several themes during the same page request
Version: 7.x-dev » 8.x-dev
Category: bug » feature
Status: Needs review » Needs work

As described in #20, this is a feature request. Reassigning to Drupal 8. There is no way we can do that in Drupal 7, as the theme system is still not fully self-contained.

That said, from the top of my head, I feel that if you need that, you are doing something wrong. It feels really wrong to design a full theme just to send emails.

attiks’s picture

Version: 8.x-dev » 7.x-dev
Category: feature » bug

Damien, I hate to disagree, but in D6 is was possible to do this, maybe it wasn't meant to be, but it was possible to switch themes during cron run.

As to why want to do this: by allowing people to select a different theme to sent out emails, we can re-use a lot of 'standard' drupal functionality like blocks to construct (HTML) emails, this is a lot easier than trying to use one theme to style both the site and the emails, especially if you want your email to be readable in most email programs / web mail implementations.

Settings this back to D7, because I think this needs more attention before being set aside for D8

Damien Tournoud’s picture

Version: 7.x-dev » 8.x-dev
Category: bug » feature

As I said, it's not possible to do that properly either in Drupal 7 or below, because the theme system is still not self-contained.

I don't understand exactly what you need here that is not a simple page template. This is how MIME Mail works, and it definitely makes a lot of sense.

attiks’s picture

Version: 8.x-dev » 7.x-dev
Category: feature » bug

Damien,

It isn't only a page template but also node, comment, block, fields, views, ... templates and all of them are using the same information from the info file. It is a lot cleaner/easier to use a separate theme.

I know MIME Mail works like this, but overwriting everything in template files or using pre process hooks is - in my opinion - complicating things,

some of our use cases:
1/ send an email with the latest news (a view), the top rated product (a block) together with a short message (node body). The view, block and message are configurable by the site admin.
2/ Depending on the season/promotion the side admin can select a different look & feel for the emails or can change some settings of the 'email' theme at /admin/build/themes/settings/my-email-theme

Damien Tournoud’s picture

Version: 7.x-dev » 8.x-dev
Category: bug » feature

There is no reason to keep bumping this back to Drupal 7. As explained already, if it worked for you in Drupal 6, it was purely by accident. If anything initialized the theme system before your module in the same cron run (and in Drupal 6 it is as easy as calling t() with a % placeholder), you were completely screwed already. The only clean way to implement what you want in Drupal 7 and below (and I *still* believe it is not really a good idea) is to add a separate page callback.

Making the theme system completely self contained (and that means for example adding the theme information to the page object and removing the global drupal_add_(css|js)() calls) is a worthy goal... for Drupal 8.

dawehner’s picture

Issue summary: View changes
Status: Needs work » Fixed

The theme information is now an object which could be re-setted during runtime, this is though not the case for assets, yet,
but given the fact that you can render a piece of render array and get all the sub-attachments, I consider this to be fixed in D8.

Status: Fixed » Closed (fixed)

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