Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSSO should be smart about merging selectors #301

Closed
aarongustafson opened this issue Jun 9, 2016 · 4 comments
Closed

CSSO should be smart about merging selectors #301

aarongustafson opened this issue Jun 9, 2016 · 4 comments

Comments

@aarongustafson
Copy link

Based on the parsing errors defined in the CSS spec, if a browser doesn’t understand any member of a compound selector, the entire rule set is thrown out. For example:

input[type="file"]::-webkit-file-upload-button {
    border: 3px solid;
}
input[type="file"]::-ms-browse {
    border: 3px solid;
}

If merged:

input[type="file"]::-webkit-file-upload-button,
input[type="file"]::-ms-browse {
    border: 3px solid;
}

The CSS would not apply in Chrome, Safari and Opera (which support the -webkit- variant) or IE and Edge (which support the -ms- variant) because the browsers don’t understand the one that applies to the other. And if merged with another rule, say

button {
    border: 3px solid;
}

to give you

button,
input[type="file"]::-webkit-file-upload-button,
input[type="file"]::-ms-browse {
    border: 3px solid;
}

no browser would get the 3 pixel border because every browser would encounter some part of that compound selector they don’t understand.

This is a BIG deal and definitely needs to be addressed. It’s also worth noting that this applies to other types of selectors too. If that last rule set were merged with a more complex selector, say

a:not(.foo) {
    border: 3px solid;
}

to create

button,
a:not(.foo) {
    border: 3px solid;
}

Any browser that doesn’t understand the :not pseudo-class would ignore the entire rule set.

The first scenario (vendor-prefixed selectors) is relatively easy to handle by never combining them with other selectors. The second is a little more complex and heavily nuanced. It could require consulting autoprefixer (or similar) or could be handled via an ignore configuration option or some sort of "ignore" comment in your CSS immediately before the rule set. Perhaps

/* csso:dont-combine */

or similar (which is similar to what UnCSS supports).

@lahmatiy
Copy link
Member

lahmatiy commented Jun 9, 2016

CSSO already behave as you describe:
proof
Do you have a test case when CSSO wrongly merge selectors?

@lahmatiy
Copy link
Member

lahmatiy commented Jun 9, 2016

Currently only those pseudos marked as safe:

  • link
  • visited
  • hover
  • active
  • first-line
  • first-letter
  • after
  • before

I know after and before are not totally safe, but this is not an issue for most browsers.

Other pseudos are treated as not safe and optimizer merges selectors with the same pseudos only.

@aarongustafson
Copy link
Author

Weird. I wonder if its an issue with the Gulp implementation. I’ll check and circle back.

@aarongustafson
Copy link
Author

I stand corrected, looks like it isn’t combining it after all. I don’t know what was causing the issue, but it seems to have resolved itself. My sincere apologies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants