Closed Bug 688929 Opened 11 years ago Closed 10 years ago

restoring the browser often leaves windows offscreen (on windows)


(Core :: Widget: Win32, defect)

7 Branch
Windows 7
Not set





(Reporter: blizzard, Assigned: jh.dev0)



(1 file, 1 obsolete file)

This bug is back, but I can't find the original.

Running the 7 betas here.  It's very often on restart that a two or three of the five windows that I have open will end up offscreen.  I have to go into my sessionstore.js file and edit it by hand to change the screenX and screenY values from -32000 to 0 so that I can see them again.
cc'ing Neil and Paul for input on how window positions could be getting scrambled in the restart-and-restore cycle.
I am also having this bug on the release version 6.0.2, so this is not only an issue of the 7 Branch. As I cannot remember any occurrence of this bug before 6.0.2, I assume that the regression happened between 6.0.1 and 6.0.2.
I can pretty much guarantee it wasn't caused by 6.0.2.
Ok, that was just an assumption. Could also be earlier.

Seems to be a problem with minimized windows.
Original bug could be bug 520178 or, even earlier, bug 502713.
Oh, yeah, I tend to leave windows maximized or minimized and shut down when doing that often.
I tried minimizing and closing several windows via taskbar and then restarting the browser but I can't reproduce.  Does anyone have steps to consistently reproduce?  I do notice that only 3 of my 5 windows get restored though.  Results are the same for me on Aurora and Nightly.  Haven't tried earlier versions.
Also using Windows 7 x64
I can confirm Windows XP (x86) is affected, having seen this with Aurora 8 and 9.

Most recently I had a window's properties set to:

"width": 160,
"height": 34,
"screenX": -32000,
"screenY": -32000,

Given the screen placement, that 160 pixels is the width of a taskbar entry/group if there is no "crowding" (at least on this system, I measured), and 34 pixels is the minimum height an Aurora window can be, looks like minimization is involved. I'll see if I can get STR.
This may be triggered by hibernating Windows with a browser window maximized. Now that I have something to test I'll try to confirm exact STR, hopefully this weekend.
That sounds reasonable. I often hibernate my PC, but until now I did not think of the possibility that this could have something to do with the messed-up windows.
I've found a way to reproduce this bug. It has nothing to do with hibernating Windows. Tested with Firefox 11 on new profiles on both XP x86 and Windows 7 x64 (with window grouping off, so each Firefox window has its own taskbar entry).

1. Open two windows and minimize the second window.
2. Restore window 2 by clicking on it in the taskbar and immediately click it again so it re-minimizes. (have to re-minimize really fast)
3. File/exit from window 1

sessionstore.js will now contain -32000/-32000 for position and 160x24 (or whatever your taskbar icon dimensions are) for window 2 and restoring previous session will result in the offscreen window.

If during step 2 you Restore window 2, then wait a second, then re-minimize it the sessionstore.js will contain the correct dimensions/position for window 2.
Nice work, J S. It's reproducible here here on Windows 7.

In step 2, to make sure you are fast enough, try clicking quickly 3 times on the minimized taskbar window entry.
Attached patch Preliminary patch (obsolete) — Splinter Review
I think this bug belongs to the Widget: Win32 component and is related to bug 665249, bug 646946 and bug 739082

The patch fixes this bug and the others mentioned. I'm including my notes on the bug for reference.

My notes:
The root cause is the NS_SIZEMODE dispatch in WM_WINDOWPOSCHANGING

The flow is something like this:
WM_WINDOWPOSCHANGING -> NS_SIZEMODE -> nsWindow::SetSizeMode (sets mSizeMode to new mode) -> dispatching NS_ACTIVATE -> nsXULWindow::SavePersistentAttributes (size/position/sizemode)

WM_WINDOWPOSCHANGED -> NS_SIZEMODE -> nsWindow::SetSizeMode (aMode == mSizeMode, returns) -> nsWindow::OnResize -> NS_SIZE

When SavePersistentAttributes is called too early (during WM_WINDOWPOSCHANGING, before WM_WINDOWPOSCHANGED), it's getting the coordinates of the window pre-restore.

The reason delaying a re-minimize works is because WM_WINDOWPOSCHANGED -> nsWindow::OnResize is dispatching an NS_SIZE event, which calls nsXULWindow::SavePersistentAttributes again after a 500ms timer fires. If the window is re-minimized (or re-maximized) too fast before that 500ms timer fires, then the persistent attributes are stuck with the bad coordinates.

Blocking the dispatch of NS_ACTIVATE in nsWindow::SetSizeMode on a window restore during WM_WINDOWPOSCHANGING will prevent saving the bad attributes,
but the call must be deferred to the next SetSizeMode that originates from WM_WINDOWPOSCHANGED so the attributes are properly updated. 

This is especially important when going from a maximized to normal window because the sizemode attribute has to be updated before OnResize is called or javascript will not see the correct sizemode during the resize event and won't update css (titlebar tabs).
Attachment #612097 - Flags: review?(jmathies)
This is a touchy area of the code so I've pushed this to try for a test run -
Assignee: nobody → jh.dev0
Try run for a90f4d69aa9e is complete.
Detailed breakdown of the results available here:
Results (out of 49 total builds):
    success: 43
    warnings: 6
Builds (or logs if builds failed) available at:
Attached patch patch v2Splinter Review
I've cleaned the patch up a bit (removed ugly bool flag/redundant code). Can I safely assume the two oranges in the windows debug build from try were unrelated to the patch?
Attachment #612097 - Attachment is obsolete: true
Attachment #612097 - Flags: review?(jmathies)
Attachment #620908 - Flags: review?(jmathies)
Comment on attachment 620908 [details] [diff] [review]
patch v2

Yeah it looks ok. Ran with this for a bit, didn't see any issues.
Attachment #620908 - Flags: review?(jmathies) → review+
Component: Session Restore → Widget: Win32
Product: Firefox → Core
QA Contact: session.restore → win32
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla15
Closed: 10 years ago
Resolution: --- → FIXED
Thanks for the contribution James!
You need to log in before you can comment on or make changes to this bug.