Add removeDisplayNone option + tests

This adds an options parameter and some tests for a `removeDisplayNone` option which specifies if the Emogrifier parser will remove elements with the style "display: none". Unfortunatly, the nice little optimization has some consiquences. When writing responsive email templates, "display: none" is used often to hide certain sections of the template for desktop email clients and show them in phone clients. I can see how it'd be nice to clean up elements that will not be displayed in some cases, so that's why I made it an options field. Conflicts: Classes/Emogrifier.php Tests/Unit/EmogrifierTest.php
MyIntervals · May 15, 2015 · f2d81ad · f2d81ad
1 parent ed72bcd
commit f2d81ad
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 10 deletions.
diff --git a/Classes/Emogrifier.php b/Classes/Emogrifier.php
@@ -121,6 +121,12 @@ class Emogrifier
      */
     private $isStyleBlocksParsingEnabled = true;
 
+    /**
+     * Determines whether elements with the `display: none` property are
+     * removed from the DOM.
+     */
+    private $isRemoveDisplayNoneEnabled = true;
+
     /**
      * This attribute applies to the case where you want to preserve your original text encoding.
      *
@@ -294,16 +300,18 @@ public function emogrify()
         // supports XPath 1.0, lower-case() isn't available to us. We've thus far only set attributes to lowercase,
         // not attribute values. Consequently, we need to translate() the letters that would be in 'NONE' ("NOE")
         // to lowercase.
-        $nodesWithStyleDisplayNone = $xpath->query(
-            '//*[contains(translate(translate(@style," ",""),"NOE","noe"),"display:none")]'
-        );
-        // The checks on parentNode and is_callable below ensure that if we've deleted the parent node,
-        // we don't try to call removeChild on a nonexistent child node
-        if ($nodesWithStyleDisplayNone->length > 0) {
-            /** @var \DOMNode $node */
-            foreach ($nodesWithStyleDisplayNone as $node) {
-                if ($node->parentNode && is_callable(array($node->parentNode,'removeChild'))) {
-                    $node->parentNode->removeChild($node);
+        if ($this->isRemoveDisplayNoneEnabled) {
+            $nodesWithStyleDisplayNone = $xpath->query(
+                '//*[contains(translate(translate(@style," ",""),"NOE","noe"),"display:none")]'
+            );
+            // The checks on parentNode and is_callable below ensure that if we've deleted the parent node,
+            // we don't try to call removeChild on a nonexistent child node
+            if ($nodesWithStyleDisplayNone->length > 0) {
+                /** @var $node \DOMNode */
+                foreach ($nodesWithStyleDisplayNone as $node) {
+                    if ($node->parentNode && is_callable(array($node->parentNode,'removeChild'))) {
+                        $node->parentNode->removeChild($node);
+                    }
                 }
             }
         }
@@ -337,6 +345,16 @@ public function disableStyleBlocksParsing()
         $this->isStyleBlocksParsingEnabled = false;
     }
 
+    /**
+     * Disables the removal of elements with `display: none` properties
+     *
+     * @return void
+     */
+    public function disableRemoveDisplayNone()
+    {
+        $this->isRemoveDisplayNoneEnabled = false;
+    }
+
     /**
      * Clears all caches.
      *

diff --git a/Tests/Unit/EmogrifierTest.php b/Tests/Unit/EmogrifierTest.php
@@ -1090,4 +1090,45 @@ public function emogrifyMergesCssWithMixedUnits()
             $this->subject->emogrify()
         );
     }
+
+    /**
+     * @test
+     */
+    public function emogrifyRemovesDisplayNoneElements()
+    {
+        $css = 'div.foo { display: none; }';
+        $html = $this->html5DocumentType . self::LF .
+            '<html><body><div class="bar"></div><div class="foo"></div></body></html>';
+
+        $expected = '<div class="bar"></div>';
+
+        $this->subject->setHtml($html);
+        $this->subject->setCss($css);
+
+        $this->assertContains(
+            $expected,
+            $this->subject->emogrify()
+        );
+    }
+
+    /**
+     * @test
+     */
+    public function emogrifyPreservesDisplayNoneElements()
+    {
+        $css = 'div.foo { display: none; }';
+        $html = $this->html5DocumentType . self::LF .
+            '<html><body><div class="bar"></div><div class="foo"></div></body></html>';
+
+        $expected = '<div class="foo" style="display: none;">';
+
+        $this->subject->setHtml($html);
+        $this->subject->setCss($css);
+        $this->subject->disableRemoveDisplayNone();
+
+        $this->assertContains(
+            $expected,
+            $this->subject->emogrify()
+        );
+    }
 }