Skip to content

Commit

Permalink
[CLEANUP] Add DeclarationBlockParser class
Browse files Browse the repository at this point in the history
This is essentially a refactoring to move the `parseCssDeclarationsBlock` method
that was duplicated in both `CssInliner` and `CssToAttributeConverter` to a
separate class.

Doing so also allows it to be tested independently, which is duly done.
Additionally, it allows the results cache to be shared.

This is also likely to be used by the solution for #1276 (as currently
proposed).

Longer-term, we should be able to use functionality from PHP-CSS-Parser for more
accurate parsing (e.g. strings containing semicolons), whereupon this class can
become a wrapper for that, but currently that functionality is not available in
a standalone class.

Any PHPStan errors from the original function have been fixed, including
handling the failure case of `preg_split` (and using `trigger_error` in that
case).

There is a false positive from PHPStan for which there is an open ticket:
phpstan/phpstan#7799.
  • Loading branch information
JakeQZ committed Sep 14, 2024
1 parent deefcc4 commit ea7d2cd
Show file tree
Hide file tree
Showing 5 changed files with 249 additions and 113 deletions.
16 changes: 8 additions & 8 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ parameters:

-
message: "#^Argument of an invalid type array\\<int, string\\>\\|false supplied for foreach, only iterables are supported\\.$#"
count: 2
count: 1
path: src/CssInliner.php

-
Expand All @@ -17,7 +17,7 @@ parameters:

-
message: "#^Only booleans are allowed in a negated boolean, int\\|false given\\.$#"
count: 3
count: 2
path: src/CssInliner.php

-
Expand Down Expand Up @@ -45,11 +45,6 @@ parameters:
count: 2
path: src/HtmlProcessor/AbstractHtmlProcessor.php

-
message: "#^Argument of an invalid type array\\<int, string\\>\\|false supplied for foreach, only iterables are supported\\.$#"
count: 1
path: src/HtmlProcessor/CssToAttributeConverter.php

-
message: "#^Method Pelago\\\\Emogrifier\\\\HtmlProcessor\\\\CssToAttributeConverter\\:\\:getAllNodesWithStyleAttribute\\(\\) return type with generic class DOMNodeList does not specify its types\\: TNode$#"
count: 1
Expand All @@ -62,7 +57,7 @@ parameters:

-
message: "#^Only booleans are allowed in a negated boolean, int\\|false given\\.$#"
count: 2
count: 1
path: src/HtmlProcessor/CssToAttributeConverter.php

-
Expand Down Expand Up @@ -115,6 +110,11 @@ parameters:
count: 1
path: src/Utilities/CssConcatenator.php

-
message: "#^Unreachable statement \\- code above always terminates\\.$#"
count: 1
path: src/Utilities/DeclarationBlockParser.php

-
message: "#^Offset 0 does not exist on array\\{0\\?\\: string\\}\\.$#"
count: 1
Expand Down
68 changes: 11 additions & 57 deletions src/CssInliner.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Pelago\Emogrifier\Css\CssDocument;
use Pelago\Emogrifier\HtmlProcessor\AbstractHtmlProcessor;
use Pelago\Emogrifier\Utilities\CssConcatenator;
use Pelago\Emogrifier\Utilities\DeclarationBlockParser;
use Pelago\Emogrifier\Utilities\Preg;
use Symfony\Component\CssSelector\CssSelectorConverter;
use Symfony\Component\CssSelector\Exception\ParseException;
Expand All @@ -24,12 +25,7 @@ class CssInliner extends AbstractHtmlProcessor
/**
* @var int
*/
private const CACHE_KEY_CSS_DECLARATIONS_BLOCK = 1;

/**
* @var int
*/
private const CACHE_KEY_COMBINED_STYLES = 2;
private const CACHE_KEY_COMBINED_STYLES = 1;

/**
* Regular expression component matching a static pseudo class in a selector, without the preceding ":",
Expand Down Expand Up @@ -75,13 +71,11 @@ class CssInliner extends AbstractHtmlProcessor
/**
* @var array{
* 0: array<string, int>,
* 1: array<string, array<string, string>>,
* 2: array<string, string>
* 1: array<string, string>
* }
*/
private $caches = [
self::CACHE_KEY_SELECTOR => [],
self::CACHE_KEY_CSS_DECLARATIONS_BLOCK => [],
self::CACHE_KEY_COMBINED_STYLES => [],
];

