Skip to content

Fix parsing CSS selectors which contain commas #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: v8.x
Choose a base branch
from
1 change: 1 addition & 0 deletions src/Property/Selector.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class Selector
[a-zA-Z0-9\x{00A0}-\x{FFFF}_^$|*="\'~\[\]()\-\s\.:#+>]* # any sequence of valid unescaped characters
(?:\\\\.)? # a single escaped character
(?:([\'"]).*?(?<!\\\\)\2)? # a quoted text like [id="example"]
(?:\(.*?\))? # an argument for pseudo selector like :not(a,b)
)*
)$
/ux';
Expand Down
64 changes: 63 additions & 1 deletion src/RuleSet/DeclarationBlock.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,19 @@ public function setSelectors($mSelector, $oList = null)
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(
static function ($sSelector) use ($aSearches, $aReplaces) {
return str_replace($aSearches, $aReplaces, $sSelector);
},
explode(',', $sSelectors)
);
}
}
foreach ($this->aSelectors as $iKey => $mSelector) {
if (!($mSelector instanceof Selector)) {
Expand All @@ -125,6 +137,56 @@ public function setSelectors($mSelector, $oList = null)
}
}

/**
* Add placeholders for parenthetical 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 = [];

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 [ $sSelectors, $aPlaceholders ];
}

/**
* Remove one of the selectors of the block.
*
Expand Down
15 changes: 14 additions & 1 deletion tests/ParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,12 @@ public function specificity()
case "li.green":
self::assertSame(11, $oSelector->getSpecificity());
break;
case "div:not(.foo[title=\"a,b\"], .bar)":
self::assertSame(31, $oSelector->getSpecificity());
break;
case "div[title=\"a,b\"]":
self::assertSame(11, $oSelector->getSpecificity());
break;
default:
self::fail("specificity: untested selector " . $oSelector->getSelector());
}
Expand All @@ -272,13 +278,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'));
}

Expand Down
4 changes: 3 additions & 1 deletion tests/fixtures/specificity.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}