@berdir, @sgabe and me have been working in this functionality for mimemail and simplenews.

We all agree that this functionality will reside in a common mail module and as long as mimemail has already mailsystem as a dependency here is the patch to add this functionality.

It consist in altering the registry of every non-mail-theme to render using the mail theme configured.

We have been thinking of other aproaches, but finally decided to this one, any other points of view are welcome.

Any submodule that want to declare a theme registry to be rendered with the mail theme should add the key mail theme to its definition in hook_theme:

function hook_theme() {
  $theme['example_message'] = array(
    'variables' => array('key' => NULL, 'recipient' => NULL, 'subject' => NULL, 'body' => NULL),
    'template' => 'example-message',
    'pattern' => 'example_message__',
    'mail theme' => TRUE,
  );
  return $theme;
}
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jherencia’s picture

FileSize
2.84 KB
jherencia’s picture

FileSize
6.16 KB

I forgot to add a file, thanks @Berdir for the warning ;).

Berdir’s picture

Status: Needs review » Needs work
+++ b/mailsystem.theme.inc
@@ -0,0 +1,97 @@
+      foreach ($theme_registry as $name => $hook) {
+        if (!empty($hooks['mail theme'])) {
+          if (isset($cache[$name])) {
+            $cache[$name]['includes'][] = drupal_get_path('theme', $theme->name) . '/template.php';
+            foreach ($base_theme as $base) {
+              $cache[$name]['includes'][] = drupal_get_path('theme', $base->name) . '/template.php';
+            }
+            // Changing current registry for the new record.
+            $theme_registry[$name] = $cache[$name];
+          }

You will need to add an inner loop here and also look for everything that goes like $name . '__' to catch theme_hook_suggestions. See my patch.

+++ b/mailsystem.theme.inc
@@ -0,0 +1,97 @@
+      $theme = $theme_key;
+      break;
+    case 'domain_theme':
+      // Fetch the theme for the current domain.
+      if (module_exists('domain_theme')) {

you are using 'domain' for the options and 'domain_theme' here...

jherencia’s picture

Status: Needs work » Needs review
FileSize
6.79 KB

Thanks @Berdir, I added the changes you suggested.

Please see the refactor I made, maybe you would like to do it in simplenews too.

Schnitzel’s picture

+1 for patch at #4, used it together with http://drupal.org/node/374222#comment-5399034 and worked perfectly.

miro_dietiker’s picture

Status: Needs review » Reviewed & tested by the community

Did a code review, looks fine. We want to build Simplenews on this feature.
Would be great to see this feature in Mailsystem asap. :-)

sgabe’s picture

Second that. We would also like to use this in Mime Mail.

rooby’s picture

Works for me along with the simplenews patch.

Seeing as mimemail also implements a similar option at its level, maybe an issue should also be opened there to change to using this setting?

Berdir’s picture

pillarsdotnet’s picture

Sorry for being offline so long. Will apply outstanding patches later today if possible.

miro_dietiker’s picture

That's cool man :-) Looking forward to hear success messages .-)

s_leu’s picture

@pillarsdotnet

Did you already apply the patch?

miro_dietiker’s picture

This is blocking us from releasing beta in simplenews...
I'll open a ticket to add me as a co-maintainer for Mail System since this dependency is needed to make HTML rendering work reliably.

miro_dietiker’s picture

Referring to my application for maintainer state for project Mail System
#1412592: Co-Maintaining project Mailsystem

Would be great to see this confirmed.

pillarsdotnet’s picture

Sorry for being so unresponsive. I have added miro_dietiker as a maintainer.

pillarsdotnet’s picture

Assigned: Unassigned » pillarsdotnet

Reviewing/applying the patch now.

pillarsdotnet’s picture

Removing the chunk dealing with Domain Theme because that module is marked as abandoned/obsolete.
Nevermind -- I see that domain_theme is a submodule of domain

Seems like a poor naming choice. Doesn't this somehow clash with the theme hook of the Domain module?

pillarsdotnet’s picture

Also seems like this will make it difficult to set the theme on a per-email basis.

Berdir’s picture

I don't think doing that is possible in any way.

Once a theme is loaded it's there. You can't switch. It's just plain impossible. theme() has a static cache for this and doesn't use drupal_static(). See http://api.drupal.org/api/drupal/includes--theme.inc/function/theme/7

That's why we have to inject it into the theme registry like this in the first place :) No other approach works.

pillarsdotnet’s picture

Status: Reviewed & tested by the community » Needs work

Okay; I've made cosmetic changes, applied the patch and pushed to the 7.x-2.x branch.

Setting to "needs work" because now I need to generate 6.x and 8.x versions. Will roll new releases in all three branches after testing.

pillarsdotnet’s picture

@#19

Once a theme is loaded it's there. You can't switch. It's just plain impossible. theme() has a static cache for this and doesn't use drupal_static(). See http://api.drupal.org/api/drupal/includes--theme.inc/function/theme/7

That's why I wrote the Echo module.

pillarsdotnet’s picture

Version: 7.x-2.x-dev » 6.x-2.x-dev
Status: Needs work » Postponed (maintainer needs more info)

Okay, what is the D6 way of doing what the D7 hook_theme_registry_alter() does?

pillarsdotnet’s picture

Pushed updates to 6.x-2.x and 8.x-2.x branches, but haven't tested yet. The 6.x version will certainly fail because the necessary hooks don't exist.

sammys’s picture

pillarsdotnet’s picture

Status: Postponed (maintainer needs more info) » Needs work

@sammys -- I could have sworn the D6 hook didn't show up the last time I looked. Thanks.

pillarsdotnet’s picture

Component: Code » Documentation
Category: feature » task
Status: Needs work » Active

Okay, pushed new releases as follows:

6.x-2.31
7.x-2.31
8.x-2.31

Documentation and screenshots need updating.

pillarsdotnet’s picture

Issue summary: View changes

Added hook_theme definition example.

  • pillarsdotnet committed e564a7e on 8.x-4.x
    Issue #1382036 by jherencia, berdir, sgabe: Add a common way to...
Manuel Garcia’s picture

Issue summary: View changes
Status: Active » Closed (outdated)

The 6.x branch is no longer supported.