852371 - Restored windows too large after bug 844604 lands

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

After the pref change in bug 844604 hits a user, the restored window sizes are significantly larger than they should be.  The end result is that all windows tend to reach far off the right and bottom of the screen.  The fact they extend past the screen makes grabbing the borders to resize somewhat challenging for most users.

While bug 844604 caused this, I suspect session restore should simply validate sizes at restore time (or if it attempts to, it needs to take this change into account).  Blocking bug 844604 anyway though.

STR in a nightly with bug 844604 having landed.

* Edit the profile's prefs.js and set 'layout.css.devPixelsPerPx' to '1.0' - this simulates Fx before that bug.
* Resize Fx such that it takes up the majority of the screen, and exit Fx.
* Edit prefs.js again, this time setting the pref to "-1.0" - this simulates the default state after the landing of bug 844604.
* Start Fx - the restored Window will be way too large for the screen.  All windows are affected (eg, the "Error Console" will also be sized inappropriately)

Comment 1

[Jonathan Kew [:jfkthame]]

(In reply to Mark Hammond (:markh) from comment #0)
> After the pref change in bug 844604 hits a user, the restored window sizes
> are significantly larger than they should be.  The end result is that all
> windows tend to reach far off the right and bottom of the screen.  The fact
> they extend past the screen makes grabbing the borders to resize somewhat
> challenging for most users.
> 
> While bug 844604 caused this, I suspect session restore should simply
> validate sizes at restore time (or if it attempts to, it needs to take this
> change into account).  Blocking bug 844604 anyway though.

Yup. Validation of the window sizes is broken, for the same reason as bug 824386 and bug 844857 - nsScreenWin doesn't take account of the resolution scaling. I should have a patch for that shortly, then we'll see if there is any further issue here.

Comment 2

[Jonathan Kew [:jfkthame]]

Now that bug 824386 and bug 851952 have landed, is this still a problem? In my local testing of switching between current Firefox release and Nightly, the restored window is now properly constrained to the screen.

(It's still likely that on hi-dpi systems, the window will be restored to a larger size than it was prior to the update; but that makes sense, because the content will now be scaled properly for the resolution, and so a larger window is needed in order to display the same area of the page.)

Mark, if you could confirm whether there's still a problem here with latest Nightly, that would be helpful - thanks.

Comment 3

[Mark Hammond [:markh] [:mhammond]]

I see a problem in a multi-monitor setup.  I have 2 monitors aligned horizontally, both with the same vertical resolution, but the one on the right being smaller horizontally.

When switching between Aurora and Nightly, I can't provoke a problem on the left (ie, the wider) screen.  However, on the right screen, if I setup Aurora such that the window takes most of the monitor, then exit and start nightly, I find the window has been constrained vertically, but extends off the screen horizontally.  It is almost as if the dimensions of the main screen are being used to constrain the size on the second screen.

Also FWIW, if I follow the original STR, I can reproduce the problem on both screens - but I can't provoke the same thing without manually tweaking prefs in a text editor.  Given that, the original STR can probably be considered invalid - but that secondary monitor problem seems something a user might be expected to hit.  I'll leave it to others to decide if this secondary monitor problem is actually a different bug allowing this to be closed as INVALID.

Comment 4

[Jonathan Kew [:jfkthame]]

(In reply to Mark Hammond (:markh) from comment #3)
> I see a problem in a multi-monitor setup.  I have 2 monitors aligned
> horizontally, both with the same vertical resolution, but the one on the
> right being smaller horizontally.
> 
> When switching between Aurora and Nightly, I can't provoke a problem on the
> left (ie, the wider) screen.  However, on the right screen, if I setup
> Aurora such that the window takes most of the monitor, then exit and start
> nightly, I find the window has been constrained vertically, but extends off
> the screen horizontally.  It is almost as if the dimensions of the main
> screen are being used to constrain the size on the second screen.

That seems odd - and suggests this is not completely fixed yet. The patch awaiting review in bug 859266 will touch this area of code again, so let's see what happens once that is fixed.

Comment 5

[Mark Hammond [:markh] [:mhammond]]

FWIW, my "installed" Aurora just updated to 22, and starting it with the normal profile I use just demonstrated the original problem (ie, the restored size was too large in both dimensions).  All my other testing was using my "dev" profile.  I'll try and repro with clean profiles...

Comment 6

[Jonathan Kew [:jfkthame]]

Hmm, I just allowed Aurora to update itself to 22, and sure enough, the post-update restart gave me an oversized window. (On a Windows machine set to 144dpi, though I assume the effect would happen to some extent with any hi-dpi setting.)

But I can't seem to reproduce that by going back to Firefox 20 or 21 and setting a large window size there, then switching back to Aurora with that same profile. It tries to make the window bigger (so as to maintain the same visible content at the new scale), but correctly constrains it to the screen.

So it seems like somehow the initial restart after the version update behaves differently, but it's not clear to me why this would be.

Comment 7

[Jonathan Kew [:jfkthame]]

OK, forget that - I can reproduce by switching between Firefox (20) and Aurora (22). It happens -only- when restoring windows from the previous session; the problem does not happen for the initial browser window (assuming the default about:home start page). So changing the prefs to "Show my windows and tabs from last time" makes it easy to reproduce.

I -don't- see the issue switching between FF20 and Nightly, however, so it looks like an additional patch that's in current Nightly but not Aurora may have fixed this. Let's see if we can pin it down...

Comment 8

[Jonathan Kew [:jfkthame]]

If I switch from Firefox 20 to the tinderbox build of the FIREFOX_AURORA_22_BASE changeset (i.e. http://hg.mozilla.org/mozilla-central/rev/1c070ab0f9db), I do -not- see any problem here. The restored window is properly constrained to the screen.

However, if I switch from Firefox 20 to the very first Aurora build after the merge (i.e. http://hg.mozilla.org/releases/mozilla-aurora/rev/400370bbc9fa), then I -do- see the problem.

It looks as though two things are happening: (1) the dock area is ignored, so the size constraint considers the entire screen rather than the available area excluding the dock; and (2) the window is slightly too large even for that, roughly as though its borders are being ignored and the client area is expanded to the full screen space.

So the problem occurs on Aurora from the very beginning of Aurora-22, even though it did -not- occur on the Nightly-22 from which Aurora was branched.

The only changesets that landed on mozilla-aurora -after- the merge from the (working) mozilla-central changeset 1c070ab0f9db and before that first (broken) Aurora build are:

400370bbc9fa	Bhavana Bajaj — IGNORE BROKEN CHANGESETS Merging in l10n changes NO BUG CLOSED TREE ba=release
b1d6429c549e	Bhavana Bajaj — Disable profiling on Aurora NO BUG CLOSED TREE ba=release
90134e75a6d4	Bhavana Bajaj — Merging in branding changes NO BUG CLOSED TREE ba=release
bc8a6b8e0280	Bhavana Bajaj — removing enable-metro from win32 and win64 builds

So either the behavior is being affected by one of these, or there's some other factor outside the actual source tree that is causing the difference. Investigation continues...

Comment 9

[Jonathan Kew [:jfkthame]]

Using local builds from the aurora tree, I've narrowed this down to

  http://hg.mozilla.org/releases/mozilla-aurora/rev/90134e75a6d4

which is "Merging in branding changes".

With a build from the changeset prior to this, the problem does not occur; with this changeset, it does.

So the unwanted behavior here is somehow being triggered by the branding change from Nightly to Aurora. I'll try to figure out exactly what it is about that change that affects window sizing, as that doesn't seem like an expected side-effect.

Comment 10

[Jonathan Kew [:jfkthame]]

I've pushed two tryserver builds to confirm exactly when this behavior change happens on Aurora:

(1) https://tbpl.mozilla.org/?tree=Try&rev=504c2b733ed2
built from http://hg.mozilla.org/releases/mozilla-aurora/rev/bc8a6b8e0280

(2) https://tbpl.mozilla.org/?tree=Try&rev=b9fe4c987518
built from http://hg.mozilla.org/releases/mozilla-aurora/rev/90134e75a6d4

When I move from Firefox 20 to build (1), on a Windows machine set to 144dpi (150%) with the "Show my windows and tabs from last time" option, the restored window is constrained to the size of the screen.

When I move from Firefox 20 to build (2), the restored window exceeds the screen size, as described earlier.

The only difference between the two builds is the single changeset:
90134e75a6d4	Bhavana Bajaj — Merging in branding changes

It appears that in build (1), although the restoreWindow and restoreHistory functions from SessionStore.jsm are used, the restoreTab function does not get called. In build (2), restoreTab *does* get called, and this is what in turn calls restoreDimensions, which sets the "old" dimensions in CSS px without comparing them to the available screen size; hence, because CSS px are in effect 50% larger now that we're honoring the Windows scale factor, the window ends up too big.

What I don't understand is *why* there's this difference in behavior, purely as a result of "Merging in branding changes". Note that in build (1), even though the restoreTab function (and thence restoreDimensions) is not called, my tabs from the previous (Firefox 20) session *are* being restored.

Maybe someone who actually understands how Session Restore works could explain the difference in behavior? I'm pretty lost in that code...

Comment 11

[Jonathan Kew [:jfkthame]]

Testing confirms that this -is- a pre-existing problem with Session Restore, which fails to ensure that the restored window is actually on-screen; see bug 864107.

Enabling hi-dpi support merely exacerbates this problem by, in effect, reducing the desktop extent for users who have high-pixel-density screens, so that windows may be restored to partially (or in extreme cases, completely) off-screen positions even -without- the user having consciously changed their system configuration.

What I still don't understand, though, is why this -doesn't- occur when migrating from FF20 to Nightly, or indeed to a build of Aurora that does not have the "branding changes" patch. Why does Session Restore behave differently in this case?

Comment 12

[Jonathan Kew [:jfkthame]]

Mark, there's a tryserver build of Aurora with the patch (pending review) in bug 864107; see bug 864107 comment 6.

With this build, I no longer see the issue here when upgrading to Aurora from an older Firefox. If you could also test and see if this resolves the issues you've seen, that would be helpful - thanks.

Comment 13

[Mark Hammond [:markh] [:mhammond]]

Sorry for the delay - I did manage to repro this every time with the instructions above, but can't repro it with that try build - ie, that try build does solve the problem for me.  Thanks!

Comment 14

[Jonathan Kew [:jfkthame]]

This should be fixed by bug 864107, which is in Nightly and has just been uplifted to Aurora.

We should verify the behavior for the FF20/21 -> 22 migration on Win-hidpi systems once the next Aurora build (with bug 864107) is available.

Comment 15

[Jonathan Kew [:jfkthame]]

I believe this is fixed on Nightly and Aurora (by bug 864107); marking qawanted as it would be good to have this confirmed by further testing.

In particular, if you have been using Firefox 20 or 21 on a hi-dpi Windows configuration (i.e. with the display scaling set larger than 100% - it's not important what the real dpi of the hardware is, only how the Windows display scaling option is set), then when you switch to Aurora and restore your session, are the restored browser window(s) constrained to the size of the screen?

Comment 16

[Mihai Morar, (:MihaiMorar)]

I confirm it works as expected on FF 22.b1 on Windows 7 x64:

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20100101 Firefox/22.0(20130514181517)