postcss / autoprefixer

Parse CSS and add vendor prefixes to rules by Can I Use
https://twitter.com/autoprefixer
MIT License
21.57k stars 1.25k forks source link

This module should only add prefixes, not remove code #332

Closed e-oz closed 9 years ago

e-oz commented 9 years ago

Get the code of Paul Irish: http://www.html5rocks.com/en/tutorials/flexbox/quick/

display: -webkit-box;
-webkit-box-orient: horizontal;
-webkit-box-pack: center;
-webkit-box-align: center;

display: -moz-box;
-moz-box-orient: horizontal;
-moz-box-pack: center;
-moz-box-align: center;

display: box;
box-orient: horizontal;
box-pack: center;
box-align: center;

And see what your module will do with it. I wasted 4 hours and tons of nerves to find out what's going on, because I couldn't imagine that 'prefixer' can remove code instead of adding prefixes - it's absolutely wrong.

ai commented 9 years ago

Yeap, Autoprefixer also removes outdated prefixes.

ai commented 9 years ago

Autoprefixer output is

a {
  display: -webkit-box;
  -webkit-box-orient: horizontal;
  -webkit-box-pack: center;
  -webkit-box-align: center;
  -moz-box-orient: horizontal;
  -moz-box-pack: center;
  -moz-box-align: center;

  display: box;
  box-orient: horizontal;
  box-pack: center;
  box-align: center;
}

It didn’t remove all -moz- prefixes because Autoprefixer expects that you write prefixed property right under unprefixed. But, what is wrong with this output?

e-oz commented 9 years ago

No, I'm getting output without display: -webkit-box; - "grunt-autoprefixer": "^0.7.6" Prefixer should not remove anything, if it's "prefixer" and not "cleaner".

e-oz commented 9 years ago

Nice way of "solving" issue, lol :) Good reason to avoid this piece of shit.

ai commented 9 years ago

Why you need -webkit-box? Browsers, that you set to Autoprefixer does not require -webkit-box, so Autoprefixer remove it.

e-oz commented 9 years ago

Because Chrome 39 (beta) requires exactly this value. And it's mentioned in the end of post I gave link to.

ai commented 9 years ago

@jamm nope, this article is old. Check Can I Use flexbox page, Chrome doesn’t requires prefix since 29.

The main idea of Autoprefixer is “don’t think about prefix”. So if you write flexbox CSS, just don’t copy any prefixes from the examples. Autoprefixer use only necessary prefixes and take prefixes info from most trustworthy Can I Use resource.

e-oz commented 9 years ago

@ai I checked it yesterday and I wasted hours so I tried ALL variants just to make this simple code work. Chrome work ONLY with -webkit-box, all other variants (including "flex" and "flexbox") are marked as incorrect. You can easily test it.

ai commented 9 years ago

OK. I found problem. This article had wrong example. There is no box value for display in current Flexbox spec. Current spec uses only flex. Also there is different names for other properties.

ai commented 9 years ago

This is fixed example by latest official Flexbox:

display: flex;
flex-direction: row;
justify-content: center;
align-items: center;​

Here is working example: http://codepen.io/anon/pen/KdJoH

ai commented 9 years ago

This article was very old (October 5th, 2010). In this times there was only one spec. And Chrome uses this old spec in prefixed properties. In 2012 spec was changed and IE used this 2012 spec (with different names) in prefixed values.

Finish spec also has different properties names (flex instead of box). And Chrome and Firefox and IE 11 used this final spec without prefixes.

So, please don’t uses old examples, because web has fast development. And if you use old exampels, please, don’t blame others in your mistakes ;).

e-oz commented 9 years ago

"Updated: November 26th, 2013"

but anyway thanks for investigation. Sorry for rudeness - it takes a lot of nerves when you writing code and browser doesn't see it and you don't understand what the h\ is going on.

e-oz commented 9 years ago

But anyway, while your module removes the code, I will avoid your software and I call it unexpected and insane behavior for "prefixer".

Chrome and Firefox and IE 11 used this final spec without prefixes No, Chrome don't work with 'flex' without prefix. Go try it before posting it.

e-oz commented 9 years ago

If you still too lazy to check, I have a chance to make screenshot: With display: flex; https://www.dropbox.com/s/l0ct3s4ini2nrn6/Screenshot%202014-10-12%2020.59.25.png?dl=0 With display: -webkit-box: https://www.dropbox.com/s/sdh79bm8oyots18/Screenshot%202014-10-12%2020.59.42.png?dl=0 Difference is obvious.

Rowno commented 9 years ago

Chrome works fine with display: flex;. See http://css-tricks.com/snippets/css/a-guide-to-flexbox/ for an up to date guide to Flexbox.

e-oz commented 9 years ago

@Rowno I just gave screenshots from Chrome Beta, so no need to read articles to see real behavior.

Rowno commented 9 years ago

The screenshots don't prove your Flexbox code is correct for the final spec. :wink:

ai commented 9 years ago

@jamm in your screenshot Chrome didn’t cross out display: flex, so it supports flex. Maybe styles didn’t work in your example, because you use property names from old specs.

Come on, do you really think, that Can I Use has wrong data about Chrome Flexbox. There are a lot of Chrome users uses Can I Use and they will fix any mistakes.

ai commented 9 years ago

@jamm I can add options to disable old prefixes removing. But what user case you have?

This example, shows us, that Autoprefixer outdated prefixes removeing helps you to find bug. If you miss this error, your styles will not be working in IE 10/11 and modern versions of Chrome and Firefox ;).

ai commented 9 years ago

I add this option for 3.2 release. Right after I will finish PostCSS 3.0.

Grawl commented 9 years ago

@jamm you can simply disable prefixing of single lines or selector contents with conditional comments https://github.com/postcss/autoprefixer#disabling

ai commented 9 years ago

@Grawl there is not way to disable for a single line ;).

Anyway I think we should have option to disable prefix cleaning.

Grawl commented 9 years ago

@ai I never tried this but have suggested to few retarded developers on Habrahabr. This option will be nice for them.

ai commented 9 years ago

Done. On autoprefixer(remove: false) option Autoprefixer 3.2 will not remove outdated prefixes.

ai commented 9 years ago

remove: false option was released today in Autoprefixer 3.2.

e-oz commented 9 years ago

thank you!