Closed Bug 942019 Opened 7 years ago Closed 7 years ago

SessionStore doesn't take Windows DPI settings into account when restoring window position

Categories

(Firefox :: Session Restore, defect)

All
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 28

People

(Reporter: markh, Assigned: markh)

References

Details

Attachments

(1 file)

STR:
* Quit Fx, noting the window position
* Start Fx

The Window is initially restored correctly, but immediately after it is shown the window moves down and to the right by a small amount (100px or so I'd guess)

Note that in a release build, this happens quickly enough that it's not always clear it was initially restored correctly (but is obvious the it ends up in the wrong place).  In a debug build it is very obvious.  The fact it is initially restored correctly means this may not actually be a session store bug.

This is a recent regression - may or may not be related to Australis.  I have DPI settings set to 125%, which also may or may not be related.
hrmph - I can actually repro this on Aurora too - the offset seems smaller there though.
I can also repro on the release channel.

The problem is the DPI settings.  On my system, nsIDOMWindowUtils.screenPixelsPerCSSPixel == 1.25, and this is the factor by which the X and Y position (but *not* the width and height) is wrong by.

With the following patch, things work correctly.  However, I'm not sure if the scaling is better applied at save time or at restore time - this patch does it at save time.  Also, I'm not at all clear on why the width and height needn't be adjusted in the same way?

Also, FWIW and IIUC, Windows 8 defaults to a scaling factor of 1.25 on most laptops, so I suspect people are hitting this in the wild when the screen isn't maximized.
Summary: On startup, window position restored correctly then immediately moves down and to the right → SessionStore doesn't take Windows DPI settings into account when restoring window position
Tim, what is the best way to turn this into a "real" patch, or otherwise push this forward?
Flags: needinfo?(ttaubert)
Thanks for figuring this out! I think we should store window dimensions in CSS pixels. It probably doesn't completely make sense to try and keep the position when DPI settings or the output device change but whatever.

ss.restoreDimensions() wants CSS pixels [1] so we shouldn't need to change anything about that function. ss._getWindowDimension() serializes outerWidth, outerHeight, screenX, and screenY.

screenX/Y return CSS pixels [2]. outerWidth/Height do so as well [3]. I'm confused now - why does your change work and why do we need it? Did I miss anything?

Also, it seems I can't reproduce the issue on my rMBP where devicePixelRatio=2.

[1] http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#6668
[2] http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#4738
[3] http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#4615
Flags: needinfo?(ttaubert)
Depends on: 943668
Fixed by bug 943668.
Assignee: nobody → mhammond
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
Keywords: verifyme
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0		
Mozilla/5.0 (Windows NT 6.1; rv:27.0) Gecko/20100101 Firefox/27.0		

Verified as fixed on Firefox 28 beta 1 (Build ID: 20140205162153).
The window position is now properly restored if the Windows DPI is set to 125%.
Status: RESOLVED → VERIFIED
Keywords: verifyme
This issue is intermittently reproducible with Firefox 28 beta 4 (Build ID: 20140218122424) on Windows 7 64 bit and 2 Windows 8.1 32bit machines. On Windows 7 64 bit is more reproducible: 6 out of 10 tries.

Mozilla/5.0 (Windows NT 6.3; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
You need to log in before you can comment on or make changes to this bug.