From fa139f65c5b098ae652c970b25e6eb03fc495eb4 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 25 Jul 2018 10:38:59 -0700 Subject: [PATCH 1/5] Fix parsing CSS selectors which contain commas --- .../CSS/RuleSet/DeclarationBlock.php | 60 ++++++++++++++++++- tests/Sabberworm/CSS/ParserTest.php | 6 ++ tests/files/specificity.css | 4 +- 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/lib/Sabberworm/CSS/RuleSet/DeclarationBlock.php b/lib/Sabberworm/CSS/RuleSet/DeclarationBlock.php index e18f5d82..26c2e128 100644 --- a/lib/Sabberworm/CSS/RuleSet/DeclarationBlock.php +++ b/lib/Sabberworm/CSS/RuleSet/DeclarationBlock.php @@ -28,7 +28,19 @@ public function setSelectors($mSelector) { if (is_array($mSelector)) { $this->aSelectors = $mSelector; } else { - $this->aSelectors = explode(',', $mSelector); + list( $sSelectors, $aPlaceholders ) = $this->addSelectorExpressionPlaceholders( $mSelector ); + if ( empty( $aPlaceholders ) ) { + $this->aSelectors = explode(',', $sSelectors); + } else { + $aSearches = array_keys( $aPlaceholders ); + $aReplaces = array_values( $aPlaceholders ); + $this->aSelectors = array_map( + function( $sSelector ) use ( $aSearches, $aReplaces ) { + return str_replace( $aSearches, $aReplaces, $sSelector ); + }, + explode(',', $sSelectors) + ); + } } foreach ($this->aSelectors as $iKey => $mSelector) { if (!($mSelector instanceof Selector)) { @@ -37,6 +49,52 @@ public function setSelectors($mSelector) { } } + /** + * Add placeholders for parenthetical/bracketed expressions in selectors which may contain commas that break exploding. + * + * This prevents a single selector like `.widget:not(.foo, .bar)` from erroneously getting parsed in setSelectors as + * two selectors `.widget:not(.foo` and `.bar)`. + * + * @param string $sSelectors Selectors. + * @return array First array value is the selectors with placeholders, and second value is the array of placeholders mapped to the original expressions. + */ + private function addSelectorExpressionPlaceholders( $sSelectors ) { + $iOffset = 0; + $aPlaceholders = array(); + + while ( preg_match( '/\(|\[/', $sSelectors, $aMatches, PREG_OFFSET_CAPTURE, $iOffset ) ) { + $sMatchString = $aMatches[0][0]; + $iMatchOffset = $aMatches[0][1]; + $iStyleLength = strlen( $sSelectors ); + $iOpenParens = 1; + $iStartOffset = $iMatchOffset + strlen( $sMatchString ); + $iFinalOffset = $iStartOffset; + for ( ; $iFinalOffset < $iStyleLength; $iFinalOffset++ ) { + if ( '(' === $sSelectors[ $iFinalOffset ] || '[' === $sSelectors[ $iFinalOffset ] ) { + $iOpenParens++; + } elseif ( ')' === $sSelectors[ $iFinalOffset ] || ']' === $sSelectors[ $iFinalOffset ] ) { + $iOpenParens--; + } + + // Found the end of the expression, so replace it with a placeholder. + if ( 0 === $iOpenParens ) { + $sMatchedExpr = substr( $sSelectors, $iMatchOffset, $iFinalOffset - $iMatchOffset + 1 ); + $sPlaceholder = sprintf( '{placeholder:%d}', count( $aPlaceholders ) + 1 ); + $aPlaceholders[ $sPlaceholder ] = $sMatchedExpr; + + // Update the CSS to replace the matched calc() with the placeholder function. + $sSelectors = substr( $sSelectors, 0, $iMatchOffset ) . $sPlaceholder . substr( $sSelectors, $iFinalOffset + 1 ); + // Update offset based on difference of length of placeholder vs original matched calc(). + $iFinalOffset += strlen( $sPlaceholder ) - strlen( $sMatchedExpr ); + break; + } + } + // Start matching at the next byte after the match. + $iOffset = $iFinalOffset + 1; + } + return array( $sSelectors, $aPlaceholders ); + } + // remove one of the selector of the block public function removeSelector($mSelector) { if($mSelector instanceof Selector) { diff --git a/tests/Sabberworm/CSS/ParserTest.php b/tests/Sabberworm/CSS/ParserTest.php index 43c22e22..a5073be7 100644 --- a/tests/Sabberworm/CSS/ParserTest.php +++ b/tests/Sabberworm/CSS/ParserTest.php @@ -148,6 +148,12 @@ function testSpecificity() { case "li.green": $this->assertSame(11, $oSelector->getSpecificity()); break; + case "div:not(.foo[title=\"a,b\"], .bar)": + $this->assertSame(31, $oSelector->getSpecificity()); + break; + case "div[title=\"a,b\"]": + $this->assertSame(11, $oSelector->getSpecificity()); + break; default: $this->fail("specificity: untested selector " . $oSelector->getSelector()); } diff --git a/tests/files/specificity.css b/tests/files/specificity.css index 82a2939a..df03ff0f 100644 --- a/tests/files/specificity.css +++ b/tests/files/specificity.css @@ -2,6 +2,8 @@ #file, .help:hover, li.green, -ol li::before { +ol li::before, +div:not(.foo[title="a,b"], .bar), +div[title="a,b"] { font-family: Helvetica; } From 10a2501c119abafced3e4014aa3c0a3453a86f67 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 20 Apr 2020 13:53:44 -0700 Subject: [PATCH 2/5] Update SELECTOR_VALIDATION_RX to account for parenthetical groups --- lib/Sabberworm/CSS/Property/Selector.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Sabberworm/CSS/Property/Selector.php b/lib/Sabberworm/CSS/Property/Selector.php index bd04b889..0f1c6432 100644 --- a/lib/Sabberworm/CSS/Property/Selector.php +++ b/lib/Sabberworm/CSS/Property/Selector.php @@ -43,6 +43,7 @@ class Selector { [a-zA-Z0-9\x{00A0}-\x{FFFF}_^$|*="\'~\[\]()\-\s\.:#+>]* # any sequence of valid unescaped characters (?:\\\\.)? # a single escaped character (?:([\'"]).*?(? Date: Mon, 20 Dec 2021 17:01:51 -0800 Subject: [PATCH 3/5] Fix specificity test by adding the additional selectors --- tests/ParserTest.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/ParserTest.php b/tests/ParserTest.php index 82d6a380..681d2597 100644 --- a/tests/ParserTest.php +++ b/tests/ParserTest.php @@ -273,13 +273,20 @@ public function specificity() new Selector('.help:hover', true), new Selector('li.green', true), new Selector('ol li::before', true), + new Selector('div:not(.foo[title="a,b"], .bar)', true), + new Selector('div[title="a,b"]', true), ], $oDoc->getSelectorsBySpecificity('<= 100')); self::assertEquals([ new Selector('.help:hover', true), new Selector('li.green', true), new Selector('ol li::before', true), + new Selector('div:not(.foo[title="a,b"], .bar)', true), + new Selector('div[title="a,b"]', true), ], $oDoc->getSelectorsBySpecificity('< 100')); - self::assertEquals([new Selector('li.green', true)], $oDoc->getSelectorsBySpecificity('11')); + self::assertEquals([ + new Selector('li.green', true), + new Selector('div[title="a,b"]', true), + ], $oDoc->getSelectorsBySpecificity('11')); self::assertEquals([new Selector('ol li::before', true)], $oDoc->getSelectorsBySpecificity(3)); } From d553951476d6e5cc37bc18e7501aa4d9262a445b Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 20 Dec 2021 17:02:19 -0800 Subject: [PATCH 4/5] Use static closure --- src/RuleSet/DeclarationBlock.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/RuleSet/DeclarationBlock.php b/src/RuleSet/DeclarationBlock.php index d6c6b1ca..bc2ef4ce 100644 --- a/src/RuleSet/DeclarationBlock.php +++ b/src/RuleSet/DeclarationBlock.php @@ -102,7 +102,7 @@ public function setSelectors($mSelector, $oList = null) $aSearches = array_keys($aPlaceholders); $aReplaces = array_values($aPlaceholders); $this->aSelectors = array_map( - function ($sSelector) use ($aSearches, $aReplaces) { + static function ($sSelector) use ($aSearches, $aReplaces) { return str_replace($aSearches, $aReplaces, $sSelector); }, explode(',', $sSelectors) From 935ea79e0389e672a9deadf9f366e6faab9110ba Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 20 Dec 2021 17:11:55 -0800 Subject: [PATCH 5/5] Fix phpcs warnings by reducing line length --- src/Property/Selector.php | 2 +- src/RuleSet/DeclarationBlock.php | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Property/Selector.php b/src/Property/Selector.php index a1a493d0..25d1ac61 100644 --- a/src/Property/Selector.php +++ b/src/Property/Selector.php @@ -56,7 +56,7 @@ class Selector [a-zA-Z0-9\x{00A0}-\x{FFFF}_^$|*="\'~\[\]()\-\s\.:#+>]* # any sequence of valid unescaped characters (?:\\\\.)? # a single escaped character (?:([\'"]).*?(?