Expand Down Expand Up @@ -393,7 +387,6 @@ private function clearAllCaches(): void
{
$this->caches = [
self::CACHE_KEY_SELECTOR => [],
self::CACHE_KEY_CSS_DECLARATIONS_BLOCK => [],
self::CACHE_KEY_COMBINED_STYLES => [],
];
}
Expand Down Expand Up @@ -465,55 +458,13 @@ static function (array $propertyNameMatches): string {
// In order to not overwrite existing style attributes in the HTML, we have to save the original HTML styles.
$nodePath = $node->getNodePath();
if (\is_string($nodePath) && !isset($this->styleAttributesForNodes[$nodePath])) {
$this->styleAttributesForNodes[$nodePath] = $this->parseCssDeclarationsBlock($normalizedOriginalStyle);
$this->styleAttributesForNodes[$nodePath] = (new DeclarationBlockParser())->parse($normalizedOriginalStyle);
$this->visitedNodes[$nodePath] = $node;
}

$node->setAttribute('style', $normalizedOriginalStyle);
}

/**
* Parses a CSS declaration block into property name/value pairs.
*
* Example:
*
* The declaration block
*
* "color: #000; font-weight: bold;"
*
* will be parsed into the following array:
*
* "color" => "#000"
* "font-weight" => "bold"
*
* @param string $cssDeclarationsBlock the CSS declarations block without the curly braces, may be empty
*
* @return array<string, string>
* the CSS declarations with the property names as array keys and the property values as array values
*/
private function parseCssDeclarationsBlock(string $cssDeclarationsBlock): array
{
if (isset($this->caches[self::CACHE_KEY_CSS_DECLARATIONS_BLOCK][$cssDeclarationsBlock])) {
return $this->caches[self::CACHE_KEY_CSS_DECLARATIONS_BLOCK][$cssDeclarationsBlock];
}

$properties = [];
foreach (\preg_split('/;(?!base64|charset)/', $cssDeclarationsBlock) as $declaration) {
/** @var list<string> $matches */
$matches = [];
if (!\preg_match('/^([A-Za-z\\-]+)\\s*:\\s*(.+)$/s', \trim($declaration), $matches)) {
continue;
}

$propertyName = \strtolower($matches[1]);
$propertyValue = $matches[2];
$properties[$propertyName] = $propertyValue;
}
$this->caches[self::CACHE_KEY_CSS_DECLARATIONS_BLOCK][$cssDeclarationsBlock] = $properties;

return $properties;
}

