504499 - Windows CE build always start up with the window maximized, change not persisted

Status: RESOLVED WONTFIX

(Firefox :: General, defect, P2)

Description

[Justin Dolske [:Dolske]]

I've noticed this for a few days, but thought it was just my device/profile. But it's persisted across a reflash, so I'm not sure what's up.

When launching firefox the window is maximized. That's actually not such a bad thing to do (maybe we should as the default!), but the window state does not persist after resizing the window (to a not-maximized state) and exiting the browser.

Comment 1

[Justin Dolske [:Dolske]]

I noticed that when toggling between the maximized and normal states, nsWindow::SetSizeMode was always being called with mode 2 (maximized), when it should toggle between 0 and 2.

The cause seems to be using mSizeMode for the window state, before it has actually been updated...

When nsWindow.cpp processes case WM_WINDOWPOSCHANGED, it creates a nsSizeModeEvent, and initializes it with event.mSizeMode = mSizeMode. The event is dispatched, and eventually received by nsWebShellWindow::HandleEvent, which calls nsWindow::SetSizeMode(). The code there suppresses size mode changes where aMode == mSizeMode. And so it never makes the call to nsBaseWidget::SetSizeMode, where mSizeMode is actually updated. So, we're stuck.

This was regressed by http://hg.mozilla.org/mozilla-central/rev/51ca7c35d98f / bug 484488... Previous to that, the nsSizeModeEvent was just initialized with nsSizeMode_Normal.

Comment 2

[Justin Dolske [:Dolske]]

Attachment: Patch v.hacky

This fixes the problem, but I have no idea if it's correct. The normal Windows code uses ::GetWindowPlacement to figure out the minimized/normal/maximized state, but that's not on CE. I fiddled around for a bit and came up with this, but it's a total hack. Maximized windows don't have the thick (resizable) window border, while normal sized windows do.

Luckily minimizing a window doesn't seem to hit this codepath...

Comment 3

[Justin Dolske [:Dolske]]

Bug 504501 seems unchanged with this hacky patch.

Comment 4

[Doug Turner (:dougt)]

ideally size mode would have been set by the thing minimizing / maximizing the window so that we could rid of the GetWindowPlacement all together.

Comment 5

[Justin Dolske [:Dolske]]

Can things external to the app minimize/maximize the window, though? I thought about tracking when we change the window size, but was wary of WM_WINDOWPOSCHANGED being the only way the app would learn some other utility resized it.

Comment 6

[Justin Dolske [:Dolske]]

robarnold suggests we use WM_SIZE on CE to detect this. The comment in the code about "WM_SIZE is too late" dates back to 2000 (bug 63052), neither of us was really sure why that was a problem.

Also looked at using ::IsIconic() / ::IsZoomed(), but crappily enough only ::IsIconic() is available on CE. O_o

Another hacky idea I found with google was to compare the window size/position with the screen size, and assume the window is maximized when they're equal.

Comment 7

[Justin Dolske [:Dolske]]

Attachment: Patch v.broken (WM_SIZE)

Fiddlesticks. This was the idea, but it doesn't work. We get the WM_SIZE messages, but they're always 0 (unless you minimize, in which case it's 1). It's like the OS itself just doesn't know about maximizing (eg, ::IsZoomed() also unavailable).

Guess we'll have to try a hacky fix?

Comment 9

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

Comment 10

[Justin Dolske [:Dolske]]

Pushed http://hg.mozilla.org/mozilla-central/rev/fd1dc8226760

Comment 11

[Doug Turner (:dougt)]

Attachment: fixes regression on windows mobile

Comment 12

[Justin Dolske [:Dolske]]

Doug: how does Windows Mobile avoid the problem described in comment 1?

Comment 13

[Brad Lassey [:blassey] (use needinfo?)]

reopening due to the regression

Comment 14

[Doug Turner (:dougt)]

@dolske on windows mobile, there is no "normal" window.  we are either maximized -- taking up the fullscreen minus the title bar, or we are fullscreen -- taking up, well, the fullscreen.  Sadly, windows mobile doesn't have GetWindowPlacement so we have to remember what state we were in.

Comment 15

[Justin Dolske [:Dolske]]

Comment on attachment 391145 [details] [diff] [review]
fixes regression on windows mobile

So... The unrelated fix for bug 504499 accidentally reverted the patch I checked in (which had caused WinMo to break), so trunk and 1.9.2 don't need dougt's regression fix because there's no regression now. :)

I think the original fix isn't quire right, anyway. So I'll double check how to fix this properly, and verify that any updated patch works correctly with WinMo too.

Comment 16

[Grae]

I am not sure whether this should be a separate bug.  However, when the browser restarts with a modal Dialog window, the dialog becomes a full sized maximized window.  Can you please advise me?

Comment 17

[Justin Dolske [:Dolske]]

Unassigning, not working on CE bugs any more.

Comment 18

[Doug Turner (:dougt)]

This year we mothballed windows mobile development.  See:

  http://blog.pavlov.net/2010/03/22/stopping-development-for-windows-mobile/

Marking bugs in the windows mobile / windows ce bucket as WONTFIX.