Pixel (px) height getting converted to points (pt)

[kfitzgerald]

Greetings!

I just updated the dependencies for one of my projects and ran into this issue. I've got a pixel height in a pseudo-selector and it's getting converted into points.

Here's a snippet of the affected css prior to minification:

.okanjo-product-sidebar .okanjo-product:first-of-type {
  height: 124px;
  border-top: none;
}

And here's the snippet after:

.okanjo-product-sidebar .okanjo-product:first-of-type{height:93pt;border-top:none}

My expected result is:

.okanjo-product-sidebar .okanjo-product:first-of-type{height:124px;border-top:none}

IMHO, pixels and points are two completely different animals, and should not be interchangeable. This is very much undesirable functionality.

After some digging, my current workaround is to kill the pt unit and set the compatibility config:

{ compatibility: '*,-units.pt' }

But I don't feel like I should have to do this. Seems more like a bug.

For reference, on my other machine where I did not experience this issue, my dependency graph is:

• gulp-minify-css@1.0.0
  • clean-css@3.1.5
    • commander@2.6.0
    • source-map@0.1.43
      • amdefine@0.1.0

And my new machine where this issue is present, the dependency graph looks like this:

• gulp-minify-css@1.2.1
  • clean-css@3.4.1
    • commander@2.8.1
      • graceful-readlink@1.0.1
  • source-map@0.4.4
    • amdefine@1.0.0

Thanks!
-Kevin

[jakubpawlowicz]

Do you experience any layout issues because of this change? According to MDN 4px is always 3pt: see https://developer.mozilla.org/en-US/docs/Web/CSS/length under "CSS units and dots-per-inch".

[westyby]

Agree with kfitzgerald. Seems more like a bug. Maybe, this feature by default should be turned off, but turned on in the options. It is also worth to do with pc

.bug1 {width:16px;}

.bug1 {width:1pc;}

[westyby]

however, it works in IE8, IE9, Windows Phone 8 IE10, Microsoft Edge, and in normal browsers.

[jakubpawlowicz]

It should be compatible with IE7+ and all other engines so unless there are any specific issues I am going to leave it on by default.

On 1 Sep 2015, at 09:10, westyby notifications@github.com wrote:

however, it works in IE8, IE9, Windows Phone 8 IE10, Microsoft Edge, and other normal browsers.

—
Reply to this email directly or view it on GitHub.

[uikid]

How can I disable this?

[rjgotten]

unless there are any specific issues

Like compatibility with JavaScript that reads the current style of a node?

Project-specific JavaScript should be allowed to work off of the very sensible expectation that particular sizes written in the uncompressed CSS files as pixels will remain as pixels and will thus be read as pixels.

Flipping units like this can lead to very, very hard to catch layout bugs...

[uikid]

@rjgotten this is exactly what I am dealing with right now.

It broke everything. This is the most useless optimization that is nowhere documented (or shown how to disable it). Com'on, because of these few chars recalculate units?

Turn that off by default, this conversion is will also be hated by many other developers, I guarantee it! There are websites that rely on a given unit, print versions etc.

[kfitzgerald]

@jakubpawlowicz Thanks for the prompt reply!

