30529 - [feature] Implement minimize/maximize/restore

Status: RESOLVED FIXED

(Core :: XUL, defect, P2)

Description

[David Hyatt]

Implement minimize/maximize/restore on nsIWidget and nsIDOMWindow.

Comment 1

[Peter Trudelle]

Mass move of all M20 bugs to M30.

Comment 2

[Peter Trudelle]

Mass move of M20 bugs to M30

Comment 3

[Peter Trudelle]

Mass-moving all M20-M30 XPToolkit bugs to Future

Comment 4

[Pavol Vaskovic]

Hi!

I need this functionality to implement some Mouse Gestures in Mozilla.
Opera has gestures for minimizing, maximizing and restoring window.
We are (optimoz.mozdev.org) mimicing Opera's gesture, so we need this to be
implemented. 

--mondo

Comment 5

[Pavol Vaskovic]

This is feature should be part of 1.0 API. Therefore I ask responsible peolple
to set the Target milestone to something like mozilla1.0 or before.
--mondo

Comment 6

[Brendan Eich [:brendan]]

pali: nsIWidget is not part of the public API, so not subject to freezing for
1.0.  for nsIDOMWindow, try jst (cc'ing him).

Pav, you should find a better assignee if you don't wanna own this bug.

/be

Comment 7

[Johnny Stenback (:jst)]

If we want this in the DOM we do have the option of adding this to the interface
nsIDOMWindowInternal, which won't be frozen any time soon. But do we really want
minimize in the scriptable DOM API? I smell abuse.

Comment 8

[HJ]

Johnny, what if it is limited to chrome uri's only?

Comment 9

[Pavol Vaskovic]

jst: I it just to be added to interface, so that we can use it in MozGest. bug
61928 is request to enable it to the masses, and I don't feel to passionate
about that...
--mondo

Comment 10

[Johnny Stenback (:jst)]

If it's not supposed to be usable by anyone, it should not be added to the
window object, unless there's no other alternative.

Comment 11

[Pavol Vaskovic]

Jst: well, it should manipulate the Window... and functions it should make
accessible through interface are here (lxr search for minimize & maximize - I'm
guessing these are those functions responsible for it...):
http://lxr.mozilla.org/seamonkey/source/widget/public/nsIWidget.h#418
http://lxr.mozilla.org/seamonkey/source/widget/src/os2/nsWindow.cpp#1117
http://lxr.mozilla.org/seamonkey/source/widget/src/windows/nsWindow.cpp#1397
http://lxr.mozilla.org/seamonkey/source/widget/src/xpwidgets/nsBaseWidget.cpp#368
I believe it belongs to window object... But they could be accesible also from
that nsIWidget - I don't care where, I just need to get to them :-)

--mondo

Comment 12

[Pavol Vaskovic]

jst: what is the status of this bug?
--mondo

Comment 13

[Johnny Stenback (:jst)]

I don't see why these methods would need to be accessible to JS, at least not to
web content. If that's the case, then it's not my bug, and I don't know what the
status is. The method you point out is already accessible through nsIWidget
(which is a non-public, non-scriptable, interface), but I'm not convinced we
need to expose such functionality to web content.

Comment 14

[Pavol Vaskovic]

I'm also against exposing this to web content. That's bug 61928 (see blocks
^^^). I want to expose this in nsIWidget for scripting with JS. See bug 76537.
We need it!
--mondo

Comment 15

[Johnny Stenback (:jst)]

Eh, nsIWidget is not, and will probably never be, a scriptable interface, if you
need to do this from JS then we need a different solution.

Comment 16

[Fabian Guisset]

OK I got a patch for this but there's a slight issue: it doesn't work on linux ;-)
I asked hwaara (he's using windows) to test my patch and he says it works for him.
But in my tests on linux (mandrake8.1, KDE2.2.1) it doesn't work. I suspect it's
a bug in the implementation of nsIWidget::SetSizeMode() on Linux rather than a
bug in my code.
Taking, marking 0.9.7. We could check this in as is then fix SetSizeMode().

Comment 17

[Håkan Waara]

I really hope this will NOT be accessable for any malicious web author; it
should probably be chrome-only.

Comment 18

[Johnny Stenback (:jst)]

Once complete, this will indeed not be accessible to web content.

Comment 19

[Fabian Guisset]

since my linux box is dead, here's what needs to be done to implement this. I
hope someone else will pick it up, otherwise it'll have to wait.
1) Wait until jst lands his nsIDOMWindowInternal => nsIDOMChromedWindow changes.
2) Add void maximize(); void minimize(); void restore();
to nsIDOMChromedWindow.idl
3) Implement GlobalWindowImpl::Maximize(), GWI::Minimize(), and GWI::Restore().
Here's what GWI::Maximize() looks like, Minimize and Restore are slightly
modified versions of this function.

NS_IMETHODIMP
GlobalWindowImpl::Maximize()
{
  nsCOMPtr<nsIBaseWindow> treeOwnerAsWin;
  nsresult rv = GetTreeOwner(getter_AddRefs(treeOwnerAsWin));
  
  if (treeOwnerAsWin) {
    nsCOMPtr<nsIWidget> widget;
    rv = treeOwnerAsWin->GetMainWidget(getter_AddRefs(widget));

    if (widget) {
      rv = widget->SetSizeMode(nsSizeMode_Maximized);
    }
  }

  return rv;
}

For GWI::Minimize(), replace nsSizeMode_Maximized with nsSizeMode_Minimized
For GWI::Restore(), replace nsSizeMode_Maximized with nsSizeMode_Normal.

