User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/29.0.1547.76 Safari/537.36 Steps to reproduce: I wrote some JavaScript to calculate the current zoom factor. The code is as follows: var zoomFactor = window.devicePixelRatio * screen.width / window.innerWidth; Tested with Firefox 25 beta. Actual results: window.innerWidth stays the same all the time - 980px Expected results: The more you zoom the lower the value for window.innerWidth. Works in other browsers on Nexus 10 (Chrome, Opera, ...).

Please test against trunk. htts://nightly.mozilla.org

I can repro as well. Is there a spec that you can point to as to what window.innerWidth is actually supposed to be in this case? I was under the impression it's the CSS viewport width so in that case what we return for that is correct.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #2) > Is there a spec that you can point to Not really a spec as such but take a look at the section "Calculate zoom level" on this page: http://www.quirksmode.org/mobile/tableViewport.html

(In reply to Kevin Brosnan [:kbrosnan] from comment #1) > Please test against trunk. htts://nightly.mozilla.org I have now done so. It is the same behaviour. Whether I have maximum zoom or minimum zoom (or anywhere in between) the values are as follows: window.devicePixelRatio -> 2 screen.width -> 1280 window.innerWidth -> 980

Matt, do you know if window.innerWidth is specced anywhere? According to the quirksmode article in comment 3 most browsers seem to return the size of the visual viewport whereas we return the layout viewport. Not sure which is more correct, and what will break if we switch.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5) > Matt, do you know if window.innerWidth is specced anywhere? As far as I know it is not specified anywhere. PPK has collected some info on how various mobile browsers behave: http://www.quirksmode.org/mobile/tableViewport.html#t01 http://www.quirksmode.org/blog/archives/2012/10/mobile_viewport.html

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5) > Not sure which is more correct, and what will break if we switch. If you do not want to change window.innerWidth can you make available a different way to calculate the current zoom factor?

(In reply to Ray Shanley from comment #7) > If you do not want to change window.innerWidth can you make available a > different way to calculate the current zoom factor? I would rather try to change window.innerWidth first, so that we are consistent with the de-facto standard. This bug probably is more appropriate in Layout since it affects the GetInnerWidth and GetInnerHeight functions that are implemented in layout. They should return the size of the scroll-clamping scroll-port size if there is one. We will also need to audit all uses of window.innerWidth in our own code to make sure they do not break.

Btw, I think this bug will also affect B2G and Metro.

So the current implementation of GetInnerWidth and GetInnerHeight end up returning the size as obtained by nsPresContext->GetVisibleArea. The docs for that function sound like they should match the desired behaviour in this bug, but clearly the function is not actually returning that value. :tn, do you know if it makes sense to change what GetVisibleArea returns to take into account the resolution, or if it's better to just change nsGlobalWindow::GetInnerSize() to also check the ScrollPositionClampingScrollPortSize if there is one?

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #10) > So the current implementation of GetInnerWidth and GetInnerHeight end up > returning the size as obtained by nsPresContext->GetVisibleArea. The docs > for that function sound like they should match the desired behaviour in this > bug, but clearly the function is not actually returning that value. The docs were written before mobile stuff came around (or the docs didn't take that into account). The visible area is the layout size of the document, the css viewport size. It is the size that the document will be reflowed into. I think the visible area has the correct value, just the description of it is not clear. > :tn, do you know if it makes sense to change what GetVisibleArea returns to > take into account the resolution, or if it's better to just change > nsGlobalWindow::GetInnerSize() to also check the > ScrollPositionClampingScrollPortSize if there is one? So probably changing the inner size getter is the way to go given what I said above.

Zooming in and clicking the button should report different sizes. If you zoom it so that exactly three boxes fit on the screen then the alert dialog should report 300.

This makes the test case work correctly, but will probably break a bunch of stuff because of how we use innerWidth and innerHeight in our code. https://tbpl.mozilla.org/?tree=Try&rev=d6a200ca51d2

That was a surprisingly clean try run. And it turns out Fennec code only uses innerWidth in a couple of places in reader mode. I exercised those pieces of code and they behave fine. And in B2G the only presshell that will (at least right now) have the ScrollPositionClampingScrollPortSize set will the browser content presshell, and that also seems to be ok. So this patch might actually not break much (fingers crossed).

Comment on attachment 810977 [details] [diff] [review] Patch The FlushDelayedResize call there is only to make sure that the visible area on the prescontext is up to date. So I would do that only if we are using the visible area. Otherwise looks good.

Updated per :tn's comment above

Comment on attachment 811269 [details] [diff] [review] Patch v2 Review of attachment 811269 [details] [diff] [review]: ----------------------------------------------------------------- Could use a test

One Linux reinstall later, I have a test! Works locally on my Android device, pending try run at https://tbpl.mozilla.org/?tree=Try&rev=a2071162265f

Comment on attachment 812371 [details] [diff] [review] Test Review of attachment 812371 [details] [diff] [review]: ----------------------------------------------------------------- !

I see bustage on inbound so flagging as checkin-needed rather than piling on top.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7e8f946d0d1a https://hg.mozilla.org/integration/mozilla-inbound/rev/47451aa820b8

https://hg.mozilla.org/mozilla-central/rev/7e8f946d0d1a https://hg.mozilla.org/mozilla-central/rev/47451aa820b8

I've tried it out on the Nexus 10 with the nightly build (27.0a1 2013-10-02) but the value for window.innerWidth is still 980 same as before. Is the change already in that nightly build?

It should be in today's nightly build (Oct 03).

Thank you - I've now tested it and 'window.innerWidth' works perfectly. 'window.innerHeight' also works better but there still seems to be a small problem with innerHeight as follows. When the zoom is greater than the minimum and the address bar is visible, 'window.innerHeight' is too large i.e. the height of the address bar is not subtracted from the total height. I've opened a new bug for that: bug 923663.