Skip to content

Commit

Permalink
Add removeDisplayNone option + tests
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@timothyasp
timothyasp committed May 27, 2015
1 parent ed72bcd commit 569fdcd
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 26 deletions.
68 changes: 51 additions & 17 deletions Classes/Emogrifier.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,14 @@ class Emogrifier
*/
private $isStyleBlocksParsingEnabled = true;

/**
* Determines whether elements with the `display: none` property are
* removed from the DOM.
*
* @var bool
*/
private $shouldKeepInvisibleNodes = true;

/**
* This attribute applies to the case where you want to preserve your original text encoding.
*
Expand Down Expand Up @@ -289,23 +297,8 @@ public function emogrify()
$this->fillStyleAttributesWithMergedStyles();
}

// This removes styles from your email that contain display:none.
// We need to look for display:none, but we need to do a case-insensitive search. Since DOMDocument only
// 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->shouldKeepInvisibleNodes) {
$this->removeInvisibleNodes($xpath);
}

$this->copyCssWithMediaToStyleNode($cssParts, $xmlDocument);
Expand Down Expand Up @@ -337,6 +330,16 @@ public function disableStyleBlocksParsing()
$this->isStyleBlocksParsingEnabled = false;
}

/**
* Disables the removal of elements with `display: none` properties.
*
* @return void
*/
public function disableInvisibleNodeRemoval()
{
$this->shouldKeepInvisibleNodes = false;
}

/**
* Clears all caches.
*
Expand Down Expand Up @@ -418,6 +421,37 @@ public function removeUnprocessableHtmlTag($tagName)
}
}

/**
* This removes styles from your email that contain display:none.
* We need to look for display:none, but we need to do a case-insensitive search. Since DOMDocument only
* 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.
*
* @param \DOMXPath $xpath
*
* @return void
*/
private function removeInvisibleNodes(\DOMXPath $xpath)
{
$nodesWithStyleDisplayNone = $xpath->query(
'//*[contains(translate(translate(@style," ",""),"NOE","noe"),"display:none")]'
);

if ($nodesWithStyleDisplayNone->length === 0) {
return;
}

// 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
/** @var \DOMNode $node */
foreach ($nodesWithStyleDisplayNone as $node) {
if ($node->parentNode && is_callable(array($node->parentNode, 'removeChild'))) {
$node->parentNode->removeChild($node);
}
}
}

/**
* Normalizes the value of the "style" attribute and saves it.
*
Expand Down
21 changes: 12 additions & 9 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,22 @@ After you have set the HTML and CSS, you can call the `emogrify` method to merge

## Options

There are several options that you can set on the Emogrifier object before
There are several options that you can set on the Emogrifier object before
calling the `emogrify` method:

* `$emogrifier->disableStyleBlocksParsing()` - By default, Emogrifier will grab
all `<style>` blocks in the HTML and will apply the CSS styles as inline
"style" attributes to the HTML. The `<style>` blocks will then be removed
"style" attributes to the HTML. The `<style>` blocks will then be removed
from the HTML. If you want to disable this functionality so that Emogrifier
leaves these `<style>` blocks in the HTML and does not parse them, you should
use this option.
* `$emogrifier->disableInlineStylesParsing()` - By default, Emogrifier
preserves all of the "style" attributes on tags in the HTML you pass to it.
However if you want to discard all existing inline styles in the HTML before
the CSS is applied, you should use this option.
* `$emogrifier->disableInvisibleNodeRemoval()` - By default, Emogrifier removes
elements from the DOM that have the style attribute `display: none;`. If
you would like to keep invisible elements in the DOM, use this option.


## Installing with Composer
Expand Down Expand Up @@ -163,18 +166,18 @@ Those that wish to contribute bug fixes, new features, refactorings and clean-up
When you contribute, please take the following things into account:

* Please cover all changes with unit tests and make sure that your code does not break any existing tests.
* To run the existing PHPUnit tests, run the following
* To run the existing PHPUnit tests, run the following
command:

composer install

and then run this command:

vendor/bin/phpunit Tests/
If you want to uninstall the development packages that were installed when

If you want to uninstall the development packages that were installed when
you ran `composer install`, you can run this command:

composer install --no-dev
* Please use the same coding style (PSR-2) as the rest of the code. Indentation is four spaces.
* Please make your code clean, well-readable and easy to understand.
Expand Down
60 changes: 60 additions & 0 deletions Tests/Unit/EmogrifierTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1090,4 +1090,64 @@ public function emogrifyMergesCssWithMixedUnits()
$this->subject->emogrify()
);
}

/**
* @test
*/
public function emogrifyByDefaultRemovesElementsWithDisplayNoneFromExternalCss()
{
$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 emogrifyByDefaultRemovesElementsWithDisplayNoneInStyleAttribute()
{
$html = $this->html5DocumentType . self::LF .
'<html><body><div class="bar"></div><div class="foobar" style="display: none;"></div>'
. '<div class="foo"></div></body></html>';

$expected = '<div class="bar"></div><div class="foo"></div>';

$this->subject->setHtml($html);

$this->assertContains(
$expected,
$this->subject->emogrify()
);
}

/**
* @test
*/
public function emogrifyAfterDisableInvisibleNodeRemovalPreservesInvisibleElements()
{
$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->disableInvisibleNodeRemoval();

$this->assertContains(
$expected,
$this->subject->emogrify()
);
}
}

0 comments on commit 569fdcd

Please sign in to comment.