/**
* Returns CSS content.
*
Expand Down Expand Up @@ -781,15 +732,16 @@ private function getCssSelectorPrecedence(string $selector): int
private function copyInlinableCssToStyleAttribute(\DOMElement $node, array $cssRule): void
{
$declarationsBlock = $cssRule['declarationsBlock'];
$newStyleDeclarations = $this->parseCssDeclarationsBlock($declarationsBlock);
$declarationBlockParser = new DeclarationBlockParser();
$newStyleDeclarations = $declarationBlockParser->parse($declarationsBlock);
if ($newStyleDeclarations === []) {
return;
}

// if it has a style attribute, get it, process it, and append (overwrite) new stuff
if ($node->hasAttribute('style')) {
// break it up into an associative array
$oldStyleDeclarations = $this->parseCssDeclarationsBlock($node->getAttribute('style'));
$oldStyleDeclarations = $declarationBlockParser->parse($node->getAttribute('style'));
} else {
$oldStyleDeclarations = [];
}
Expand Down Expand Up @@ -864,9 +816,10 @@ private function attributeValueIsImportant(string $attributeValue): bool
*/
private function fillStyleAttributesWithMergedStyles(): void
{
$declarationBlockParser = new DeclarationBlockParser();
foreach ($this->styleAttributesForNodes as $nodePath => $styleAttributesForNode) {
$node = $this->visitedNodes[$nodePath];
$currentStyleAttributes = $this->parseCssDeclarationsBlock($node->getAttribute('style'));
$currentStyleAttributes = $declarationBlockParser->parse($node->getAttribute('style'));
$node->setAttribute(
'style',
$this->generateStyleStringFromDeclarationsArrays(
Expand Down Expand Up @@ -905,7 +858,8 @@ private function removeImportantAnnotationFromAllInlineStyles(): void
*/
private function removeImportantAnnotationFromNodeInlineStyle(\DOMElement $node): void
{
$inlineStyleDeclarations = $this->parseCssDeclarationsBlock($node->getAttribute('style'));
$style = $node->getAttribute('style');
$inlineStyleDeclarations = (new DeclarationBlockParser())->parse((bool) $style ? $style : '');
/** @var array<string, string> $regularStyleDeclarations */
$regularStyleDeclarations = [];
/** @var array<string, string> $importantStyleDeclarations */
Expand Down
51 changes: 3 additions & 48 deletions src/HtmlProcessor/CssToAttributeConverter.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Pelago\Emogrifier\HtmlProcessor;

use Pelago\Emogrifier\Utilities\DeclarationBlockParser;
use Pelago\Emogrifier\Utilities\Preg;

/**
Expand Down Expand Up @@ -44,21 +45,17 @@ class CssToAttributeConverter extends AbstractHtmlProcessor
],
];

/**
* @var array<string, array<string, string>>
*/
private static $parsedCssCache = [];

/**
* Maps the CSS from the style nodes to visual HTML attributes.
*
* @return $this
*/
public function convertCssToVisualAttributes(): self
{
$declarationBlockParser = new DeclarationBlockParser();
/** @var \DOMElement $node */
foreach ($this->getAllNodesWithStyleAttribute() as $node) {
$inlineStyleDeclarations = $this->parseCssDeclarationsBlock($node->getAttribute('style'));
$inlineStyleDeclarations = $declarationBlockParser->parse($node->getAttribute('style'));
$this->mapCssToHtmlAttributes($inlineStyleDeclarations, $node);
}

Expand All @@ -75,48 +72,6 @@ private function getAllNodesWithStyleAttribute(): \DOMNodeList
return $this->getXPath()->query('//*[@style]');
}

/**
* Parses a CSS declaration block into property name/value pairs.
*
* Example:
*
* The declaration block
*
* "color: #000; font-weight: bold;"
*
* will be parsed into the following array:
*
* "color" => "#000"
* "font-weight" => "bold"
*
* @param string $cssDeclarationsBlock the CSS declarations block without the curly braces, may be empty
*
* @return array<string, string>
* the CSS declarations with the property names as array keys and the property values as array values
*/
private function parseCssDeclarationsBlock(string $cssDeclarationsBlock): array
{
if (isset(self::$parsedCssCache[$cssDeclarationsBlock])) {
return self::$parsedCssCache[$cssDeclarationsBlock];
}

$properties = [];
foreach (\preg_split('/;(?!base64|charset)/', $cssDeclarationsBlock) as $declaration) {
/** @var array<int, string> $matches */
$matches = [];
if (!\preg_match('/^([A-Za-z\\-]+)\\s*:\\s*(.+)$/s', \trim($declaration), $matches)) {
continue;
}

$propertyName = \strtolower($matches[1]);
$propertyValue = $matches[2];
$properties[$propertyName] = $propertyValue;
}
self::$parsedCssCache[$cssDeclarationsBlock] = $properties;

return $properties;
}

/**
* Applies $styles to $node.
*
Expand Down
68 changes: 68 additions & 0 deletions src/Utilities/DeclarationBlockParser.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?php

declare(strict_types=1);

namespace Pelago\Emogrifier\Utilities;

/**
* Provides a common method for parsing CSS declaration blocks.
* These might be from actual CSS, or from the `style` attribute of an HTML DOM element.
*
* Caches results globally.
*
* @internal
*/
class DeclarationBlockParser
{
/**
* @var array<string, array<string, string>>
*/
private static $cache = [];

/**
* Parses a CSS declaration block into property name/value pairs.
*
* Example:
*
* The declaration block
*
* "color: #000; font-weight: bold;"
*
* will be parsed into the following array:
*
* "color" => "#000"
* "font-weight" => "bold"
*
* @param string $declarationBlock the CSS declarations block without the curly braces, may be empty
*
* @return array<string, string>
* the CSS declarations with the property names as array keys and the property values as array values
*/
public function parse(string $declarationBlock): array
{
if (isset(self::$cache[$declarationBlock])) {
return self::$cache[$declarationBlock];
}

$declarations = \preg_split('/;(?!base64|charset)/', $declarationBlock);
if ($declarations === false) {
\trigger_error('Regex failure parsing declaration block `' . $declarationBlock . '`', E_USER_ERROR);
return [];
}

$properties = [];
foreach ($declarations as $declaration) {
$matches = [];
if (!(bool) \preg_match('/^([A-Za-z\\-]+)\\s*:\\s*(.+)$/s', \trim($declaration), $matches)) {
continue;
}

$propertyName = \strtolower($matches[1]);
$propertyValue = $matches[2];
$properties[$propertyName] = $propertyValue;
}
self::$cache[$declarationBlock] = $properties;

return $properties;
}
}
Loading

0 comments on commit ea7d2cd

Please sign in to comment.