195963 - Mozilla hides taskbar badly for full screen mode (affects multi monitor setups)

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Brian Rogers]

Currently, for full screen mode, Mozilla hides the taskbar and sizes the window
to the screen's dimensions (the patch for bug 127575 makes this work with
multiple monitors).

The thing is, the taskbar is hidden whether or not the Mozilla window is on the
same screen.  Detecting if they are on the same screen would probably be tricky,
especially since the taskbar can be on any screen, not just the primary screen.

And then there's the fact that there can be other always on top windows docked
to the side of the screen that Mozilla won't know about.  For example, ICQ can
do this.  Another case is built into explorer itself.  Just drag any sort of
container (a folder, My Computer, etc) to the side of the screen and you'll have
something you can right-click and set "Always on top."  Mozilla can't hide these.

IE's fullscreen mode doesn't have any of these problems, so I checked out how
that works.  It simply sets itself as always on top so it doesn't need to know
about any of those windows.  When it loses focus, it removes that attribute so
you can see other windows again.  Mozilla should do the same.

Comment 1

[:deanis74]

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

Comment 2

[jag (Peter Annema)]

I'm so not gonna get to this anytime soon. helpwanted.

Comment 3

[[e-mail]]

Attachment: C program to demonstrate "easy" full-screen (single monitor)

Comment 4

[[e-mail]]

Attachment: Application version. Double-click go switch in/out of full screen.

Comment 5

[:deanis74]

mine

Comment 6

[:deanis74]

Attachment: cvs diff -u widget/src/windows

This works for me.  When you open a new window the taskbar doesn't reappear,
but that happens with the current implementation also and is a separate bug. 
The good thing about this patch is that I can kill the application from Task
Manager and the taskbar reappears.

burpmaster, can you apply this and see if fullscreen works better for you?

Comment 7

[Brian Rogers]

Yeah, with this patch it works how it should.

I tried something myself, though.  I commented out the code in HideAllOSChrome()
and that fixed it too.  Apparantly the window doesn't have to be made topmost to
overlap the taskbar.  Some other property of the window must already be allowing
that.  I wish I gave this information earlier.

Still, there's good reason to set topmost.  Other applications set as topmost
(ICQ, at least) aren't overlapped.  Also, I believe that in older versions of
Windows (95 and possibly 98), the taskbar doesn't hide as easily, so this will
be necessary as long as the taskbar isn't being poked out of existence.

However, I can tell that your patch isn't successfully setting topmost like it's
supposed to, because the ICQ window still isn't being overlapped.  But for the
reason I mentioned earlier, it appeared to work.

Comment 8

[:deanis74]

Yes, I recall having a similar discussion with neil a while back, and I think we
concluded that making a window the size of the screen was all that was needed on
windows me/2000 and above, but wasn't sufficient on 95/98.  I'm curious about
ICQ staying above the window.  Does that happen with IE as well, or does it rise
above ICQ?

Comment 9

[Brian Rogers]

IE rises above the ICQ window.  I verified that IE is setting itself topmost
even though that isn't necessary to just cover the taskbar.

Comment 10

[James Nord]

Seeing this as well (XP).  I'll test the patch there.  
This is bad behaviour, shouldn't this be (requested as) blocking 1.4?

Comment 11

[James Nord]

Tetsed patch on XP.
Solves the problem.

Comment 12

[:deanis74]

Does anyone have 95/98 that thy can test attachment 119432 [details] on?  I need to know
whether the positioning the window to HWND_TOPMOST is superfluous on all OSes.

Comment 13

[[e-mail]]

Dean, I tested Win95 and W2K but I'll test Win98 too if you want.

Comment 14

[:deanis74]

If you could, I wouldn't mind.  I was sure there was some reason why just making
the window really large didn't work on all systems.

Neil, happen to know how IE puts itself on top of ICQ?  Obviously setting it to
HWND_TOPMOST isn't enough?

* idea *
Burpmaster, can you try one thing with my patch?  After:

+                ::SetWindowPos((HWND) widget->GetNativeData(NS_NATIVE_WINDOW),
+                  hWndInsertAfter, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE);

Can you add:

                 if (aTopMost)
                   ::SetForegroundWindow((HWND)
widget->GetNativeData(NS_NATIVE_WINDOW));

You'll have to add that to the source file after you apply the patch, not to the
patch itself.

Comment 15

[[e-mail]]

Interesting - when I full-screen IE it initially appears to cover all windows,
but then the topmost ones reassert themselves. Sadly I don't have Spy++ and IE
on the same machine to see what's going on.

