Open Bug 321595 Opened 15 years ago Updated 4 months ago

window.maximize() lays the window over the taskbar when hidechrome=true

Categories

(Core :: Widget, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

ASSIGNED

People

(Reporter: aaiyer, Assigned: bent.mozilla)

References

Details

Attachments

(2 files, 2 obsolete files)

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322)
Build Identifier: 

When hidechrome is set and the window is maximized with window.maximize() it lays the window over the taskbar. This is fine for the fullscreen feature for firefox, but there needs to be a way to do this without eating up the taskbar for a XUL window.

Reproducible: Always

Steps to Reproduce:
Attached file testcase
what version are you using?
SeaMonkey 1.5a;Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051226 Mozilla/1.0
did you open it using -chrome ? window.maximize is not there when an XUL is opened in just a browser's area.
Handles WM_GETMINMAXINFO events for hidechrome windows... Fixes the maximization problem...
Attached file patch (obsolete) —
Handles WM_GETMINMAXINFO events for hidechrome windows... Fixes the maximization problem...
Attachment #215743 - Attachment is obsolete: true
That patch attachment just looks like binary garbage to me. Can you attach a real patch?

Don't forget to ask for review. For this we'll probably want r from emaijala, sr from me (roc@ocallahan.org)
Attachment #215744 - Flags: superreview?(roc)
Attachment #215744 - Flags: review?(emaijala)
Attachment #215744 - Flags: review?(emaijala)
Attachment #215744 - Attachment is patch: false
Attachment #215744 - Attachment mime type: text/plain → application/zip
Attachment #215744 - Flags: review?(emaijala)
it was a zip file, i set the mime type for it.. :)
Can you attach it as a CVS diff? I assume it's not very big
Attached patch revised patchSplinter Review
revised patch to the current cvs tree. also fixes the problem where the window size is not being properly resized when the size of the work area area changes.
Attachment #215744 - Attachment is obsolete: true
Attachment #215991 - Flags: superreview?(roc)
Attachment #215991 - Flags: review?(emaijala)
Attachment #215744 - Flags: superreview?(roc)
Attachment #215744 - Flags: review?(emaijala)
+      if (mIsHideChrome) {
+        // When the window's chrome is hidden and the work area changes, 
+        // the window is not invalidated as it is supposed to

Why is that?

If the taskbar changes from shown to hidden, does Windows call WM_GETMINMAXINFO on maximised windows and then resize them automatically?
WM_GETMINMAXINFO is called everytime the window is resized or maximized to find out the maximum and minimum size of the window and the area in which the window should be restricted within when maximized (i.e. the client area in this case).

But, windows does not invalidate the window as it is supposed to when the client area changes. Even though WM_GETMINMAXINFO gets called, the window is not resized to match the new client area. This is usually done for a normal window by windows itself if WS_CAPTION and its aliases are set, but windows does not handle these events fully for a chromeless window as these windows were traditionally only used for screensavers.
One way to see how the window is not correctly resized is by moving the task bar around the screen (i.e. from bottom to left to top and so on..) without the invalidate in WM_DISPLAYCHANGE.
You're saying that the call to Invalidate() causes the window to be resized? How does that work?
Yes, the call to invalidate redraws the windows to the correct size when the window is maximized.. The invalidate is only needed when the window is maximized, but I didnt do a check to see if the window was maximized as that would be too expensive and as people would not be changing the client size all the time I thought this would be a satisfactory tradeoff.
This happens as Invalidate will call UpdateWindow, which will see if the window is maximized and then recalculate the window's client area from MAXMININFO and resize the window to the right size. 
Thats true, but most of the behaviour of a borderless window is pretty undefined and not documented well. This is similar to a techinque I used in another project to aleviate the problem of the window not being resized properly while being maximized and the client area changes. I will try to find a much more detailed explaination for this, but for now I can offer nothing more than the fact that an UpdateWindow fixes this maximized+resize problem .
Hey Anand... the reason this works is that when a window's client area is invalidated (InvalidateRect is called) and a UpdateWindow is called, a WM_PAINT is triggered. Then the rcPaint RECT in the PAINTSTRUCT returned by BeginPaint will have the correct size of what the maximized window should be, thus repainting the window to the right size. Hope this helps.

I also think this would break the fullscreen (F11) facility in firefox, maybe we need a hidechronme="nofullscreen" type tag?
thanks joe... i think most current applications that depend on hidechrome would expect it to be either true or false, maybe we should have a additional tag fullscreen for a xul window? I think I will implement that as a part of this patch...
i think the minheight and minwidth problem with xul window can also be (bug 291331) can also be fixed for windows as a result of handling WM_GETMINMAXINFO, initial experimentation shows that resizer element behaves a little wierd because of this.
If in WM_DISPLAYCHANGE, instead of calling UpdateWindow we call SPI_GETWORKAREA and use SetWindowPos to set our size to the workarea, does everything work?
Comment on attachment 215991 [details] [diff] [review]
revised patch

I'm postponing review until comment #23 is answered.
Attachment #215991 - Flags: superreview?(roc)
Attachment #215991 - Flags: review?(emaijala)
Status: UNCONFIRMED → NEW
Ever confirmed: true
I'm going to fix this as part of the fix for bug 357725. Ere and roc, I'd welcome your review on that approach.
Assignee: general → bent.mozilla
Status: NEW → ASSIGNED
I still get this bug in the latest 1.9 CVS, will the patch in Comment #11 work?
(In reply to comment #27)
> I still get this bug in the latest 1.9 CVS, will the patch in Comment #11 work?
> 

So yes, the patch of Comment #11 is working, not as-is, but changing the lines manually will do the trick.
I'm still getting this bug with XULRunner build 1.9.0.4pre - 2008100509.

However, the runtime that comes with Songbird 0.7 (reported as 1.9.0.1 2008072921) does proper window maximisation?!
QA Contact: general
Ping for this
It's seems nobody tracking down this issue now.
I propose add a new value to sizemode[https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Attribute/sizemode], so it's possible value getting to be
fullscreen
maximized
normal

And also add a new method fullscreen to window, so window have the following three method
window.fullscreen
window.maximzie
window.restore

Mass-removing myself from cc; search for 12b9dfe4-ece3-40dc-8d23-60e179f64ac1 or any reasonable part thereof, to mass-delete these notifications (and sorry!)

You need to log in before you can comment on or make changes to this bug.