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.
Comment | File | Size | Author |
---|---|---|---|
#27 | 981654-27.patch | 5.05 KB | Jelle_S |
#25 | 981654-25.patch | 5.18 KB | Jelle_S |
#21 | 981654-21.patch | 5.19 KB | Jelle_S |
#19 | 981654-19.patch | 5.19 KB | Jelle_S |
#18 | 981654-18.patch | 5.09 KB | Jelle_S |
Comments
Comment #1
attiks CreditAttribution: attiks commentedI 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
Reset the theme
Comment #2
attiks CreditAttribution: attiks commentedIncluded a patch that changes drupal_theme_initialize() to drupal_theme_initialize($force_theme = '')
Comment #3
attiks CreditAttribution: attiks commentedThis 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?
Comment #4
Jelle_SAdded patch based on comment #3
Comment #6
Jelle_Sprevious 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.
Comment #8
Jelle_SThe Simpletest frameworks resets static cache from time to time (drupal_static_reset()), so a check was added to ensure static cache isn't empty.
Comment #10
attiks CreditAttribution: attiks commentedtestbot try again
Comment #11
attiks CreditAttribution: attiks commented#8: 981654-6.patch queued for re-testing.
Comment #13
attiks CreditAttribution: attiks commentedSame patch without test, fingers crossed
Comment #15
attiks CreditAttribution: attiks commentedI'm baffled, testbot can't install this, on my local dev server i can install all three profiles without any problem ...
Comment #16
attiks CreditAttribution: attiks commented#8: 981654-6.patch queued for re-testing.
Comment #17
attiks CreditAttribution: attiks commentedFYI, error is
Encountered error on [install], details:
array (
'@reason' => 'failed to set database information',
)
Comment #18
Jelle_Sfrom post #8 :
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:
Comment #19
Jelle_Sforgot 'global $install_state'... This one really should work...
Comment #21
Jelle_SFrom post #18:
Correction: this check must always return true when installing AND updating. Fixed in this patch:
Comment #22
Jelle_SComment #23
attiks CreditAttribution: attiks commentedremove $install_state
remove blank line
add space before {
add spaces
add spaces
Powered by Dreditor.
Comment #24
attiks CreditAttribution: attiks commentedI'm setting this to RTBC so we can get some feedback from others
Comment #25
Jelle_SCleaned up patch based on #23.
Comment #27
Jelle_SAgain, cleaned up patch based on #23.
Comment #28
Jelle_SComment #29
attiks CreditAttribution: attiks commentedI'm looking for feedback from anybody who wants to use theme switching in code
Comment #30
David_Rothstein CreditAttribution: David_Rothstein commentedI 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:
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.)
Comment #31
David_Rothstein CreditAttribution: David_Rothstein commentedI 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:
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".
Comment #32
attiks CreditAttribution: attiks commentedDavid, 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.
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.
Comment #33
Damien Tournoud CreditAttribution: Damien Tournoud commentedAs 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.
Comment #34
attiks CreditAttribution: attiks commentedDamien, 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
Comment #35
Damien Tournoud CreditAttribution: Damien Tournoud commentedAs 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.
Comment #36
attiks CreditAttribution: attiks commentedDamien,
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
Comment #37
Damien Tournoud CreditAttribution: Damien Tournoud commentedThere 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.
Comment #38
dawehnerThe 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.