PHP-CSS-Parser icon indicating copy to clipboard operation
PHP-CSS-Parser copied to clipboard

Fix/more invalid selector fixes

Open raxbg opened this issue 1 year ago • 4 comments

Pseudo classes cannot have a space after the : sign. We should consider such occurrences invalid.

raxbg avatar Feb 27 '23 17:02 raxbg

Hi @raxbg,

Thanks for the suggestion.

However, moving forwards, we are looking to isolate core functionality (parsing) from additional CSS debugging features (#463).

I'd like to keep this open, so it's captured, but for now I think it's blocked by #461/#462/#463.

The invalid selectors shouldn't be included in the parsed tree I think. Take a look at this fiddle comparing how the browser parses the invalid cases: https://jsfiddle.net/sm67z4nb/

They do not seem to be included in the final tree.

I like the idea of the debugging tools btw :)

raxbg avatar Jul 10 '24 08:07 raxbg

The invalid selectors shouldn't be included in the parsed tree I think. Take a look at this fiddle comparing how the browser parses the invalid cases: https://jsfiddle.net/sm67z4nb/

You're right that if there's something in any of a possibly comma-separated list of selectors the browser doesn't recognize, the entire rule is dropped.

For example, before ::placeholder was standardized, I had to do this:

.example input::-webkit-input-placeholder {
  /* lots or property declarations */
}
.example input::-ms-input-placeholder {
  /* the same property declarations repeated again */
}
.example input:-moz--placeholder {
  /* the same property declarations repeated again */
}
.example input::-moz--placeholder {
  /* the same property declarations repeated again */
}

It was not possible to put the selectors in a comma-separated list, because each browser would barf upon encountering a rival's vendor prefix.

Obviously the parser shouldn't be so prejudiced.

But if there is a selector consutruct which is invalid for all browsers, it will never work, and perhaps should be dropped. Either way, such situation would be an opportune use for the logger (TBI).

@sabberworm, @oliverklee, WDYT - should we drop rules with selector constructs that appear to be invalid?

We would need to be careful not to drop actually-valid constructs.

JakeQZ avatar Jul 10 '24 22:07 JakeQZ

Yes, let's drop them. To quote from https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_syntax/Error_handling:

When a CSS error is encountered, the browser's parser ignores the line containing the errors, discarding the minimum amount of CSS code before returning to parsing the CSS as normal. The "error recovery" is just the ignoring or skipping of invalid content.

oliverklee avatar Jul 11 '24 08:07 oliverklee

It was not possible to put the selectors in a comma-separated list, because each browser would barf upon encountering a rival's vendor prefix.

Take care not to confuse syntactically-invalid rules with rules that are syntactically valid but a specific browser doesn’t know the semantics of. div: hover is an example of the former (and it would be OK to warn and drop that) where input::-webkit-input-placeholder would be the latter for non-webkit browsers (and we should never drop such a rule).

However, to complicate matters, we also have some syntactically-invalid stuff that we do parse because they were wildly-deployed workarounds of browser bugs.

IMHO, we should validate selectors only once we actually parse them.

sabberworm avatar Jul 11 '24 13:07 sabberworm