I am using simplenews without hindrance on one of my sites. However, I'm experiencing a strange thing on a different site. I was investigating why a seemingly small newsletter was 170kb in size (where it should be more like 30kb). I discovered in the source of the email that just about all my drupal site's css is being embedded in the head tag. My theme's css, and even some module's stylesheets.

I am quite befuddled, as the operational newsletter on the one site is doing great, and certainly not embedded stuff where it shouldn't.

Can anyone steer me in a direction?

much appreciated
Teezi

Comments

Sutharsan’s picture

The direction is Mime Mail module settings.

Teezi’s picture

Thanks for the tip, but there's nothing of use in the Mimemail settings. This newsletter needs to be HTML.

Teezi’s picture

Still need to find the reason for this happening....

Teezi’s picture

My humblest apologies. The direction WAS Mimemail, after all. Seems the mimemail.tpl.php was the culprit. Fixed now.

Thanks!

Sutharsan’s picture

Project:Simplenews» Mime Mail
Version:6.x-1.0» 6.x-1.x-dev

Could this have been prevented with better documentation? Please help to improve, any suggestion is better than no suggestion.

Michelle’s picture

A setting would be nice. I am using Mime Mail with Simplenews because I want links to show up as linked text, not written out links. But I certainly don't want to cram all of my site's CSS into a newsletter. I see it can be changed with theming but this is really crying out for a checkbox on the settings page to include or not include the site's CSS and let more specific tailoring be done at the theme level from there.

Michelle

sgabe’s picture

Title:Site's css embedded in newsletter» Optional site's css embedding
Version:6.x-1.x-dev» 6.x-1.0-alpha1
Category:support» feature
Status:Active» Needs review
StatusFileSize
new2.98 KB

The attached patch provides the feature that Michelle described above.

I am changing version and category, since the development snapshot is older than the current alpha release (the patch applies to that) and this is rather a feature, than a support request.

Please review it and give feedback.

JGonzalez’s picture

The patch works great for what is supposed to do.

AlexisWilke’s picture

Actually, you can create an empty mail.css in your theme and it would reduce the style to nothing... or whatever you put in your mail.css file.

The problem with a flag is if you have 3 newsletters then all 3 are affected. There is an issue with that too though since it would be useful to have 3 different mail.css files assigned depending on the newsletter term... Hmmm...

Wim Leers’s picture

Version:6.x-1.0-alpha1» 6.x-1.0-alpha5
StatusFileSize
new3.13 KB

Reroll of the patch in #7. It indeed works great. What's the reason for not yet committing this?

@AlexisWilke: that's correct, but you can simply choose *not* to use that flag. This simple flag solves the problem for most users, thus it's an acceptable compromise for now.

Michelle’s picture

I agree with Wim Leers. If you have 3 newsletters and really need to get detailed, you can always fall back to the theming option. A setting works for people who just want to put a newsletter on their site and keep it simple.

Michelle

sgabe’s picture

@Wim Leers: I want to kill most of the bugs and make sure the module is stable enough before implementing any new features, that's top priority now. However we are getting nearer and nearer to the point where we can add new features.

sgabe’s picture

Version:6.x-1.0-alpha5» 7.x-1.x-dev
StatusFileSize
new3.36 KB

Attaching a revised patch of #10.

  • Don't change the default value of the alteration option. (My first patch set the default to TRUE which is wrong).
  • Delete the mimemail_sitestyle variable on uninstall.
  • Corrected code comments.

Will go into HEAD.

AlexisWilke’s picture

sgabe,

I see that this patch checks whether the mail.css file exists. Not only that, it is a copy and paste of the code which uses:

<?php
$theme
= variable_get('theme_default', NULL);
?>

I'm thinking that you want to write a function that tells you what the .css files are. A function that returns an array with all the .css filenames to be used with the current global $theme. Maybe with a flag to say that you only want to know whether there is a mail.css file.

Yet, the current theme (global $theme) may not be the one that is current when sending the email. So instead you should present a list of themes/flags to be correct.

Thank you.
Alexis

P.S. see also: #883284: Wrong theme selection in template_preprocess_mimemail_message() and #338460: Doens't look for mail.css in Zen subthemes: suggesting better integration with Zen theme

fmjrey’s picture

