Closed Bug 476705 Opened 11 years ago Closed 11 years ago

toplevel windows and dialogs should be maximized on windows mobile

Categories

(Core :: Widget: Win32, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dougt, Assigned: dougt)

References

Details

(Keywords: fixed1.9.1)

Attachments

(3 files, 1 obsolete file)

Attached patch patch v.1 (obsolete) — Splinter Review
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.
Attachment #360355 - Flags: superreview?(pavlov)
Attachment #360355 - Flags: review?(emaijala)
wouldn't it be easier to specify the maximized stuff in the XUL document rather than hardcoded in C++?
On windows mobile, each application usually has one top level window. We want to ensure that top levels fill up the screen.
Flags: blocking-fennec1.0+
Does this patch do dialogs too?
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.
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.
Attached patch patch v.2Splinter Review
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.
Attachment #360355 - Attachment is obsolete: true
Attachment #360355 - Flags: superreview?(pavlov)
Attachment #360355 - Flags: review?(emaijala)
Attachment #360443 - Flags: review?(pavlov)
Attachment #360533 - Flags: review?(mark.finkle)
Attachment #360533 - Flags: review?(mark.finkle) → review+
Comment on attachment 360533 [details] [diff] [review]
fennec patch to use sizemode

wait to land until the platform fix lands
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).
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
Attachment #360443 - Flags: review?(pavlov) → review+
stuart, any idea about these resize events?
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.
(In reply to comment #11)
> stuart, any idea about these resize events?

Can you file a follow up bug for them?
Filled bug 477653 for the multiple resize event problem as discussed on IRC.
Attachment #360443 - Flags: approval1.9.1?
Could this have caused Bug 477671? This just showed up in the last day or so.
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.
Yes it did - 

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

fixes the problem.
that should be wrapped in an #ifdef WINCE/#endif.  Doug, can you push a quick fix?
yeah, my bad.
Attached patch window state fixSplinter Review
Attachment #361343 - Flags: review?
Attachment #361343 - Flags: review?
Depends on: 477671
needs to land on 1.9.1
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment on attachment 360443 [details] [diff] [review]
patch v.2

a191=beltzner
Attachment #360443 - Flags: approval1.9.1? → approval1.9.1+
this landed on 1.9.1. clearing request flags.
Flags: wanted1.9.1?
You need to log in before you can comment on or make changes to this bug.