4) add the following to all.js 
pref("capability.policy.default.Window.minimize","noAccess");
pref("capability.policy.default.Window.maximize","noAccess");
pref("capability.policy.default.Window.restore","noAccess");
to make them chrome-only.

Comment 20

[Fabian Guisset]

I'm sorry my linux box is still down so this bug needs a new owner. Pushing to
0.9.8 in the mean time.

Comment 21

[Fabian Guisset]

Attachment: Implement window.maximize(), minimize(), and restore()

After so long... note that it doesn't work on my KDE for some reason, but
hwaara said it works on windows

Comment 22

[Fabian Guisset]

jst, reviews please?

Comment 23

[Brendan Eich [:brendan]]

jst, anyone: are the maximize, minimize, and restore names added here to
nsIDOMWindowInternal going to collide with script uses of the same names?

/be

Comment 24

[Johnny Stenback (:jst)]

Yes, that's why they're going in the not yet officially available
nsIDOMChromeWindow. I have a patch that adds a chrome only nsIDOMChromeWindow
interface that we should combine with this patch. Once that lands we'll finally
get rid of window.title too in content windows :-)

Comment 25

[Fabian Guisset]

Blake, I'm about to implement window.restore(), window.minimize() and
window.maximize() (available from chrome). I remember you needed
window.isMinimized() and window.isMaximized earlier. Do you still need it?
Should I implement window.isMinimized() or window.minimized?

Comment 26

[Fabian Guisset]

Attachment: Add nsIDOMChromeWindow, implement window.minimize(), maximize(), restore()

The new file nsIDOMChromeWindow.idl is not included in the diff, gotta wait
till jst cvs adds it. The patch just cleans up what jst sent me and merges with
the other patch in this bug.

Comment 27

[Brendan Eich [:brendan]]

isMinimized is the better name (minimized is only one letter different, at the
end yet, and doesn't follow the Java-esque isFoo naming convention).

/be

Comment 28

[Johnny Stenback (:jst)]

Or do we want a more generic property for describing the current window state,
i.e. minimized, maximized, ...? Maybe something like window.windowState and some
constants in nsIDOMChromeWindow?

Comment 29

[Blake Ross]

Yeah, I think windowState and constants are the best.

Comment 30

[Johnny Stenback (:jst)]

mozilla/dom/public/idl/base/nsIDOMChromeWindow.idl added to CVS.

Comment 31

[Fabian Guisset]

Attachment: Proposed patch for nsIDOMChromeWindow

Summary of changes:
- Add windowState, constants, minimize(), maximize(), restore() to
nsIDOMChromeWindow
- Implement them in a new class, nsGlobalChromeWindow
- Add the necessary DOMClassInfo stuff
- Modify the docshell to create the correct global scripting object
- SetTitle no longer checks for a chrome context
- Add a helper function, nsGlobalChromeWindow::GetMainWidget()
- Change the creator functions for new script global objects to take a
aIsChrome parameter
- changed window.GetAttention() to window.getAttention() in all the places
where it's used.

Comment 32

[Fabian Guisset]

Attachment: Non-bitrotted patch.

Up-to-date patch. Since this touches many files, I'd appreciate if we could get
rid of it pretty fast, it's a maintance nightmare. Thanks!

Comment 33

[Johnny Stenback (:jst)]

Comment on attachment 67515 [details] [diff] [review]
Non-bitrotted patch.

- Make nsGlobalChromeWindow::GetWindowState() always set the out param to a
default just in case we encounter an unexpected condition.

sr=jst

Comment 34

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 67515 [details] [diff] [review]
Non-bitrotted patch.

r=heikki (pls remember to do the change jst requested)

Comment 35

[Fabian Guisset]

Sure I'd make the change, but since I'm not the one checking in... ;-)
Unless you want a new patch for this change.

Comment 36

[Johnny Stenback (:jst)]

I'll check this in an make those changes...

Comment 37

[Johnny Stenback (:jst)]

Suresh, this will need 3 minor changes in the AIM code in the commercial tree,
window.GetAttention() needs to be replaced with window.getAttention() (search
for GetAttention in the commercial lxr), and a few C++ callers need to make sure
they have a chrome window and QI the window to nsIDOMChromeWindow and then call
GetAttention() on that. Could you apply this patch and make AIM happy again?

Comment 38

[Fabian Guisset]

Triaging my bug list. Marking P2, bug has a reviewed patch, functionality is
blocking a couple of other bugs.
Suresh, can those changes to AIM be done? Should we ask someone else?

Comment 39

[[not reading bugmail]]

is this affecting bug 120155?

Comment 40

[Fabian Guisset]

FIXED, thanks alot to jst and suresh for landing the patch :-) 
Marking FIXED.

Comment 41

[Dan M]

*** Bug 61928 has been marked as a duplicate of this bug. ***

Comment 42

[Georg Maaß]

What has happend to the getAttention method? It is no longer available in the
location bar. I've looked for it after loading a HTML document and after loading
a XML document. It ist present in the JavaScript console, but this window has no
icon to animate, so I can't see an effect, wenn calling it.

Th documentation writes about the old GetAttention to flash the icon in the
system tray. What is the system tray? I'm using Linux with KDE 1.0 and an other
Linux with KDE 2.0, but I don't see any animation.

I beleive to remeber to see the Mozilla M-Icon to anmimate on Windows with 0.9.7
using GetAttention in the location bar like loading something. But I do not get
such an effect with 0.9.7 on Linux.

What exactly should I expect and where can I use getAttention?

Comment 43

[Johnny Stenback (:jst)]

GetAttention() was renamed to getAttention() and it is only available in chrome
windows, not in content windows.