1025833 - Aurora selects wrong URI from img srcset when zoomed

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Johannes Koch]

Attachment: The attached file is the HTML used in the linked test case.

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:32.0) Gecko/20100101 Firefox/32.0 (Beta/Release)
Build ID: 20140614004002

Steps to reproduce:

I tested this in current Aurora (Mozilla/5.0 (Windows NT 6.1; rv:32.0) Gecko/20100101 Firefox/32.0) with dom.image.srcset.enabled true.
Given the following code:

<img src="900x450.png" width="300" srcset="225x112.png 0.75x, 300x150.png 1x, 450x225.png 1.5x, 600x300.png 2x" alt="Test.">

(available at http://waldbaer.leute.server.de/srcset/)

I change the zoom (CTRL + '-' or CTRL + '+') and then reload.


Actual results:

The following URIs are selected for the following DPRs:

225x112.png for DPR: 0.30000001192092896
300x150.png for DPR: 0.5, 0.6666666865348816, 0.8, 0.89552241563797, 1, 1.0909091234207153, 1.2000000476837158, 1.3333333730697632
600x300.png for DPR: 1.5, 1.7142857313156128, 2, 2.4000000953674316
900x450.png for DPR: 3



Expected results:

I would have expected the following URIs to be selected for the following DPRs:

225x112.png for DPR <= 0.75
300x150.png for 0.75 < DPR <= 1
450x225.png for 1 < DPR <= 1.5
600x300.png for 1.5 <= DPR <= 2
900x450.png for 2 < DPR <= 3

So
225x112.png for DPR: 0.30000001192092896, 0.5, 0.6666666865348816
300x150.png for DPR: 0.8, 0.89552241563797, 1
450x225.png for DOR: 1.0909091234207153, 1.2000000476837158, 1.3333333730697632, 1.5
600x300.png for DPR: 1.7142857313156128, 2
900x450.png for DPR: 2.4000000953674316, 3

Comment 1

[Johannes Koch]

Sorry, the code I tested is

<img src="300x150.png" width="300" srcset="225x112.png 0.75x, 300x150.png 1x, 450x225.png 1.5x, 600x300.png 2x, 900x450.png 3x" alt="Test.">

Comment 2

[John Schoenick [:johns]]

The issue here is PresContext::CSSPixelsToDevPixels has a int -> int as well as a float -> float definition, so |double CSSPixelsToDevPixels(1)| is invisibly rounding our current density estimate to the nearest int.

http://dxr.mozilla.org/mozilla-central/source/content/base/src/ResponsiveImageSelector.cpp#230

Comment 3

[John Schoenick [:johns]]

Attachment: Fix img srcset inadvertently rounding density estimate to nearest int

(The tests in bug 1023516 will catch this)

Comment 4

[John Schoenick [:johns]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8d2002e67d13

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8d2002e67d13

Comment 6

[John Schoenick [:johns]]

Comment on attachment 8440889 [details] [diff] [review]
Fix img srcset inadvertently rounding density estimate to nearest int

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
bug 870021 (introduced srcset support)

User impact if declined: 
Incorrect behavior for developers testing srcset feature with dom.image.srcset.enabled, no effect on default configuration.

Testing completed (on m-c, etc.): 
on m-c

Risk to taking this patch (and alternatives if risky): 
None, only affects new feature behind pref, and trivial patch at that.

String or IDL/UUID changes made by this patch:
None

Comment 7

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8440889 [details] [diff] [review]
Fix img srcset inadvertently rounding density estimate to nearest int

Aurora approval granted.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8d78b04d12cb