premailer / css_parser

Ruby CSS Parser
Other
279 stars 110 forks source link

v2 Change befaviour when working with selector #160

Open stoivo opened 3 months ago

stoivo commented 3 months ago

I am looking at fixing a bug when paring css with selector div.not(.a, .b). The Problem is that in RuleSet we parse the selector. https://github.com/stoivo/css_parser/blob/935308c5955b57a2d826d99ae7291902deccdcd5/lib/css_parser/rule_set.rb#L453-L462

We do two different operations here. First think is to split by , which I am confident we do because a ruleset might have a selector like .one,.two which means .one or .two and when we want to calculate specificity we can do it on an or. Also we want it split so if you search for selector .one you will find it. The other operation is to replace subsequent spaces with one space. I think we do that so if someone has a stylesheet with .one .two(many spaces) we collapse it so you can search for it by .one .two (one space)

So to my problems with this and proposed solution

I think we should stop to manipulate the selectors. If someone has a style sheet with the selector .a > .b (double spaces). We shrink it into .a > .b, but the most compact way to write this selector is .a>.b, esbuild demo. We have those who have rules on multiple lines, thy would need to match the selector exactly. I think this is fine because I don't understand why people would use this to look for a specific rule.

Another issue with this stripping is that if someone has a selector like input[name="Joe Doe"] esbuild example we would remove the spaces from the selector which changes the rule which I don't think is expected.

I would like to stop doing the removing of white spaces. If users want to have there selector minifies, minify the css before passing it to css_parser. That's my suggestion. And for the splitting on ,, it's not that hard when we have the crass parser to split on comma on top level tokens.

stoivo commented 3 months ago

I created PR https://github.com/premailer/css_parser/pull/161. What do you think?

grosser commented 3 months ago

yeah just stripping everything sounds like a bad idea 👍 ... sounds like worst case we emit a few extra whitespaces 🤷

stoivo commented 3 months ago

Exactly. I can only think of rule like .a,.b{color:red;} where we would add two spaces, after , and after b. I think think thats bad. Also if you want to minify use a minifier like esbuild or terser