Update CSS Compressor

subscribing,

I hve the same problem.
A solution would be to define one specific css for the newsletter css compressor, instead of all css of the website. this way, descendent selectors wouldn't be so essentials; and the newsletter wouldn't be polluted by all the other css properties that are note concerning it. and it would be lighter.

;)

@seren10pity:

A solution would be to define one specific css for the newsletter css compressor, instead of all css of the website.

You can use a custom mail.css placed in your theme folder, instead of the site style sheets.

The CSS Compressor is based on Emogrifier and the code in the module needs to be updated. After a quick test with -Mania-'s example, it seems for me the latest version supports descendant selectors.

I am attaching a patch with the updated code. Please, test it and report back.

Attaching new patch against current HEAD.

• Fixed indentation and code formatting.
• Respected committed bug fixes:
  • #729334: Flawed CSS to XPath conversion for class selectors in Mime Mail CSS Compressor
  • #700996: Custom inline style properties overwritten when using CSS Compressor

Note that there is another related issue: #517306: Mime Mail Compress mangles absolute URLs in CSS properties

Rerolled patch after alpha5 for HEAD.

Please ignore the previous patch.

+++ modules/mimemail_compress/mimemail_compress.inc	15 Aug 2010 18:19:39 -0000
@@ -1,79 +1,169 @@
+    $encoding = mb_detect_encoding($body);
+    $body = mb_convert_encoding($body, 'HTML-ENTITIES', $encoding);
...
       $doc = new DOMDocument('1.0', 'utf8');

Can you explain why you left the encoding detection in but don't apply it to $doc anymore, like the original emogrifier.php does?

+++ modules/mimemail_compress/mimemail_compress.inc	15 Aug 2010 18:19:39 -0000
@@ -1,79 +1,169 @@
     if (class_exists('DOMDocument', FALSE)) {

Can we move this to the top of the function, please? Like so:

    if (!class_exists('DOMDocument', FALSE)) {
      return $this->html;
    }

+++ modules/mimemail_compress/mimemail_compress.inc	15 Aug 2010 18:19:39 -0000
@@ -1,79 +1,169 @@
-              $new_style = $this->css_style_to_array($matches[2][$key]);

$matches[2][$key] is now $value['attributes'].

+++ modules/mimemail_compress/mimemail_compress.inc	15 Aug 2010 18:19:39 -0000
@@ -1,79 +1,169 @@
-              $compressed = array_merge($new_style, $old_style);

In the original emogrifier the order is new overwrites old styles. Can you explain why you left the current behavior?

I know both have disadvantages: new over old doesn't keep CSS styles set in the style attribute, i.e. CSS selectors always overwrite existing styles (which is against the specs).
The current behavior, old over new, on the other side, means having to write CSS selectors in upside-down flipped order, which is very counter-intuitive.

+++ modules/mimemail_compress/mimemail_compress.inc	15 Aug 2010 18:19:39 -0000
@@ -1,79 +1,169 @@
-              foreach ($compressed as $k => $v) $style .= ($k . ':' . $v . ';');

strtolower() around $k is missing here.

+++ modules/mimemail_compress/mimemail_compress.inc	15 Aug 2010 18:19:39 -0000
@@ -1,79 +1,169 @@
+      $nodes = $xpath->query('//*[contains(translate(@style," ",""), "display:none;")]');

Whitespace before and semicolon after "display:none" doesn't exist in original emogrifier.

+++ modules/mimemail_compress/mimemail_compress.inc	15 Aug 2010 18:19:39 -0000
@@ -81,28 +171,63 @@
+  private static function sort_selector_precedence($a, $b) {

Can we make this sort_by_selector_precedence(), please?

+++ modules/mimemail_compress/mimemail_compress.inc	15 Aug 2010 18:19:39 -0000
@@ -81,28 +171,63 @@
       preg_match("/([^:]+):(.+)/", $def, $matches);
       list( , $key, $value) = $matches;

This has been changed to list($key, $value) = explode(':', $def, 2); in the original emogrifier.

Powered by Dreditor.

I should note that with explain I meant add a comment/reference to a Drupal issue, if there is any. Otherwise nobody knows where the differences are coming from in the long run.

Thanks for reviewing, I see I missed some things. So, I am attaching a new patch regarding your comment.

• Applied encoding detection.
• DOMDocument check moved to the top of compress()
• About the new and old styles merging see #882960: CSS Mail Style Sheet Overrides. This will be reverted.
• Added some whitespace to respect the coding standards.
• I think sort_by_selector_precedence() would be unnecessary verbose.
• I see the original Emogrifier is solved #517306: Mime Mail Compress mangles absolute URLs in CSS properties, then we shall use that.

Changed title to a more descriptive one. Attached a new patch with a bug fix (unnecessary bracket breaks it totally).

Committed to HEAD.