[BUGFIX] Parse attribute selectors

[JakeQZ]

Don't treat spaces or characters like ~ within them as a stop character representing a combinator.

The bug was introduced by #1496 and related changes that split selector representation and parsing into compound selectors and combinators.

Addresses some of the issues in #1533.

[coveralls]

coverage: 72.384% (+0.3%) from 72.114%
when pulling 6a81ef5 on bugfix/attribute-selector-parsing
into 34ba5d4 on main.

[oliverklee]

This is a step in the right direction. However, it does not fully fix #1533 yet.

Before this fix, there were 57 errors and 36 failures, and with it, there still are 40 errors and 35 failures, but different ones. Now we have exceptions from PHP-CSS-Parser instead of from the Symfony CSS library:
Sabberworm\CSS\Parsing\UnexpectedTokenException : Selector component is not valid: :nth-last-child(2n+3) [line no: 1]

We could either try to fix all of them in one PR, or merge this one and then add more PRs until the Emogrifier is green again. I'd be fine either way.

WDYT?

(And if you would like help getting your local setup to work so you can run the Emogrifier tests with your local copy of PHP-CSS-Parser, I'll be happy to help.)

[oliverklee]

(And we might also want to make some of the Emogrifier tests more lenient concerning whitspace.)

[JakeQZ]

:nth-last-child(2n+3)

I can reproduce this by adding a test for it. I think it's because in CompoundSelector::isValid() I removed + as a potentially valid character. That will need to be added back. (In this PR I added back ~.)

We could either try to fix all of them in one PR, or merge this one and then add more PRs until the Emogrifier is green again. I'd be fine either way.

I think merge this one (which focuses on attributes) first, then apply the fix to allow +. We probably also need to allow > which might occur in something like :not(p > span) if that's currently being rejected.

[JakeQZ]

This is a step in the right direction. However, it does not fully fix #1533 yet.

I think merge this one (which focuses on attributes) first, then apply the fix to allow +.

I've updated the commit message so that it won't close #1533, just refer to it as addressing some issues.

[JakeQZ]

(And we might also want to make some of the Emogrifier tests more lenient concerning whitspace.)

I just tested Emogrifier with the current dev-main of PHP-CSS-Parser. The issues in #1533 and #1534 seem to be fixed, but the change to selector representation and rendering means that tests are failing when the expected selector combinators are rendered back out with surrounding spaces (the default OutputFormat setting).

I wouldn't like to change the tests to try to cater for one or the other - that's messy. Ideally the tests would perhaps use OutputFormat::setSpaceAroundSelectorCombinator() to request rendering without spaces, but that method won't be available until 9.2 is released. Or they could be changed to expect renderings with spaces, but that expectation will also fail until 9.2 is released.

I don't consider the the changes made to Selector to be breaking, though they do seem to be causing issues for us, regarding the tools.

[JakeQZ]

(And we might also want to make some of the Emogrifier tests more lenient concerning whitspace.)

the change to selector representation and rendering means that tests are failing when the expected selector combinators are rendered back out with surrounding spaces (the default OutputFormat setting).

Resolved with MyIntervals/emogrifier#1574. The only remaining issue with using the dev-main branch in Emogrifier is that Stan reports an unnecessary assert(), since Selector::getSelector() is now declared to return a non-empty-string. I expect this can be easily addressed when the next release of PHP-CSS-Parser is made, though it will initially break the tests. I can't think of a way of allowing both to satisfy Stan.

[oliverklee]

After we‘ve released PHP-CSS-Parser, let‘s directly create a PR for Emogrifier that both bumps the dependency as well as updates the PHPStan baseline. (We then can clean up the code in later PRs.)

[JakeQZ]

After we‘ve released PHP-CSS-Parser, let‘s directly create a PR for Emogrifier that both bumps the dependency as well as updates the PHPStan baseline. (We then can clean up the code in later PRs.)

I was looking to get some renaming done before working on #1445. But we've decided against the original idea I had for that (which may have been a quicker fix of the sticking-plaster variety), and I can't see any obvious low-hanging fruit now. There have also been a number of fixes, changes and deprecations since 9.1, so this is probably a good time to create a 9.2 release.