In WebCore/css/MediaQueryEvaluator.cpp the method device_aspect_ratioMediaFeatureEval() confuses width and height in line 241. As a result, the MQ evaluation of "device-aspect-ratio" always fails. To correct the bug, line 241 must read: return b != 0 && cmpvalue(a * (int)sg.width(), b * (int)sg.height(), op); <rdar://problem/5380295>
Created attachment 15853 [details] PAtch for bug Patch from ADC developer
Comment on attachment 15853 [details] PAtch for bug Fix looks fine. Needs a regression test.
Reassigning to webkit-unassigned for more visibility. All we need is a test case to land this fix.
(In reply to comment #3) > Reassigning to webkit-unassigned for more visibility. > > All we need is a test case to land this fix. Actually, I believe the current code is correct (but with poorly-named variables). We should be cross-multiplying the values to compare them, which the current code does. http://www.w3.org/TR/css3-mediaqueries/#device-aspect-ratio I'm going to write test cases for [min-|max-|]device-aspect-ratio and fix the variable names.
(In reply to comment #4) > Actually, I believe the current code is correct (but with poorly-named > variables). We should be cross-multiplying the values to compare them, which > the current code does. > > http://www.w3.org/TR/css3-mediaqueries/#device-aspect-ratio The current code is incorrect, but so is the patch (Attachment #15853 [details]). The entire first and second arguments need to be swapped, not just the sg.width() and sg.height() calls. > I'm going to write test cases for [min-|max-|]device-aspect-ratio and fix the > variable names. Test cases are a Good Thing(tm).
Created attachment 17078 [details] Patch v2 Now with correct fix and 3 layout tests.
Comment on attachment 17078 [details] Patch v2 r=me (I probably would have used h/v instead of longer names.)
The test cases also pass on Opera 9.50b and Firefox 2.0.0.9 on Tiger. (Note that the shouldBe() test fails on Opera 9.50b because computed styles are accessed differently than Safari/Firefox. The text turns green as it is supposed to, though.)
(In reply to comment #8) > Note > that the shouldBe() test fails on Opera 9.50b because computed styles are > accessed differently than Safari/Firefox. Filed Bug 15888 to provide cross-browser method for getComputedStyle().
Committed revision 27581.
(In reply to comment #7) > (I probably would have used h/v instead of longer names.) I made this change before committing.