From e7a4387854c3ca120e1b6fb2e4d0f0e656a15ebc Mon Sep 17 00:00:00 2001 From: Jake Hotson Date: Tue, 9 Dec 2025 14:23:10 +0000 Subject: [PATCH 1/2] [BUGFIX] Allow commas in attributes in `setSelectors` This fixes an issue noted in #1324. --- CHANGELOG.md | 2 + src/RuleSet/DeclarationBlock.php | 12 ++++- tests/Unit/RuleSet/DeclarationBlockTest.php | 55 +++++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5882abc..74bdef76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ Please also have a look at our ### Fixed +- Support attribute selectors with values containing commas in + `DeclarationBlock::setSelectors()` (#1419) - Allow `removeDeclarationBlockBySelector()` to be order-insensitve (#1406) - Fix parsing of `calc` expressions when a newline immediately precedes or follows a `+` or `-` operator (#1399) diff --git a/src/RuleSet/DeclarationBlock.php b/src/RuleSet/DeclarationBlock.php index 65a4862f..7eb0fa97 100644 --- a/src/RuleSet/DeclarationBlock.php +++ b/src/RuleSet/DeclarationBlock.php @@ -20,6 +20,7 @@ use Sabberworm\CSS\Property\KeyframeSelector; use Sabberworm\CSS\Property\Selector; use Sabberworm\CSS\Rule\Rule; +use Sabberworm\CSS\Settings; /** * This class represents a `RuleSet` constrained by a `Selector`. @@ -98,7 +99,14 @@ public function setSelectors($selectors, ?CSSList $list = null): void if (\is_array($selectors)) { $selectorsToSet = $selectors; } else { - $selectorsToSet = \explode(',', $selectors); + // A string of comma-separated selectors requires parsing. + // Parse as if it's the opening part of a rule. + $parserState = new ParserState($selectors . '{', Settings::create()); + $selectorsToSet = self::parseSelectors($parserState); + $parserState->consume('{'); // throw exception if this is not next + if (!$parserState->isEnd()) { + throw new UnexpectedTokenException('EOF', $parserState->peek(5)); + } } // Convert all items to a `Selector` if not already @@ -261,7 +269,7 @@ public function render(OutputFormat $outputFormat): string * * @throws UnexpectedTokenException */ - private static function parseSelectors(ParserState $parserState, array &$comments): array + private static function parseSelectors(ParserState $parserState, array &$comments = []): array { $selectors = []; $selectorParts = []; diff --git a/tests/Unit/RuleSet/DeclarationBlockTest.php b/tests/Unit/RuleSet/DeclarationBlockTest.php index 5f226846..445fa4bb 100644 --- a/tests/Unit/RuleSet/DeclarationBlockTest.php +++ b/tests/Unit/RuleSet/DeclarationBlockTest.php @@ -8,6 +8,7 @@ use Sabberworm\CSS\CSSElement; use Sabberworm\CSS\CSSList\CSSListItem; use Sabberworm\CSS\Parsing\ParserState; +use Sabberworm\CSS\Parsing\UnexpectedTokenException; use Sabberworm\CSS\Position\Positionable; use Sabberworm\CSS\Property\Selector; use Sabberworm\CSS\Rule\Rule; @@ -293,4 +294,58 @@ public function setSelectorsIgnoresKeys(): void self::assertSame([0, 1], \array_keys($result)); } + + /** + * @test + * + * @param non-empty-string $selector + * + * @dataProvider provideSelector + */ + public function setSelectorsSetsSingleSelectorProvidedAsString(string $selector): void + { + $subject = new DeclarationBlock(); + + $subject->setSelectors($selector); + + $result = $subject->getSelectors(); + self::assertSame([$selector], self::getSelectorsAsStrings($subject)); + } + + /** + * @test + * + * @param non-empty-string $firstSelector + * @param non-empty-string $secondSelector + * + * @dataProvider provideTwoSelectors + */ + public function setSelectorsSetsTwoCommaSeparatedSelectorsProvidedAsString( + string $firstSelector, + string $secondSelector + ): void { + $joinedSelectors = $firstSelector . ', ' . $secondSelector; + $subject = new DeclarationBlock(); + + $subject->setSelectors($joinedSelectors); + + $result = $subject->getSelectors(); + self::assertSame([$firstSelector, $secondSelector], self::getSelectorsAsStrings($subject)); + } + + /** + * @test + * + * @param non-empty-string $selector + * + * @dataProvider provideInvalidSelector + */ + public function setSelectorsThrowsExceptionWithInvalidSelector(string $selector): void + { + $this->expectException(UnexpectedTokenException::class); + + $subject = new DeclarationBlock(); + + $subject->setSelectors($selector); + } } From 268f57cc02a468a2ee53c28b0391a57adf4a6a9a Mon Sep 17 00:00:00 2001 From: Jake Hotson Date: Tue, 9 Dec 2025 18:27:46 +0000 Subject: [PATCH 2/2] Test the exception message. Also make it more useful. And add additional test data to fully exercise the code. --- src/RuleSet/DeclarationBlock.php | 21 ++++++++++++++++----- tests/Unit/RuleSet/DeclarationBlockTest.php | 15 +++++++++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/RuleSet/DeclarationBlock.php b/src/RuleSet/DeclarationBlock.php index 7eb0fa97..81590d51 100644 --- a/src/RuleSet/DeclarationBlock.php +++ b/src/RuleSet/DeclarationBlock.php @@ -101,11 +101,22 @@ public function setSelectors($selectors, ?CSSList $list = null): void } else { // A string of comma-separated selectors requires parsing. // Parse as if it's the opening part of a rule. - $parserState = new ParserState($selectors . '{', Settings::create()); - $selectorsToSet = self::parseSelectors($parserState); - $parserState->consume('{'); // throw exception if this is not next - if (!$parserState->isEnd()) { - throw new UnexpectedTokenException('EOF', $parserState->peek(5)); + try { + $parserState = new ParserState($selectors . '{', Settings::create()); + $selectorsToSet = self::parseSelectors($parserState); + $parserState->consume('{'); // throw exception if this is not next + if (!$parserState->isEnd()) { + throw new UnexpectedTokenException('EOF', 'more'); + } + } catch (UnexpectedTokenException $exception) { + // The exception message from parsing may refer to the faux `{` block start token, + // which would be confusing. + // Rethrow with a more useful message, that also includes the selector(s) string that was passed. + throw new UnexpectedTokenException( + 'Selector(s) string is not valid.', + $selectors, + 'custom' + ); } } diff --git a/tests/Unit/RuleSet/DeclarationBlockTest.php b/tests/Unit/RuleSet/DeclarationBlockTest.php index 445fa4bb..685112d5 100644 --- a/tests/Unit/RuleSet/DeclarationBlockTest.php +++ b/tests/Unit/RuleSet/DeclarationBlockTest.php @@ -333,16 +333,31 @@ public function setSelectorsSetsTwoCommaSeparatedSelectorsProvidedAsString( self::assertSame([$firstSelector, $secondSelector], self::getSelectorsAsStrings($subject)); } + /** + * Provides selectors that would be parsed without error in the context of full CSS, but are nonetheless invalid. + * + * @return array + */ + public static function provideInvalidStandaloneSelector(): array + { + return [ + 'rogue `{`' => ['a { b'], + 'rogue `}`' => ['a } b'], + ]; + } + /** * @test * * @param non-empty-string $selector * * @dataProvider provideInvalidSelector + * @dataProvider provideInvalidStandaloneSelector */ public function setSelectorsThrowsExceptionWithInvalidSelector(string $selector): void { $this->expectException(UnexpectedTokenException::class); + $this->expectExceptionMessageMatches('/^Selector\\(s\\) string is not valid. /'); $subject = new DeclarationBlock();