New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 333035 link

Starred by 23 users

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Feb 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Chrome 32 m is not recognizing multiple expression media queries with the word "and" directly following a closing parenthesis.

Reported by jasonbuc...@gmail.com, Jan 9 2014

Issue description

Chrome Version       : Version 32.0.1700.72 m
URLs (if applicable) : http://www.jasonbuckboyer.com/playground/anc/blah.html
Other browsers tested:
    Safari 6: OK
  Firefox 20: OK
    IE 10/11: OK

What steps will reproduce the problem?
1. Add these styles to a page:
<style>
@media only screen and (min-width:0px) and (max-width:4000px){ body { background-color:blue; } }
@media only screen and (min-width:0px)and (max-width:4000px){ body { background-color:red; } }
</style>
2. View the page in all browsers
3. Note that only chrome displays the background color as blue.

What is the expected result?
The media query should be recognized, making the page red.

What happens instead?
All of the styles inside of the media query are ignored, making the page blue.


Please provide any additional information below. 

According to the specification, the space is optional: http://www.w3.org/TR/css3-mediaqueries/#syntax
 
which-one-stands-out.png
103 KB View Download
This also applies to the other keywords 'only' and 'not'

Comment 2 by ilplot...@gmail.com, Jan 10 2014

We have this issue in production version due to CSS-optimizer (http://css.github.com/csso/) which removes unnecessary spaces in media queries.

Comment 3 by blow...@gmail.com, Jan 10 2014

I beleive Ajax Min produces the same results when minified.
Cc: komoroske@chromium.org
Labels: Cr-Blink Needs-Bisect M-32 ReleaseBlock-Stable
Can we get a Blink bisect of this?

Alex heads up we've got a media query bug in M32 that's going out; who knows about media queries on the Blink  eng team?

Marking stable blocker until we know how bad this is
Cc: wiltzius@chromium.org
Cc: r...@opera.com

Comment 7 by tasak@google.com, Jan 10 2014

CSSGrammar.y has the following:

media_query_exp_list:
    media_query_exp {
	$$ = parser->createFloatingMediaQueryExpList();
        $$->append(parser->sinkFloatingMediaQueryExp($1));
    }
    | media_query_exp_list space MEDIA_AND space media_query_exp {
        $$ = $1;
        $$->append(parser->sinkFloatingMediaQueryExp($5));
    }
    ;


I guess, the "space" might be "maybe_space" or something.

Comment 8 by r...@opera.com, Jan 10 2014

Status: Untriaged
It's because of [1]. If we're the only ones implementing this errata, and it's causing problems, it should probably revert it.

[1] https://code.google.com/p/chromium/issues/detail?id=288803

I'm checking all CSS minifiers to see which trigger this issue.  

Initial report looks decent. 

sass 3.3.x: OK!
sass 3.2.x: OK!
LibSass: OK!
Stylus: OK!
cleancss (used by grunt-contrib-cssmin): OK!
LESS: OK!
YUI Compressor 2.4.8: OK!
cssminifier.com: OK!
willpeavy.com/minifier/: OK!
freeformatter.com/css-minifier.html: OK!

Probably another 7 or so to check…
csso: FAIL.
http://bem.info/tools/optimizers/csso/

Ajax Min (.net): FAIL. (reported in comment #3)
http://ajaxmin.codeplex.com/


csstidy (php): OK!
slimmer (python): OK!
cssmin (ruby gem): OK!
Closure Stylesheets: unknown, but presumably OK.

minify (php): unknown…
https://code.google.com/p/minify/wiki/UserGuide

----

JasonBuckBoyer, yngve.nilsen: Are you also using csso?

It looks like csso and AjaxMin are the only minifiers that will make a problem here. 

Comment 11 Deleted

No we are not using csso. We are using an in-house minifier built from the CSS specs at the time.

We should revert rune's commit [1] and merge it up to stable before shipping to m32.
[1] https://src.chromium.org/viewvc/blink?revision=158717&view=revision

This affects Microsoft's Ajax Minifier which is basically the default tool for .NET projects to minify their CSS. It's been around for 4 years and has a few hundred thousand downloads. 

The bug is also triggered by the `csso` project.

I bet the .NET community is not on Beta nearly as much so the 10 stars now will likely balloon to FAR more if we let this slip to stable. It's a regression and it breaks the page pretty severely, making appearance on mobile devices totally broken.

Thank you all for addressing this so quickly. The site I work on has several million active users and this bug would severely break our mobile experience.
AjaxMin has committed a fix already: https://ajaxmin.codeplex.com/SourceControl/changeset/106150

And just to point out, it's not just beta channel users who are seeing this issue. I'm on the stable channel but I was apparently "part of a small set of lucky stable channel users that get the new release candidate early." (Which I didn't realize was a thing. Took me a while to figure out why I was seeing a Beta version.)

http://productforums.google.com/forum/#!topic/chrome/zmgiVCv9X0A%5B1-25-false%5D

Comment 16 by r...@opera.com, Jan 10 2014

I've prepared a revert for master:

https://codereview.chromium.org/134893002/

Cc: kareng@google.com le...@chromium.org
Labels: Merge-Requested
levi is going to double-check the revert for safety, but heads up Karen that it may be safer to revert this than leave as-is

Comment 19 by le...@chromium.org, Jan 10 2014

Cc: eseidel@chromium.org
I've been told that Eric is a reliable source for this. I'm sadly not super familiar with CSSGrammar.y
dglazkov@ suggests we take the change based on a cursory glance at it
Project Member

Comment 22 by bugdroid1@chromium.org, Jan 11 2014

Thank you Eric and Levi!

Comment 24 by kareng@google.com, Jan 11 2014

Labels: Merge-Merged
ty ty!

we merged this to M32 so pls keep an eye out for any issues and let us know so we can revert.

Comment 25 by kareng@google.com, Jan 11 2014

Labels: OS-All
Amazing work guys! i'm using Microsofts minifier btw. Had a million chromeusers pr day, but managed to write a workaround which I blogged about here: http://blog.novanet.no

Comment 27 by r...@opera.com, Jan 13 2014

@kareng: the M32 merge review I got only reverted the layout tests. Pls have a look.

Comment 28 by kareng@google.com, Jan 13 2014

yes no worries, i had to do it over. this r164914 looks right, right?

Comment 29 by kareng@google.com, Jan 13 2014

this will not be in stable1 for desktop but we'll pick it up in round 2.
Yeah I think that's OK, not great but we'll have to roll with it. Paul +
other DevRel folks can help do some outreach if any sites get broken.
New stable AjaxMin released on Saturday to enforce the erratum-required whitespace: https://ajaxmin.codeplex.com/releases/view/117194
Verified that url in #0 : http://www.jasonbuckboyer.com/playground/anc/blah.html loads red on Android Chrome M32/32.0.1700.99 with:
New Nexus 7/KOT49C(Beta)
Nexus 4/KOT49M (Beta)
HTC ONE X /JDQ39(stable apk) 


Labels: TE-Verified-32.0.1700.98 TE-Verified-M32
Tested this fix on Latest M32#32.0.1700.98 (Official Build 244465) on Win7, Linux Ubuntu 12.04 & Mac OS X 10.9.1 - Working as intended.

Thank you.

Comment 34 by Deleted ...@, Jan 15 2014

Thank you for fixing this! Threw my whole day off. When can we expect an update? M32#32.0.1700.76 still has this bug.
Confirm that the issue is still reproduced in 32.0.1700.76 m.
It is strange as this should be stable release and the issue is labeled as "ReleaseBlock-Stable".
See http://googlechromereleases.blogspot.com/2014/01/stable-channel-update.html:
"The Chrome Team is excited to announce the promotion of Chrome 32 to the Stable channel. 32.0.1700.76 for Windows..."

When can we expect an update? M32#32.0.1700.76 still has this bug.
Labels: -Needs-Bisect

Comment 38 by kareng@google.com, Jan 17 2014

Labels: -M-32 M-33
this is merged to m32. i think it may need to be merged to m33. 

Comment 39 by laforge@google.com, Jan 18 2014

Labels: -Merge-Requested Merge-Approved

Comment 40 by laforge@google.com, Jan 24 2014

Owner: eseidel@chromium.org
Status: Assigned
Cc: rponnada@chromium.org
Labels: Needs-Feedback
Issue is fixed on Win7, MAC & Linux - M32 :: 32.0.1700.102 (Official Build 246481) m

Issue is still repro on Win7 M33:: 33.0.1750.54 (Official Build 247046) m

Please confirm, if any plans to merge into M33 for further verification on M33. 
Ran:
drover --merge 164913 --branch 1750
Issue created. URL: https://codereview.chromium.org/148403006
Committed revision 165888.

Project Member

Comment 43 by bugdroid1@chromium.org, Jan 27 2014

Confirming that the fix is working in 32.0.1700.102
Thank you Version 32.0.1700.102 m update fixed this issue
Status: Fixed
I believe this has been merged to M33 as well?

Comment 47 by Deleted ...@, Apr 14 2014

Not sure about it being fixed since I'm seeing such issues with Ver. 34.0.1847.116 m. Media queries are being recognized by my W7 Safari, FF, and IE, but not Chrome.

Any update?
Cc: y...@yoav.ws
This was confirmed as fixed in M32 and M33.  r164913 is definitely in M34 (whose base blink-revision is r167304).

The original test page http://www.jasonbuckboyer.com/playground/anc/blah.html definitely looks red (correctly) in M34.

Toad: I think whatever bug you are seeing may be different from this one?  Could you please file a new bug with a new test case?

Comment 49 by y...@yoav.ws, Apr 15 2014

Toad: Could you please CC me on related future bugs? 

Sign in to add a comment