Skip to content

Fix parsing CSS selectors which contain commas #138

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 59 additions & 1 deletion lib/Sabberworm/CSS/RuleSet/DeclarationBlock.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions tests/Sabberworm/CSS/ParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
4 changes: 3 additions & 1 deletion tests/files/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;
}