476705 - toplevel windows and dialogs should be maximized on windows mobile

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Doug Turner (:dougt)]

Attachment: patch v.1

Although, it might not be a long term solution (as there might be a need to create top level windows on windows mobile that are smaller than the full size of the display), having all top level windows be maximized make Fennec much more usable.

Comment 1

[Stuart Parmenter]

wouldn't it be easier to specify the maximized stuff in the XUL document rather than hardcoded in C++?

Comment 2

[Doug Turner (:dougt)]

On windows mobile, each application usually has one top level window. We want to ensure that top levels fill up the screen.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Does this patch do dialogs too?

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Also, Maemo forces windows to be maximized too, right? Maybe we should be using a better, more generic flag to control this kind of behavior.

Comment 5

[Doug Turner (:dougt)]

mfinkle, new patch coming up.  this is, right now, windows specific.

the approach i was thinking was to just make the sizemode=maximized attribute on xulwindow work on windows mobile.

Comment 6

[Doug Turner (:dougt)]

Attachment: patch v.2

This patch is similar to the last, it just uses a smaller hammer.

1) any time we call ShowWindow, we also call SetForeground window.  This is done so that our top most window is forced into the foreground when it is activated.  It might be that we only have to do this once, but it isn't clear if ShowWindow will always bring the window to the foreground when required.

2) Dropped the SPI_SETWORKAREA stuff.  It isn't required; it doesn't do anything

3) When we handle the sizemode messages, we force the mode to always be maximized.  This is required because Windows Mobile doesn't have GetWindowPlacement, and it really can't accuractly be implemented in terms of WM_SIZE wparam messages (as it turns out, it looks like we need the "current sizemode" before it is available via the WM_SIZE event.

The result of this change is if you have a xul window, with the attribute sizemode set to "maximized", you will get a maximized window  (full screen).  If you instead specify a height and width, you will get that.  However, on windows mobile, if you start out as a "sizemode=maximized" window and you will not be able to resize or reposition that window.

Comment 7

[Doug Turner (:dougt)]

Attachment: fennec patch to use sizemode

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 360533 [details] [diff] [review]
fennec patch to use sizemode

wait to land until the platform fix lands

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Looks like the fennec patch causes two extra resizes on the n810:
without patch:
resize: 720x420

with patch:
resize: 1025x800
resize: 1025x800
resize: 720x420

I don't know where the 1025x800 is coming from (or why there are two events fired).

Comment 10

[Doug Turner (:dougt)]

i see a similar.  3 resize events on windows mobile when setting the sizemode to maximized.

resize 1024 799
resize 478 772
resize 478 772

Comment 11

[Doug Turner (:dougt)]

stuart, any idea about these resize events?

Comment 12

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

IMO setting sizemode to maximized is the right thing to do. As I understand it, setting width and height are a workaround to avoid the extra resize events.

Given that it will cause a Ts regression for the n810, can we #ifdef around it and have the width and height hard coded for NS_HILDON and sizemode maximized for everything else?

The m-c patch should be ready to land regardless of what we decide to do with mobile-browser.

Comment 13

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

(In reply to comment #11)
> stuart, any idea about these resize events?

Can you file a follow up bug for them?

Comment 14

[Doug Turner (:dougt)]

patch 2:

http://hg.mozilla.org/mozilla-central/rev/c89168932753

Comment 15

[Doug Turner (:dougt)]

mobile-browser:
http://hg.mozilla.org/mobile-browser/rev/1328d52ef693

Comment 16

[Doug Turner (:dougt)]

Filled bug 477653 for the multiple resize event problem as discussed on IRC.

Comment 17

[Jim Mathies [:jimm]]

Could this have caused Bug 477671? This just showed up in the last day or so.

Comment 18

[Doug Turner (:dougt)]

i don't think so.

i just pushed these patches this morning.  The widget change should be WINCE only, and the other patch should only be for fennec.

Comment 19

[Jim Mathies [:jimm]]

Yes it did - 

  if (mWindowType == eWindowType_dialog || mWindowType == eWindowType_toplevel) {
    //aMode = nsSizeMode_Maximized;
  }

fixes the problem.

Comment 20

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

that should be wrapped in an #ifdef WINCE/#endif.  Doug, can you push a quick fix?

Comment 21

[Doug Turner (:dougt)]

yeah, my bad.

Comment 22

[Jim Mathies [:jimm]]

Attachment: window state fix

Comment 23

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/d5f1c222aa28

thanks jimm.

Comment 24

[Doug Turner (:dougt)]

needs to land on 1.9.1

Comment 25

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 360443 [details] [diff] [review]
patch v.2

a191=beltzner

Comment 26

[Doug Turner (:dougt)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/13fef0ff49fb
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a1098ad55999

Comment 27

[Doug Turner (:dougt)]

this landed on 1.9.1. clearing request flags.