search

tubalmartin / YUI-CSS-compressor-PHP-port

A PHP port of the YUI CSS compressor.
230 stars 34 forks source link

Missing space character causes rendering issues in WebKit #40

Closed denzel100 closed 7 years ago

denzel100 commented 7 years ago

Hi,

minifying "-webkit-text-size-adjust: none;" removes the space character and breaks WebKit rendering. Confirmed for Safari 5, Safari 9.1 and Midori 0.5.11. Safari 10 and other modern browsers appear to correct this bug.

tubalmartin commented 7 years ago

Thanks for reporting @denzel100 Could you check whether that happens with "auto" and percentage values too?

denzel100 commented 7 years ago

I did some further testing. The problem appears to be a bit more general and obscure than that. The css where I first observed this issue is part of the popular bbPress WordPress plugin. It contains this css:

@media screen and (max-device-width: 480px), screen and (-webkit-min-device-pixel-ratio: 2) {
    -webkit-text-size-adjust: none;
}

I have created a minimal working example page:

<html><head><style>
@media screen {
    color:black;
}
body { background-color: blue; }
</style><body></body></html>

If you insert a space after the colon and reload, the background turns blue. It would seem the missing space causes css execution to stop, but only under very specific circumstances. If instead of "black" you enter "5", it works. The bug appears if the value starts with an alphabetic character (including umlauts) or underscore, but not with numbers or any of the other special characters I've tested. Also, adding a second declaration to the media query fixes the issue, as does nesting the single declaration inside a block.

So, as far as my current testing indicates, the bug could be formulated as follows: Media queries containing a single declaration at the global level that has no space after the colon and a value starting with an alphabetic character, including umlauts and the underscore, cause css execution to stop prematurely in the WebKit versions used by the browsers mentioned earlier.

tubalmartin commented 7 years ago

Ummm I see. As far as I know, you cannot declare a property inside a media query without enclosing it inside a selector. So I guess, that bbPress WP plugin has a bug in its CSS code.

Correct code should be, if I'm not wrong, something like this:

@media screen and (max-device-width: 480px), screen and (-webkit-min-device-pixel-ratio: 2) {
  html {
    -webkit-text-size-adjust: none;
  }
}

Please modify that plugin's code with mine. Give it a try and report.

denzel100 commented 7 years ago

Thanks for your response. I had the same thought (and it does solve this particular display issue), but I didn't know enough about the css standards to be sure. However, I just remembered the w3c css validator and confirmed that declarations at the media query level are incorrect syntax, so there is indeed a bug in the plugin's css. I've looked into it, and the bug appears to be fixed in their current beta. I'm not sure about the details, but it would seem some css processor in their build chain was to blame.

So, since I'm now aware that the css was invalid to begin with, and the missing space character is symptomatic only under very specific circumstances and with older WebKit versions that will die out eventually, I no longer believe a workaround at the level of minification is warranted. I'll go ahead and close this issue.