I don't know much about drupal/php/theming but it seems there could be some inspiration to be taken from ckeditor.module code:

    // add custom stylesheet if configured
    // lets hope it exists but we'll leave that to the site admin
    $css_files = array();
    switch ($conf['css_mode']) {
      case 'theme':
        global $language, $theme_info, $base_theme_info;

        if (!empty($theme_info->stylesheets)) {
          $editorcss = """;
          foreach ($base_theme_info as $base) { // Grab stylesheets from base theme
            if (!empty($base->stylesheets)) { // may be empty when the base theme reference in the info file is invalid
              foreach ($base->stylesheets as $type => $stylesheets) {
                if ($type != "print") {
                  foreach ($stylesheets as $name => $path) {
                    if (file_exists($path)) {
                      $css_files[$name] = $host . $path;
                      // Grab rtl stylesheets ( will get rtl css files when thay are named with suffix "-rtl.css" (ex: fusion baased themes) )
                      if (defined('LANGUAGE_RTL') && $language->direction == LANGUAGE_RTL && substr($path,0,-8) != "-rtl.css") {
                        $rtl_path = substr($path,0,-4)."-rtl.css";
                        if (file_exists($rtl_path)) {
                          $css_files[$name."-rtl"] = $host . $rtl_path;
                        }
                      }
                    }
                  }
                }
              }
            }
          }
          if (!empty($theme_info->stylesheets)) { // Grab stylesheets from current theme
            foreach ($theme_info->stylesheets as $type => $stylesheets) {
              if ($type != "print") {
                foreach ($stylesheets as $name => $path) {
                  if (file_exists($path)) {
                    $css_files[$name] = $host . $path;
                    // Grab rtl stylesheets ( will get rtl css files when thay are named with suffix "-rtl.css" (ex: fusion baased themes) )
                    if (defined('LANGUAGE_RTL') && $language->direction == LANGUAGE_RTL && substr($path,0,-8) != "-rtl.css") {
                      $rtl_path = substr($path,0,-4)."-rtl.css";
                      if (file_exists($rtl_path)) {
                        $css_files[$name."-rtl"] = $host . $rtl_path;
                      }
                    }
                  }
                  elseif (!empty($css_files[$name])) {
                    unset($css_files[$name]);
                  }
                }
              }
            }
          }
          // Grab stylesheets local.css and local-rtl.css if they exist (fusion based themes)
          if (file_exists($themepath . 'css/local.css')) {
            $css_files[] = $host . $themepath . 'css/local.css';
          }
          if (defined('LANGUAGE_RTL') && $language->direction == LANGUAGE_RTL && file_exists($themepath . 'css/local-rtl.css')) {
            $css_files[] = $host . $themepath . 'css/local-rtl.css';
          }
         
          // Grab stylesheets from color module
          $color_paths = variable_get('color_'. $theme .'_stylesheets', array());
          if (defined('LANGUAGE_RTL') && $language->direction == LANGUAGE_RTL) {
            if (!empty($color_paths[1])) {
              $css_files[] = $host . $color_paths[1];
            }
          }
          elseif (!empty($color_paths[0])) {
            $css_files[] = $host . $color_paths[0];
          }
        }
        else {
          if (file_exists($themepath .'style.css')) {
            $css_files[] = $host . $themepath .'style.css';
          }
        }
        $css_files[] = $module_full_path ."/ckeditor.css";
        break;

      case 'self':
        if (file_exists($module_drupal_path .'/ckeditor.css')) {
          $css_files[] = $module_full_path .'/ckeditor.css';
        }
        foreach (explode(',', $conf['css_path']) as $css_path) {
          $css_path = trim(str_replace("%h%t", "%t", $css_path));
          $css_files[] = str_replace(array('%h', '%t'), array($host, $host . $themepath), $css_path);
        }
        break;

      case 'none':
        if (file_exists($module_drupal_path .'/ckeditor.css')) {
          $css_files[] = $module_full_path .'/ckeditor.css';
        }
        $css_files[] = $editor_path .'/contents.css';
        break;
    }

The above code contains ckeditor's patch found in #818378: Not all css files are loaded.
With that patch ckeditor also uses Fusion css/local.css for styling text in the editor. The local.css file in Fusion is used to override styles so it's quite a big deal to not include it when the theme has been customized.
Right now Mimemail also ignores the file css/local.css in Fusion.
So with all these patches and code around, couldn't we find a final fix for this?

sgabe’s picture

@fmjrey: Please, open a new issue for this.

fmjrey’s picture

sgabe’s picture

Status:Needs review» Fixed

Committed to HEAD with minor changes.

Status:Fixed» Closed (fixed)

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