My program does not in fact hide the taskbar on Windows 98 :-( Sigh...

BTW, my attachment fails on NT because I miscompiled it - I compiled without the
CRT so nCmdShow is invalid (my code should use SW_SHOWDEFAULT anyway).

Comment 16

[Brian Rogers]

In response to comment 14: I just tried that (after correcting aTopMost to
aTopmost), and there was no change.
Setting HWND_TOPMOST should be enough.  Considering that I can't get the
fullscreen window to behave in any way like a topmost window, I suspect the code
to set HWND_TOPMOST is failing.

Comment 17

[:deanis74]

I don't know if what we're doing is necessarily a bad thing.  I don't have ICQ
to test with, but I tried setting the Task Manager to topmost and going
fullscreen in IE.  The Task Manager went behind IE, but only for a second.

Regardless, I don't think we should be trying to get in front of all
explicitly-topmost windows.  There's a difference to me between "taking up the
entire screen area" and "in front of all other windows".

Comment 18

[Brian Rogers]

Yeah, it probably isn't a bad thing.  But we need to make sure the taskbar is
covered in Windows 95.

Comment 19

[:deanis74]

According to comment 13, Neil's already tested the patch in Win95.  This part
worries me, though.  "My program does not in fact hide the taskbar on Windows 98
:-( Sigh..."  Neil, does my patch hide the taskbar on Win98?

Comment 20

[Jason M. Abels]

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

Comment 21

[:deanis74]

Neil, is this patch good for you, or should I change it?

Comment 22

[Jo Hermans]

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

Comment 23

[Jo Hermans]

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

Comment 24

[[e-mail]]

Sorry I haven't got back to you, but that's what you get when you forget to CC
yourself on a bug :-)

On Windows 98, the taskbar sometimes hides itself. In particular, if you've got
System Monitor (say) partially covering it then it always hides itself. Also, if
you switch tasks then the taskbar hides itself, but only after the switch... I'm
still trying to investigate why only 98 has the problem.

On Windows 95 the taskbar immediately hides.

On Windows 2000 the taskbar hides itself after no more than one second.

Comment 25

[[e-mail]]

Comment on attachment 122736 [details] [diff] [review]
cvs diff -u widget/src/windows

Actually appears to work exactly the same way as IE works (i.e. intermittently
on Windows 98) even without calling SetTopmostWindow.

Comment 26

[:deanis74]

Comment on attachment 122736 [details] [diff] [review]
cvs diff -u widget/src/windows

You're saying that nsFullScreen::SetTopmostWindow() isn't needed at all?

Comment 27

[[e-mail]]

So it would seem; I just double-checked on W2K as that's all I've got right this
minute, but this is the code from nsFullScreen.cpp that I'm using:

NS_IMPL_ISUPPORTS1(nsFullScreen, nsIFullScreen)

nsFullScreen::nsFullScreen()
{
}
nsFullScreen::~nsFullScreen()
{
}
NS_IMETHODIMP nsFullScreen::HideAllOSChrome()
{
  return NS_OK;
}
NS_IMETHODIMP nsFullScreen::ShowAllOSChrome()
{
  return NS_OK;
}
NS_IMETHODIMP nsFullScreen::GetChromeItems(nsISimpleEnumerator **aResult)
{
  return NS_OK;
}

Comment 28

[Matthias Versen [:Matti]]

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

Comment 29

[Olivier Cahagne]

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

Comment 30

[:deanis74]

Neil: that seems consistent with your testing and mine.  I'd r= a patch like that.

Comment 31

[[e-mail]]

Attachment: What I use

Dean, this works for me as well as IE does, that is to say, perfectly on
Windows 95, intermittently on Windows 98 and after a short delay on Windows
2000.

I might investigate an alternative approach which is to hide the chrome,
maximize the window, and restore the chrome, if you think that's worthwhile.

Comment 32

[:deanis74]

Comment on attachment 136669 [details] [diff] [review]
What I use

So this basically rips out nsIFullScreen support on Windows and lets the OS
handle hiding the os chrome.  I'm good with that.  I like this approach better
than manually hiding and showing the taskbar, etc., since this way we don't
have to worry about what happens after a crash.

Comment 33

[[e-mail]]

Comment on attachment 136669 [details] [diff] [review]
What I use

jst, I only picked you because you're logged as giving joint sr on
nsIFullScreen.idl and I want to know if (assuming you agree with dean) we
should also rip out nsIFullScreen as this patch removes the only instance of
it.

Comment 34

[Johnny Stenback (:jst)]

/me can sr but wants to hear danm's word on this first (dan, you had your
fingers in on this code, right?)

Comment 35

[Dan M]

I've been in the widget code a fair bit but this is my first brush with the
fullscreen code. Still, thanks for cc:ing me. I'm all for the simpler approach
though merely setting yourself topmost would seem to have some obvious flaws
(fighting with other topmost windows, as in comment 15). But everybody seems to
feel this is still a better approach, and I can go along.

However, gotta say, I'm unhappy with making the widget library dependent on
windowwatcher, with ViewManager, PresShell et.al. That seems like some sort of
kitten-swatting travesty to me. Widget code asking for windowwatcher help
because it can't find its own widget with both hands seems backwards. I'd prefer
to see the SetTopmost code moved into nsWindow.cpp, probably at
MakeFullScreen(). It'll have direct access to the correct HWND there, won't it?

Comment 36

[:deanis74]

Dan, me thinks you looked at the wrong patch.  Please check out attachment
136669 [details] [diff] [review].  And this code was originally written by Ben, so I'll CC: him.

Comment 37

[Dan M]

So I did. Gee, that was dumb. 136669, is that finished? Sure, removing the
FullScreen service gives me no qualms, if that's what you want to do.

Comment 38

[Johnny Stenback (:jst)]

So would someone outline how full-size windows work w/o this code then?

Comment 39

[[e-mail]]

Basically, setting your client area to the screen size should trigger
full-screen mode for all appbars, including Explorer's taskbar. (Actually in
Windows 95 only the window needs to be the screen size.)

Comment 40

[Johnny Stenback (:jst)]

Comment on attachment 136669 [details] [diff] [review]
What I use

Ok then, sr=jst.

Please do the proper cleanup when checking this in, i.e. cvs rm the files that
are no longer needed (unless other platforms depend on them or somesuch).

Comment 41

[[e-mail]]

Note to self: cvs rm mozilla/widget/src/windows/nsFullScreen.*

Comment 42

[[e-mail]]

Fix checked in.

Comment 43

[Boris Zbarsky [:bzbarsky]]

What about removing nsIFullScreen?