Closed Bug 840916 Opened 7 years ago Closed 7 years ago

Wrong values for media query min-resolution

Categories

(Firefox for Android :: General, defect)

18 Branch
All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 25
Tracking Status
fennec + ---

People

(Reporter: koch, Assigned: kats)

References

Details

(Keywords: testcase)

Attachments

(2 files)

Attached file mq_resolution.zip
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:18.0) Gecko/20100101 Firefox/18.0
Build ID: 20130201065344

Steps to reproduce:

I tested media queries using min-resolution with both dpi and dppx. Attached are two test files: mq.html contains

<meta name="viewport" content="width=device-width, initial-scale=1.0">

, mq2.html doesn't. Both use the external JavaScript mq.js. I tested with Firefox Mobile 18.0 for Android on an LG E960 (Google Nexus 4).


Actual results:

min--moz-device-pixel-ratio: 1
min-resolution [dppx]: 3.3dppx
min-resolution [dpi]: 320dpi

BTW, window.devicePixelRatio also is 1.


Expected results:

After reading <https://developer.mozilla.org/en-US/docs/CSS/Media_queries#-moz-device-pixel-ratio> I would expect:
min--moz-device-pixel-ratio: 2
min-resolution [dppx]: 2dppx
min-resolution [dpi]: 192dpi

window.devicePixelRatio should report 2.

Compare results from Chrome 18 (on the same device):
-webkit-min-device-pixel-ratio: 2
(window.devicePixelRatio: 2)

and Opera Mobi 12.10 (on the same device):
-o-min-device-pixel-ratio: 20/10
min-resolution: 2dppx
min-resolution: 192pi
(window.devicePixelRatio: 2)
Testfiles are online (at least for some time): <http://waldbaer.leute.server.de/display/mq.html>, <http://waldbaer.leute.server.de/display/mq2.html>
Status: UNCONFIRMED → NEW
Depends on: 779527, 803207
Ever confirmed: true
Keywords: testcase
OS: Windows 7 → Android
Hardware: x86 → All
Some more test results...

Firefox Tablet Android 18.0 on ASUS Google Nexus 7 running Android 4.2:

window.devicePixelRatio: 1
mq min--moz-device-pixel-ratio: 1
mq min-resolution: 2.2dppx
mq min-resolution: 213dpi
mq width: 534px

Compare Chrome 18.0 on the same platform:

window.devicePixelRatio: 1.3312499523162842
mq -webkit-min-device-pixel-ratio: 1.3
mq width: 600px

Compare Opera Tablet 12.10 on the same platform:

window.devicePixelRatio: 1.5
mq -o-min-device-pixel-ratio: 15/10
mq min-resolution: 1.5dppx
mq min-resolution: 144pi
mq width: 533px


Firefox Mobile Android 19.0 on Samsung GT-N7000 running Android 2.3.5:

mq min--moz-device-pixel-ratio: 1
mq min-resolution: 3.3dppx
mq min-resolution: 320dpi
mq width: 400px

Compare Android Native Browser on the same platform:

window.devicePixelRatio: 2
mq -webkit-min-device-pixel-ratio: 2
mq width: 400px

Compare Opera Mobi 12.10 on the same platform:

window.devicePixelRatio: 2
mq -o-min-device-pixel-ratio: 20/10
mq min-resolution: 2dppx
mq min-resolution: 192dpi
mq width: 400px
 

Firefox Mobile Android 21.0 on Sony C6603 (Xperia Z) running Android 4.1.2:

window.devicePixelRatio: 1
mq min-moz-device-pixel-ratio: 1
mq min-resolution: 4dppx
mq min-resolution: 400dpi
mq width: 360px

Compare Chrome 18.0 on the same platform:

window.devicePixelRatio: 3
mq -webkit-min-device-pixel-ratio: 3
mq width: 360px
tracking-fennec: --- → ?
tracking-fennec: ? → +
Can you please re-test on the latest nightly build? Bug 803207 is fixed now which should fix most of this. window.devicePixelRatio at least should definitely be fixed, if not all of these properties.
Flags: needinfo?(koch)
I tested with a nightly build on Sony C6603 (Xperia Z) running Android 4.1.2. Results:

window.devicePixelRatio: 3
min--moz-device-pixel-ratio: 3

That's good, but:

min-resolution: 4dppx
min-resolution: 400dpi

That's still wrong. min-resolution should be 3dppx and 288dpi (3*96).
Flags: needinfo?(koch)
Presumably this mismatch comes from min-resolution reporting the actual resolution of the device, and window.devicePixelRatio reporting what we're using in Gecko to map between CSS pixels and device pixels. The Xperia Z must be reporting a DPI between 450 and 600 (note that your mq.js file maxes out at 4dppx / 400dpi, and will never report values higher than that), so that's why we end up using a devicePixelRatio of 3 (see code at [1]).

