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

Property merging breaks for particular property orderings that involve vendor prefixes #636

Closed
rjgotten opened this issue Aug 6, 2015 · 3 comments

Comments

@rjgotten
Copy link

rjgotten commented Aug 6, 2015

Using a straight up cleancss .test.css using clean-css v 3.3.7 from the commandline, there is some very weird behavior going on regarding the merging of properties involving vendor prefixes.

/* PREFIX-GROUPED INPUT */
.test {
  -ms-text-overflow: ellipsis;
  -ms-text-overflow: " …";
      text-overflow: ellipsis;
      text-overflow: " …";
}

/* PROPER OUTPUT  (reformatted for legibility) */
.test {
  -ms-text-overflow: ellipsis;
  -ms-text-overflow: " …";
      text-overflow: ellipsis;
      text-overflow: " …";
}
/* VALUE-GROUPED INPUT */
.test {
  -ms-text-overflow: ellipsis;
      text-overflow: ellipsis;
  -ms-text-overflow: " …";
      text-overflow: " …";
}

/* IMPROPER OUTPUT  (reformatted for legibility) */
.test {
  -ms-text-overflow: " …";
      text-overflow: " …";
}

As you can see, in the second case the first value ellipsis is completely swallowed. The order of input in the second case is likely to occur when processing CSS that was generated by CSS pre-processors like Less or Sass, when an author has used mixin functions to generate vendor-prefix rules, e.g.

/* MIXIN */
.text-overflow() {
  -ms-text-overflow: @arguments;
      text-overflow: @arguments;
}

/* CALL-SITE */
.example {
  .text-overflow(ellipsis);
  .text-overflow(" …");
}

Admittedly, my repro-case is a case in which the vendor prefix is actually not strictly necessary: IE reads plain old text-overflow as well, meaning I can (and will) remove the prefixed property from the affected stylesheets altogether. However, encountering this problem here means clean-css is likely to have the same type of issues with property-merging on other sets of properties that do really require vendor prefixes, now or in the future.

@rjgotten
Copy link
Author

rjgotten commented Aug 6, 2015

Adding --skip-aggressive-merging circumvents the issue, btw.
So whatever logic is broken, is probably tied to that functionality.

@jakubpawlowicz
Copy link
Collaborator

Yeah, #290 is pending with fix to this. So far it's all order-dependent so please use the switch for now.

@jakubpawlowicz
Copy link
Collaborator

I'll close this as a duplicate for now. #290 will be the sole point addressed in the version after 3.4.0.

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

No branches or pull requests

2 participants