begriffs / css-ratiocinator

because your CSS is garbage
MIT License
1.03k stars 49 forks source link

All non-webkit prefixed properties and values are discarded. [$35] #52

Open knpwrs opened 10 years ago

knpwrs commented 10 years ago

Example: http://www.csstrashman.com/styles/38305

All values which webkit considers invalid (anything prefixed with another vendor's prefix) is discarded.

The following two rules of are examples of this:

display: -ms-flexbox;
-ms-flex: 1 1 auto

A possible solution (albeit messy and not that great in general) would be to run the resulting CSS through a prefixer.

There is a $35 open bounty on this issue. Add to the bounty at Bountysource.

begriffs commented 10 years ago

Thanks for the bug report.

Would it suffice for ratiocinator to add -moz-X, -o-X, and -ms-X every time it outputs -webkit-X? Maybe enable this feature with an optional command line argument?

knpwrs commented 10 years ago

No, because sometimes different browsers have different property names and webkit sometimes has multiple prefixes to support older webkits. Consider the following base flexbox code:

  display: -webkit-box;
  display: -moz-box;
  display: -ms-flexbox;
  display: -webkit-flex;
  display: flex;

-webkit- appears twice for box and flex, -ms- once for flexbox, and -moz- once for box.

knpwrs commented 10 years ago

There is also a typo in one CSS property in webkit where the prefix isn't actually the prefix... I can't seem to recall which property that is...

begriffs commented 10 years ago

I wonder if the prefix complexities are best solved by putting the output through an existing prefixer like you mentioned. If so which one do you recommend?

Would you send a pull request to update the readme to mention the problem and show how to use the prefixer?

knpwrs commented 10 years ago

Another issue related to this one is that invalid properties are excluded. So if something requires a prefix for Phantom to recognize it, and somebody passes in something without a prefix (e.g., intending to use something like Prefix Free, then that property will be stripped.

I'll submit a pull request for the README. As for actual prefixing functionality, we need to plan more. I'm not sure if we'll be able to adequately satisfy all edge cases so we may need to just document any edge cases that come up.

knpwrs commented 10 years ago

Pull request #53 submitted.

Another issue we may need to consider is that all comments are being stripped. On my site I am using third party CSS and have preserved copyright comments. Maybe we should prepend all copyright comments to the final output? Ratiocinator tends to move things around so comment positioning doesn't mean as much.

begriffs commented 10 years ago

What's an example for the invalid property bug? Can you create a failing test for that one?

knpwrs commented 10 years ago

An invalid property would be anything prefixed by -ms-, -o-, or -moz- or an old -webkit-. For example:

div {
   -moz-box-sizing: border-box;
}

Mozilla still requires a prefix on box-sizing according to this.

begriffs commented 10 years ago

So I ran http://www.csstrashman.com/styles/38305.css through prefixr.com and ended up with https://gist.github.com/begriffs/8391042 . Did this correct the problem?

knpwrs commented 10 years ago

Not really. See screenshot:

screen shot 2014-01-12 at 7 42 34 pm

Even worse in Firefox (see icons at the bottom):

screen shot 2014-01-12 at 7 43 40 pm

begriffs commented 10 years ago

I hope someone can figure this out. I added a bounty for this solution of this issue.