From f2d81ad2a39f0fc7a48c0b66d14dca0b7aab5517 Mon Sep 17 00:00:00 2001
From: Timothy Asp <tim@ifixit.com>
Date: Tue, 4 Nov 2014 12:09:36 -0700
Subject: [PATCH] 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
---
 Classes/Emogrifier.php        | 38 +++++++++++++++++++++++---------
 Tests/Unit/EmogrifierTest.php | 41 +++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 10 deletions(-)

diff --git a/Classes/Emogrifier.php b/Classes/Emogrifier.php
index 479dfbde..aea4fa33 100644
--- 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
index 9016a2f4..3f92fb22 100644
--- 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()
+        );
+    }
 }