Presumably if we implement what mbrubeck suggested at the bottom of [2] then you'll end up with different numbers for window.devicePixelRatio (something closer to 2.8, I think) but min-resolution will still be the same.

I think your assumption about 96dpi being the scaling factor between window.devicePixelRatio and min-resolution is incorrect; that 96 would actually be 160 for consistency with other Android apps (see [3]).

IMO the only thing left to be done here is to change our devicePixelRatio to be the same as DisplayMetrics.density (probably in a separate bug) and close this one as INVALID. Do you disagree?

[1] http://mxr.mozilla.org/mozilla-central/source/widget/android/nsWindow.cpp#342
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=803207#c9
[3] http://developer.android.com/reference/android/util/DisplayMetrics.html#density
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5)
> Presumably this mismatch comes from min-resolution reporting the actual
> resolution of the device, and window.devicePixelRatio reporting what we're
> using in Gecko to map between CSS pixels and device pixels.

I think we need to use the latter value (device pixels per CSS px) in both cases.  For details see bug 771390.

What causes these to use different values, anyway? Is it the code I added in bug 779527?  Perhaps that should be changed to use GetDefaultScale now that GetDefaultScale contains the correct logic on Android.  Or maybe we can just remove that code now..?

> I think your assumption about 96dpi being the scaling factor between
> window.devicePixelRatio and min-resolution is incorrect; that 96 would
> actually be 160 for consistency with other Android apps (see [3]).

1dppx == 96dpi by definition for CSS media queries, since 1 CSS 'in' == 96 CSS 'px': http://www.w3.org/TR/CSS2/syndata.html#length-units

A device pixel is a "dot" in CSS resolution terms.  http://w3c-test.org/csswg/mediaqueries3/#units defines 'dpi' as "dots per CSS in" which by definition is "dots per 96 CSS px".  And of course 'dppx' is "dots per CSS px".
Yes I disagree. Thanks, Mark for the explanation.
Ok, makes sense.

I pushed a try build with bug 779527 backed out to https://tbpl.mozilla.org/?tree=Try&rev=c7fba21a52ca but I don't have a hidpi device at the moment to test it out on.
So on my Nexus 4 the above try build shows:

min--moz-device-pixel-ratio: 2
min-resolution [dppx]: 2dppx
min-resolution [dpi]: 192dpi

which looks correct as per the discussion above. Johannes, do you mind trying out the build at [1] and let me know if you see any other problems with it? That build will be named "Nightly" when you install it, and cannot co-exist with other nightly builds on your device.

[1] http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/kgupta@mozilla.com-c7fba21a52ca/try-android/fennec-25.0a1.en-US.android-arm.apk
Flags: needinfo?(koch)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #9)
> So on my Nexus 4 the above try build shows:
> 
> min--moz-device-pixel-ratio: 2
> min-resolution [dppx]: 2dppx
> min-resolution [dpi]: 192dpi
> 
> which looks correct as per the discussion above.

Yep

> Johannes, do you mind
> trying out the build at [1] and let me know if you see any other problems
> with it? That build will be named "Nightly" when you install it, and cannot
> co-exist with other nightly builds on your device.
> 
> [1]
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/kgupta@mozilla.com-
> c7fba21a52ca/try-android/fennec-25.0a1.en-US.android-arm.apk

This test build on Sony C6603 (Xperia Z) running Android 4.1.2 shows:

min--moz-device-pixel-ratio: 3
min-resolution [dppx]: 3dppx
min-resolution [dpi]: 288dpi

That's ok, thanks.
Flags: needinfo?(koch)
Running tests on try at https://tbpl.mozilla.org/?tree=Try&rev=11816aae7fc6
Assignee: nobody → bugmail.mozilla
Attachment #775699 - Flags: review?(mbrubeck)
Comment on attachment 775699 [details] [diff] [review]
Backout bug 779527

Review of attachment 775699 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/style/test/test_media_queries.html
@@ +527,5 @@
>    var dpi_high;
>    var dpi_low = resolution - 1;
>    if (query_applies("(min-resolution: " + resolution + "dpi)")) {
>      // It's exact!
> +    is(resolution % 96, 0, "resolution should be a multiple of 96dpi");

The test changes should not be reverted -- these assertions are still incorrect when running on a device where the default scale is not an integer (e.g. Android devices with HDPI / 1.5dppx displays).
Attachment #775699 - Flags: review?(mbrubeck) → review+
Try is green (presumably because none of our Android test platforms have a HDPI display). I removed the test changes from the patch and pushed to inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/75465ea1cfb6
https://hg.mozilla.org/mozilla-central/rev/75465ea1cfb6
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
You need to log in before you can comment on or make changes to this bug.