Sizes of newly opened unmaximized windows aren't remembered and are always very small after landing patches from bug #1446264

VERIFIED FIXED in Firefox 61

Status

()

defect
--
major
VERIFIED FIXED
Last year
2 months ago

People

(Reporter: Virtual, Assigned: xidorn)

Tracking

({nightly-community, regression})

61 Branch
mozilla61
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox59 unaffected, firefox60 unaffected, firefox61 verified)

Details

Attachments

(3 attachments)

STR:
1. Open Firefox
2. Open new Firefox window
3. Unmaximize it
and see that this new Firefox window has very tiny dimensions and doesn't remember its unmaximized size set from before
Caused by:
bug #1446264

Regression range pushlog:
https://hg.mozilla.org/integration/autoland/json-pushes?changeset=6dbe66440bd2b872776e22d1a2cbd32755c81a2c&full=1

mozregression-gui log:
> 2018-04-17T12:12:40: INFO : Narrowed inbound regression window from [a75367f4, 422ce7e8] (3 builds) to [a75367f4, 6dbe6644] (2 builds) (~1 steps left)
> 2018-04-17T12:12:40: DEBUG : Starting merge handling...
> 2018-04-17T12:12:40: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=6dbe66440bd2b872776e22d1a2cbd32755c81a2c&full=1
> 2018-04-17T12:12:41: DEBUG : Found commit message:
> Bug 1446264 part 5 - Add test for this bug. r=bz
> 
> MozReview-Commit-ID: DS2ucDd0Met
> 
> 2018-04-17T12:12:41: INFO : The bisection is done.
> 2018-04-17T12:12:41: INFO : Stopped
Blocks: 1446264
Has Regression Range: --- → yes
Flags: needinfo?(xidorn+moz)
Summary: Mozilla Firefox don't remember unmaximized sizes of newly opened windows which always starting very small → Sizes of newly opened unmaximized windows aren't remembered and are always very small
Summary: Sizes of newly opened unmaximized windows aren't remembered and are always very small → Sizes of newly opened unmaximized windows aren't remembered and are always very small after landing patches from bug #1446264
What's your operating system? What's your screen resolution?
FWIW, I cannot reproduce the issue from the STR in comment 0 on Windows 10.

There is a known issue similar to this on Linux which is bug 1449166. That's unrelated to the new window, though. If you unmaximize the original window, it would also be very small.
Posted video screencast.mp4
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #2)
> What's your operating system?
My system is Windows 7 (64-bit) with latest security patches, as stated in "Platform: x86_64 Windows 7"

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #2)
What's your screen resolution?
My screen resolution is 1680x1050.

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #3)
> FWIW, I cannot reproduce the issue from the STR in comment 0 on Windows 10.
I'm attaching screenshot, so you can see how this issue looks on my end. 

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #3)
> There is a known issue similar to this on Linux which is bug 1449166. That's
> unrelated to the new window, though. If you unmaximize the original window,
> it would also be very small.

Looks similar, but probably it's not about this specific issue.
In your screencast, what if you restore the original window rather than the new window?
I mean, what happens if you restore the original window?
Posted video screencast2.mp4
Depends, please see new screencast. I probably tested all needed steps.
Looks like only size of first initial window is remembered, but sizes of all new created windows are not remembered.
Component: Untriaged → DOM
Product: Firefox → Core
Ok, I managed to reproduce this on Windows 7 (but not Windows 10).

I suspect this is a widget issue that if a window is opened in maximized, its original size is not forgotten by the system. The reason that the original window is not affected is probably due to browser.startup.blankWindow. When I switched off that pref, if the original window is opened maximized, it would be restored to a small size as well.

