Closed
Bug 688929
Opened 13 years ago
Closed 12 years ago
restoring the browser often leaves windows offscreen (on windows)
Categories
(Core :: Widget: Win32, defect)
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: blizzard, Assigned: jh.dev0)
Details
Attachments
(1 file, 1 obsolete file)
3.94 KB,
patch
|
jimm
:
review+
|
Details | Diff | Splinter Review |
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.
Comment 1•13 years ago
|
||
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.
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.
Reporter | ||
Comment 5•13 years ago
|
||
Oh, yeah, I tend to leave windows maximized or minimized and shut down when doing that often.
Comment 6•13 years ago
|
||
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.
Comment 7•13 years ago
|
||
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.
Comment 10•13 years ago
|
||
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.
Comment 11•12 years ago
|
||
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.
Comment 12•12 years ago
|
||
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.
Assignee | ||
Comment 13•12 years ago
|
||
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)
Comment 14•12 years ago
|
||
This is a touchy area of the code so I've pushed this to try for a test run - https://tbpl.mozilla.org/?noignore=0&tree=Try&rev=a90f4d69aa9e
Updated•12 years ago
|
Assignee: nobody → jh.dev0
Comment 15•12 years ago
|
||
Try run for a90f4d69aa9e is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=a90f4d69aa9e Results (out of 49 total builds): success: 43 warnings: 6 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jmathies@mozilla.com-a90f4d69aa9e
Assignee | ||
Comment 16•12 years ago
|
||
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 17•12 years ago
|
||
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+
Keywords: regression → checkin-needed
Updated•12 years ago
|
Component: Session Restore → Widget: Win32
Product: Firefox → Core
QA Contact: session.restore → win32
Comment 19•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ef0a63ddcc8f
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 20•12 years ago
|
||
Thanks for the contribution James!
You need to log in
before you can comment on or make changes to this bug.
Description
•