katiefenn / parker

Stylesheet analysis tool.
Other
2.47k stars 73 forks source link

IDs falsely reported in attribute selectors #28

Closed csswizardry closed 9 years ago

csswizardry commented 10 years ago

These two selectors have the same specificity (jsFiddle):

[href="#main"] {
    color: red;
}

.link {
    color: green;
}

However, I think Parker is reporting [href="#main"] as being an ID. When running Parker over a stylesheet containing only the above CSS (abridged for brevity):

PARKER-JS
Total Stylesheets: 1
Total Stylesheet Size: 64
Total Rules: 2
Total Selectors: 2
Total Identifiers: 3
Total Declarations: 2
Selectors Per Rule: 1
Identifiers Per Selector: 1.5
Specificity Per Selector: 55.5
Top Selector Specificity: 101
Top Selector Specificity Selector: [href="#main"]
Total Id Selectors: 1
...
robinpokorny commented 10 years ago

The same problem would be if there is quoted ., [, : or :: inside the selector.

It might be a good idea to use mdevils/node-css-selector-parser or similar to parse the selector instead of a regex.

katiefenn commented 10 years ago

Thanks for raising this. You're right, the regexes Parker uses are insufficient. I think this could be fixed by writing a metric that splits selectors into component identifiers and determines their type individually. False positives such as those mentioned should then be easier to avoid.

@robinpokorny: Thanks for the link. I decided to parse CSS myself wherever possible, because I plan on parsing and measuring CSS hacks at some point. Many of the parsers I looked into early on (correctly) discard hacks. The risk is spending a lot of time to get it right, so I'll try as hard as I can to refine the metrics first ;-)

decadecity commented 9 years ago

There's a similar issue with the pseudo element specificity detection/calculation.

::before is matching :b in pseudo class as well as :: in pseudo element so is being assigned a specificity of 11 rather than 1.

callumacrae commented 9 years ago

@csswizardry I guess you didn't get a notification for this, but I just submitted a fix for this (among with a fix for my own bug) in #35.