It is not clear to me why it appears on Windows 7 but not Windows 10. (I don't have dev environment on Windows 7 so it could become quite tricky to track down this issue...)
Also, the real regression is probably not bug 1446264 but bug 1439875, because the latter introduced a regression that window would not be opened maximized, which was fixed by the former.
Component: DOM → Widget: Win32
Flags: needinfo?(xidorn+moz)
Flags: needinfo?(xidorn+moz)
The width and height property are correct on the new window, so it isn't an issue related to persisting (and thus not an issue related to DOM or XUL).

I would probably need to do some local debug build and install them in the win7 vm to test :/
If you need some logs from debug build, I can help and probably do this. Just some information will be needed, what to do and how to do that.
So I setup a debug environment on a Win7 VM, and managed to reproduce this bug.

As far as I can see, we do invoke the system API `SetWindowPos` for the restore size before we maximizing the window, and the system acknowledge this change with a `WM_WINDOWPOSCHANGED` message, and at that point, we can get the correct restore size from the system API `GetWindowRect` and `GetClientRect`. So it seems that the system somehow loses the track of restore size.

But there is one interesting observation that, after the message triggered by resizing, there is a following message with `SWP_FRAMECHANGED` set, triggered by `SetNonClientMargins` (from syncing "chromemargin" attribute). When handling that message, `GetWindowPlacement` returns `SW_SHOWMAXIMIZED` in `pl.showCmd`, but the system shouldn't have known that the window will be maximized. The size returned from `GetClientRect` later in the handler is still the restored size, though.
The following is a log with WINSTATE_DEBUG_OUTPUT. In this log, the second OnWindowPosChanged is invoked from the Resize method, and everything seems to be setup correctly there (and I also confirmed that the result of GetWindowPlacement has the right size).

The third OnWindowPosChanged is invoked by SetNonClientMargins, and in that, given the information returned from GetWindowPlacement, system has known that the window is maximized, and the normal rect has already been wrong.

The fourth one is triggered by Show method, which opens the window in maximized state. I think that works as expected.

So I guess the question is, between the second and the third message, what tells the system that the window is maximized. That can probably provide some clue on why the normal rect is messed up.


[3812:Main Thread]: I/Widget *** OnWindowPosChanged: [  top] 
[3812:Main Thread]: I/Widget WINDOWPOS flags:
[3812:Main Thread]: I/Widget SWP_FRAMECHANGED 
[3812:Main Thread]: I/Widget SWP_NOSIZE 
[3812:Main Thread]: I/Widget SWP_NOZORDER 
[3812:Main Thread]: I/Widget SWP_NOACTIVATE 
[3812:Main Thread]: I/Widget 
[3812:Main Thread]: I/Widget *** mSizeMode: nsSizeMode_Normal

[3812:Main Thread]: I/Widget *** OnWindowPosChanged: [  top] 
[3812:Main Thread]: I/Widget WINDOWPOS flags:
[3812:Main Thread]: I/Widget SWP_NOZORDER 
[3812:Main Thread]: I/Widget SWP_NOACTIVATE 
[3812:Main Thread]: I/Widget 
[3812:Main Thread]: I/Widget *** Resize window: 0 x 0 x 1452 x 990
[3812:Main Thread]: I/Widget glass margins: left:2 top:2 right:2 bottom:2

[3812:Main Thread]: I/Widget *** OnWindowPosChanged: [  top] 
[3812:Main Thread]: I/Widget WINDOWPOS flags:
[3812:Main Thread]: I/Widget SWP_FRAMECHANGED 
[3812:Main Thread]: I/Widget SWP_NOSIZE 
[3812:Main Thread]: I/Widget SWP_NOZORDER 
[3812:Main Thread]: I/Widget SWP_NOACTIVATE 
[3812:Main Thread]: I/Widget 
[3812:Main Thread]: I/Widget *** mSizeMode: nsSizeMode_Maximized
[3812:Main Thread]: I/Widget *** SetFocus: [  top] raise=0

[3812:Main Thread]: I/Widget *** OnWindowPosChanged: [  top] 
[3812:Main Thread]: I/Widget WINDOWPOS flags:
[3812:Main Thread]: I/Widget SWP_FRAMECHANGED 
[3812:Main Thread]: I/Widget SWP_SHOWWINDOW 
[3812:Main Thread]: I/Widget SWP_NOZORDER 
[3812:Main Thread]: I/Widget 
[3812:Main Thread]: I/Widget *** mSizeMode: nsSizeMode_Maximized
[3812:Main Thread]: I/Widget *** Resize window: -8 x -8 x 1936 x 1079
The "glass margins" log gave me some hint to try, and it seems that if I disable aero theme (by switching to Windows Classic or Windows 7 Basic theme), this issue disappears.

So it must be code related to glass border causing this issue...
I have a patch which seems to work...
Assignee: nobody → xidorn+moz
Flags: needinfo?(xidorn+moz)
And I'm pretty sure the test I added in bug 1446264 has covered this case: https://searchfox.org/mozilla-central/rev/59a9a86553e9bfd9277202748ff791fd9bc0713b/toolkit/content/tests/chrome/test_maximized_persist.xul#80-89

It doesn't fail because the Windows 7 instance we run for testing doesn't enable Aero, and thus the problematic code path here is not triggered.
Comment on attachment 8969167 [details]
Bug 1454589 - Don't change window style according to sizemode if the window hasn't be shown.

https://reviewboard.mozilla.org/r/237898/#review243804

Looks ok to me. Please keep an eye out for regression bugs.
Attachment #8969167 - Flags: review?(jmathies) → review+
Comment on attachment 8969167 [details]
Bug 1454589 - Don't change window style according to sizemode if the window hasn't be shown.

https://reviewboard.mozilla.org/r/237898/#review243810
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/80ada5ae02fe
Don't change window style according to sizemode if the window hasn't be shown. r=jimm
https://hg.mozilla.org/mozilla-central/rev/80ada5ae02fe
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
I'm confirming that bug is fixed, starting in Mozilla Firefox Nightly 61.0a1 (2018-04-20), so I'm marking this bug as VERIFIED.

Thank you very much! \o/
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.