While the pixel-physical unit relationship MDN notes from the W3 CSS2 spec is valid, the CSS2 specification (http://www.w3.org/TR/CSS2/syndata.html#length-units) goes into more detail, specifically:

Absolute length units are fixed in relation to each other. They are mainly useful when the output environment is known. The absolute units consist of the physical units (in, cm, mm, pt, pc) and the px unit:

• in: inches — 1in is equal to 2.54cm.
• cm: centimeters
• mm: millimeters
• pt: points — the points used by CSS are equal to 1/72nd of 1in.
• pc: picas — 1pc is equal to 12pt.
• px: pixel units — 1px is equal to 0.75pt.

For a CSS device, these dimensions are either anchored (i) by relating the physical units to their physical measurements, or (ii) by relating the pixel unit to the reference pixel. For print media and similar high-resolution devices, the anchor unit should be one of the standard physical units (inches, centimeters, etc). For lower-resolution devices, and devices with unusual viewing distances, it is recommended instead that the anchor unit be the pixel unit. For such devices it is recommended that the pixel unit refer to the whole number of device pixels that best approximates the reference pixel.

Note that if the anchor unit is the pixel unit, the physical units might not match their physical measurements. Alternatively if the anchor unit is a physical unit, the pixel unit might not map to a whole number of device pixels.

Note that this definition of the pixel unit and the physical units differs from previous versions of CSS. In particular, in previous versions of CSS the pixel unit and the physical units were not related by a fixed ratio: the physical units were always tied to their physical measurements while the pixel unit would vary to most closely match the reference pixel. (This change was made because too much existing content relies on the assumption of 96dpi, and breaking that assumption breaks the content.)

... more stuff about reference pixels, DPI, and viewing angle ...

So to summarize, I don't mind if my units get converted to a more optimal format. However, I do mind if my specified units get converted from physical units to non-physical units (pixel), or vise versa.

Since browsers / screen-media might not be the only avenue that could benefit from clean-css (print-media, PrinceXML etc) I would suggest avoiding blurring the lines between physical units and pixels.

Thanks again!
-Kevin

[westyby]

How can I disable this?

while you can be fixed so

'use strict';
var gulp = require('gulp');
var minifycss = require('gulp-minify-css');

var task = {};

gulp.task('cssmin', function () {
    gulp.src('./src.css')
        .pipe(minifycss({
            //https://www.npmjs.com/package/clean-css#how-to-set-compatibility-mode
            compatibility: 'ie7,' +
            '-units.ch,' +
            '-units.in,' +
            '-units.pc,' +
            '-units.pt,' +
            '-units.rem,' +
            '-units.vh,' +
            '-units.vm,' +
            '-units.vmax,' +
            '-units.vmin'
        }))
        .pipe(gulp.dest('./build/gulp'))
});
gulp.task('default', ['cssmin']);

[uikid]

@westyby does this disable/remove all the listed units? If yes, that's no solution, I use multiple units in my css.

I switched my package to get the job done. Good luck with the owner here!

[westyby]

uikid, No, it disables automatic translation units during the processing plugin. All these units you can still use CSS, they will remain
before

.bug1 {
    width: 16px;
    height: 16pc;
}
.bug2 {
    font-size: 12rem;
}

after

.bug1{width:16px;height:16pc}.bug2{font-size:12rem}

[jakubpawlowicz]

@kfitzgerald thanks for digging into the w3c spec. I agree we should not blur the lines and provide a nice set of options to control this behavior - I can think of deriving optimization rules from an output medium (screen, print, etc) which could be the first step of setting up css minification. So far clean-css targets mostly browsers but you can still customize quite for other use cases.

@westyby @uikid you can disable in, pc, and pt units only to achieve the same effect.

Please bear in mind that any optimizations, be it lossy image encoding, JavaScript (to lesser extent) and CSS can break your site if you do something unusual.

Re argument that getting property values from JavaScript would break sites is true to any optimization any CSS optimizer does. However I would be interested in a way of telling people what settings they should use for their particular use case. Any ideas?

[rjgotten]

Re argument that getting property values from JavaScript would break sites is true to any optimization any CSS optimizer does.

Nonsense.

Merging long-hand properties into a short-hand property or removing duplicated properties are examples of optimizations that are safe. Those type of changes will not be reflected in getComputedStyle() and will not leak any further into the runtime environment than the CSSOM itself.

In contrast, changing the unit of a property value is a prime example of an operation that is unsafe because those changes do leak out through getComputedStyle() and can have major effects.

---

You should really have a look at what UglifyJS does: only enable a set of strictly safe transformations by default and make everything else opt-in only.

That way users can reasonally safely upgrade the package to resolve bugs, get better performance or better compression results, without requiring that they tread glass and keep their fingers crossed hoping that no new unsafe transformations were added that would make an existing project fall apart.

This is seriously the sixth time in four months time that I'm seeing this kind of thing happen. In my book that makes it a big, priority one, problem to solve.

[jakubpawlowicz]

Please correct me if I'm wrong but getComputedStyle cares about converting units back to pixels, see http://jsfiddle.net/9rj3sf88/

And it all depends on a context. Are clean-css transformations safe for IE7+ context? Nope, you have an option for that. Can you merge in a longhand margin-top: -moz-calc(...) into a margin shorthand? Nope, unless you know a context - let's say Firefox-only site.

Similarly if you rely on JavaScript Function.prototype.toString() then UglifyJS will break your code by default by rewriting function declarations. But in general context it does safe transformation unless you do something very special in your code.

I am sorry to hear clean-css broke your site. I do my best to keep everything well tested but it's not always possible to cover 100% of test cases.

[pepelsbey]

Please record my vote for making this optimization optional i.e. not enabled by default. Such experiments should be backed up by a set of wide tests in many browsers, not by “there’s an article on MDN stating that it’s safe” reply days after the release.

[rjgotten]

Please correct me if I'm wrong but getComputedStyle cares about converting units back to pixels, see http://jsfiddle.net/9rj3sf88/

As you wish.

DOM2 mentions that:

the getComputedStyle method provides read only access to the computed value of an element.
— http://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-OverrideAndComputed

Where computed values are specified by CSS2 as:

Specified values may be absolute (i.e., they are not specified relative to another value, as in 'red' or '2mm') or relative (i.e., they are specified relative to another value, as in 'auto', '2em', and '12%'). For absolute values, no computation is needed to find the computed value.

Relative values, on the other hand, must be transformed into computed values: percentages must be multiplied by a reference value (each property defines which value that is), values with relative units (em, ex, px) must be made absolute by multiplying with the appropriate font or pixel size, 'auto' values must be computed by the formulas given with each property, certain keywords ('smaller', 'bolder', 'inherit') must be replaced according to their definitions.
— http://www.w3.org/TR/1998/REC-CSS2-19980512/cascade.html#computed-value

The pt unit is, according to 'CSS Values and Units Module Level 3', still defined as an absolute unit.

Browsers that convert absolute units to px via getComputedStyle are doing work that they should not actually be doing. This discrepancy between specifications and browser implementations is clarified by documentation of the getComputedStyle API on MDN:

The values returned by getComputedStyle are known as resolved values. These are usually the same as the CSS 2.1 computed values, but for some older properties like width, height or padding, they are instead the used values. Originally, CSS 2.0 defined the computed values to be the "ready to be used" final values of properties after cascading and inheritance, but CSS 2.1 redefined computed values as pre-layout, and used values as post-layout. For CSS 2.0 properties, the getComputedStyle function returns the old meaning of computed values, now called used values. An example of difference between pre- and post-layout values includes the resolution of percentages that represent the width or the height of an element (also known as its layout), as those will be replaced by their pixel equivalent only in the used value case.
— https://developer.mozilla.org/en-US/docs/Web/API/Window/getComputedStyle

In other words; depending on what property you are reading, absolute units may be converted due to maintaining backwards compatibility or may not be converted when used within newer CSS constructs. Thus you cannot make the case that they will be converted in general, because they won't.

Also, browser vendors are in principal free to, at one point, drop the legacy CSS 2.0 behavior completely and fully adopt the correct CSS 2.1 behavior for all CSS properties.

---

Similarly if you rely on JavaScript Function.prototype.toString() then UglifyJS will break your code by default by rewriting function declarations. But in general context it does safe transformation unless you do something very special in your code.

One of the core optimizations of a minifier is to perform safe name substitution. If you use reflection/inflection flavored constructs such as serializing a function body to a string, then yes: that will break. But anyone using a minifier can reasonably expect that kind of thing to break.

Swapping out units on a CSS value to crunch out ~200 additional bytes is not a core optimization of a CSS minifier. From this thread alone it has already been proven destructive in practice, which means you shouldn't do it _by default_. (Note that? It should still be an option for those people that know their code can handle it. Principle of least surprise at work there...)

[kizu]

I'm also against enabling such option by default.

1. While that optimization is marked as safe, without through testing (especially with old browsers) it can't be considered as such.
2. Even minified code still can be tweaked in dev tools. Changing units makes it harder for developers, designers and managers to tweak values on the live site, by moving items here and there by several pixels/changing elements' dimensions.
3. There should be tests with the profit after gzip. px everywhere is better gzippable vs. different units.
4. Such code is harder to work with when used with different js polyfills that parse CSS in runtime. There is a possibility that polyfill's authors won't parse pt values properly, resulting to behaviour bugs after this optimization.

[bcarbonn]

I'm also against enabling such option by default, please correct this asap...

[ahmedelgabri]

+1 for not having this option enabled by default.

[avromao]

+1

[sylvainbaronnet]

+1

I was like wtf happened to my css^^

[bgriffith]

+1
Same as above!

[DanPurdy]

+1

Seems a little much.. Please disable this by default.

[smilingcheater]

+1 for disabling this by default

[PeterRao]

+1 for disabling this by default

[ruandre]

+1 for disabling this by default.

For one thing, it screws up HTML emails.

[jakubpawlowicz]

@rjgotten that's an argument against getComputedStyle in general - if one day browsers change the implenatation according to 2.1 spec then we'll have a problem anyway

@kizu it was tested in IE7 up to 11 and Edge, old and new Firefox, Chrome, Safari, and Opera. Re 2 & 4 the same applies to other transformations, re 3 it's not implemented anyway

@ruandre does it happen in browsers or in email clients?

I am still looking forward to a reproducible rendering issue as per https://github.com/jakubpawlowicz/clean-css/blob/master/CONTRIBUTING.md - if there's such I will seriously consider reverting the change.

[kfitzgerald]

Hi @jakubpawlowicz !

I'm not sure that reverting the change is the best option. I feel like adjusting the existing functionality to keep apples-to-apples (pixels) and oranges-to-oranges (physical units) would be a better alternative, satisfying the vast majority of cases.

Thanks again,
-Kevin

[jakubpawlowicz]

Thanks @kfitzgerald - yeah, that's what I meant.

[kizu]

@jakubpawlowicz

Re 2 & 4 the same applies to other transformations

Can you provide examples of those transformations?

For example, the first google result for “position sticky polyfill codepen” can't understand pt instead of px (it just treats them as px, haha)., oops, codepen just didn't update the code after changing the units, my bad. Nevertheless, there are a lot of similar polyfills where authors forgot to tweak print units, handling only px, em and %.

Right now clean-css would break such polyfilled code. Other transformations don't have this issue, as authors would expect users to use both padding and padding-left for paddings for example.

Regarding dev tools — which transformations make it harder to tweak values at the same level as converting units?

[ryami333]

+1 for disabling this by default.
I just don't even understand why a minification plugin is going anywhere near my units.

[dukhevych]

+1 for disabling this by default

[ACastans]

+1 for disabling this by default.
I lost 1 day to understand what happend to my css output and trying to find a fix.
It's a nice feature but it shouldn't be a default one.
I think every developper should be free to decide what to a do with his unit css in his dev.
And it's not very easy to understand how to disable or enable it....

[rjgotten]

And it's not very easy to understand how to disable or enable it....

As far as I understand, it's not even possible to disable the optimization cleanly; you have to disable parsing/using the target unit of the conversion (pt in this case) entirely. That's not an option switch; that's a hack.

[Leolik]

+1 for disabling this by default.

[ghost]

+1 for disabling this by default.

[joe-watkins]

+1 for disabling this by default.

[jakubpawlowicz]

It's released as 3.4.2 where length unit optimizations are disabled by default. Those can be brought back using shorterLengthUnits switch: https://github.com/jakubpawlowicz/clean-css/blob/master/test/selectors/simple-test.js#L833

Thanks everyone for all the comments and +1s. It made me realize it wasn't the best idea to turn it on by default.

[kfitzgerald]

Thanks @jakubpawlowicz ! Much appreciated.

[yisibl]

There are a lot of radical compression should be disabled by default.

[jakubpawlowicz]

@yisibl Will deal with it in the next major release.

[rene-poulsen]

+1 for disabling it by default

[jakubpawlowicz]

@rene-poulsen it's off in 3.4.2+.

[yisibl]

@jsw0528 fix.

[antife-yinyue]

Cooooooooooooooooooooooooool~

[laurent-le-graverend]

+1 thanks for disabling it, it gave me headaches to understand why a very specific case got broken in Safari due to the px to picas conversion.

[devfreddy]

@jakubpawlowicz +1 for conceding to peer pressure gracefully (i.e. listening to user demands), thank you, much appreciated.