Skip to content

Commit

Permalink
[BUGFIX] Allow @charset without error
Browse files Browse the repository at this point in the history
Filter out `@charset` at-rules from the CSS before processing, to avoid an error
when presented with CSS containing this at-rule.

Obviously this does not mean `@charset` is supported - its value is still
ignored - but does address the immediate error in MyIntervals#296.

Added PHPUnit test like that for `@import`, and also test that such CSS content
does not prevent Emogrifier working correctly on the remainder of the CSS.
  • Loading branch information
JakeQZ committed Feb 8, 2018
1 parent 2c2b785 commit 2c48871
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
### Removed

### Fixed
- Allow `@charset` in the CSS without error (note: its value is currently
ignored, [#507](https://github.com/MyIntervals/emogrifier/pull/507))
- Allow attribute selectors in descendants
([#506](https://github.com/MyIntervals/emogrifier/pull/506),
[#381](https://github.com/MyIntervals/emogrifier/issues/381))
Expand Down
2 changes: 1 addition & 1 deletion src/Emogrifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -1258,7 +1258,7 @@ function ($matches) use (&$media) {

// filter the CSS
$search = [
'import directives' => '/^\\s*@import\\s[^;]+;/misU',
'import/charset directives' => '/^\\s*@(?:import|charset)\\s[^;]+;/misU',
'remaining media enclosures' => '/^\\s*@media\\s[^{]+{(.*)}\\s*}\\s/misU',
];

Expand Down
18 changes: 18 additions & 0 deletions tests/Unit/EmogrifierTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,7 @@ public function unneededCssThingsDataProvider()
'CSS comments with one asterisk' => ['p {color: #000;/* black */}', 'black'],
'CSS comments with two asterisks' => ['p {color: #000;/** black */}', 'black'],
'@import directive' => ['@import "foo.css";', '@import'],
'@charset directive' => ['@charset "UTF-8";', '@charset'],
'style in "aural" media type rule' => ['@media aural {p {color: #000;}}', '#000'],
'style in "braille" media type rule' => ['@media braille {p {color: #000;}}', '#000'],
'style in "embossed" media type rule' => ['@media embossed {p {color: #000;}}', '#000'],
Expand Down Expand Up @@ -1083,6 +1084,23 @@ public function emogrifyFiltersUnneededCssThings($css, $markerNotExpectedInHtml)
static::assertNotContains($markerNotExpectedInHtml, $result);
}

/**
* @test
*
* @param string $css
*
* @dataProvider unneededCssThingsDataProvider
*/
public function emogrifyUnaffectedByUnneededCssThings($css)
{
$this->subject->setHtml('<html><body></body></html>');
$this->subject->setCss($css . ' body { color: green; }');

$result = $this->subject->emogrify();

static::assertContains('<body style="color: green;">', $result);
}

/**
* Data provider for media rules.
*
Expand Down

0 comments on commit 2c48871

Please sign in to comment.