223545 - Mac OS X 10.3's expose feature reveals "hidden" window

Status: VERIFIED FIXED

(SeaMonkey :: General, defect)

Description

[Matt McHenry]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.5) Gecko/20031007
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.5) Gecko/20031007

Version 10.3 of Mac OS X (aka "Panther") contains a feature called "expose" that
shrinks and tiles all windows, allowing the user to choose another window to
bring to the foreground.  When using this feature with Mozilla, an extra window
labelled "hidden" is revealed.  Clicking this window to bring it to the
foreground does nothing, though.

Reproducible: Always

Steps to Reproduce:
1. Start Mozilla
2. Press F10 to tile all application windows


Actual Results:  
The additional "hidden" window is shown.

Expected Results:  
Only the user-visible mozilla windows should have been displayed.

Comment 1

[Matt McHenry]

Attachment: screenshot including the extra "hidden" window

Comment 2

[louis bennett]

see also bug 150028

Comment 3

[Dan M]

Confounded helpful operating systems. I don't suppose a
kEventWindowBoundsChanging handler would easily fix all these not-so-hidden
window problems...

Bumping severity. If the hidden window is visible, the user can close it, and
that brings Mozilla down.

Comment 4

[Mike Pinkerton (not reading bugmail)]

bwahahahahahahahaahahahahahaha

Comment 5

[Matt McHenry]

I actually can't figure out how to close the window.  As I said in the initial
description, when I actually try to make the window active, it vanishes and I
get one of the real mozilla windows instead.  So I think this might be just a
cosmetic problem ...

Comment 6

[Jo Hermans]

I would have expected that when the kWindowInWindowMenuAttribute flag isn't set
(which removes the window from the windowmenu), the OS would also make sure that
the window name doesn't appear on Exposé or in the dock (bug 150028).

Comment 7

[cyangarden]

I would like to know what this hidden window is for.

Comment 8

[Simon Fraser [no longer active]]

The hidden window owns the menu bar that displays when no other windows are open.

Comment 9

[Brian Mastenbrook]

Isn't the easiest solution to make the window a palette instead of a full window? Those windows 
simply fade to nil during expose, and can't be minimized with option-click either.

Comment 10

[Mike Connor [:mconnor]]

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

Comment 11

[Simon Fraser [no longer active]]

There's a hacky solution here:
<http://cocoa.mamasam.com/COCOADEV/2004/02/1/83135.php>

Comment 12

[Creighton Fox]

This occurs in the the new Firefox 0.8 as well.

Comment 13

[Greg Lynn]

Is there going to be a solution to this anytime soon?  When I first discovered it, I thought it was 
someone who had managed to sort out a way to bypass the popup blocker.  What about a 1 pixel 
window?
(In reply to comment #11)
> There's a hacky solution here:
> <http://cocoa.mamasam.com/COCOADEV/2004/02/1/83135.php>

Comment 14

[David Johnston]

This bug is only reproducible on a single display, the "hidden" window can't be
seen on multiple monitors.

Comment 15

[Marek Stępień [:marcoos, inactive]]

There are reports from Thunderbird users about this hidden window (on the Polish
Mac users newsgroup, pl.comp.sys.macintosh, Message-ID: c8im6f$un6$[e-mail])

Comment 16

[weijia]

This bug is still there on the new 0.9 release.  Anyone have any new information
on this?

Comment 17

[timeless]

[e-mail]: generally people post new information. given comment
11, why don't you just write a patch?

(i'll probably write a patch for cocoa, so please don't complain to me.)

Comment 18

[weijia]

(In reply to comment #17)
> [e-mail]: generally people post new information. given comment
> 11, why don't you just write a patch?
> 
> (i'll probably write a patch for cocoa, so please don't complain to me.)

Have you already started your patch? Perhaps we can collaborate, no point in
doing duplicate work.  Also, I was under the impression the OSX portions of
Firefox was written in Carbon?  Thanks

Comment 19

[timeless]

i'll be writing my patch for cocoa which happens to be used by camino (you can
build firefox against cocoa but it isn't recommended and doesn't work very
well). no i haven't started, i haven't made it home [to my mac...] in a few days
(too much work).

there isn't much on which to collaborate. from the looks of it, the carbon
implementation requires you to get an api which will give you a windownumber
(good luck), after that it should be relatively downhill.

Comment 20

[Simon Fraser [no longer active]]

> i'll be writing my patch for cocoa which happens to be used by camino

There's no point. Camino doesn't make any XUL windows, so the hidden window
stuff doesn't apply, and Mozilla doesn't ship any XUL apps that use the Cocoa
toolkit.

Comment 21

[timeless]

blah, i new that was wrong right after i posted it. i'll still look into it (yes
i know xul-cocoa is broken, i might pick it up), it'll still be easier for me to
play with.

Comment 22

[Peter Van der Beken [:peterv]]

I have a patch to do the CGSTagSticky hack, but that doesn't block the movement
of the window during Exposé's "Show Windows", only during "Show Desktop". We'll
need something else.

Comment 23

[Peter Van der Beken [:peterv]]

Attachment: v1

This "hack" seems to work. The hidden window isn't exposed in the window list,
you can't cycle into it and it doesn't appear through option-minimize or
Exposé.

Comment 24

[Simon Fraser [no longer active]]

Looks OK, but it would be nice to replace all the NewCWindow calls with
CreateNewWindow.

Comment 25

[Peter Van der Beken [:peterv]]

Attachment: v2 (diff -w)

Comment 26

[Peter Van der Beken [:peterv]]

Comment on attachment 155177 [details] [diff] [review]
v2 (diff -w)

I think I handled all cases correctly, but the old code was spread out so it's
hard to know for sure. I tried to simplify it by setting all the attributes
upfront. Note that I'm not at all sure about the replacement for plainDBox.

Comment 27

[Peter Van der Beken [:peterv]]

Attachment: v2

Comment 28

[jhp (no longer active)]

Comment on attachment 155177 [details] [diff] [review]
v2 (diff -w)

Terrific patch!  Some comments:

+      default:
+	 windowClass = kDocumentWindowClass;
+	 attributes = kWindowCollapseBoxAttribute;
+	 if (allOrDefault || aInitData->mBorderStyle & eBorderStyle_close)
+	   attributes |= kWindowCloseBoxAttribute;
+	 break;
     }

Is this ever hit?  It seems to me that you've covered all of the cases except
for eWindowType_java, and that one doesn't seem to be used anywhere in the
code.  Might it not be better to the 'default' case assert, in case a new type
is added or eWindowType_java is actually used?

Since you removed some of the "#if TARGET_CARBON" lines, you may as well remove
the use of "nsToolkit::OnMacOSX()".

Comment 29

[jhp (no longer active)]

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

Comment 30

[jhp (no longer active)]

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

Comment 31

[Andrew Ferguson]

Bug 223779 (which is now duped to this) had blocking-aviary1.0mac+

Comment 32

[Peter Van der Beken [:peterv]]

(In reply to comment #28)
> Is this ever hit?  It seems to me that you've covered all of the cases except
> for eWindowType_java, and that one doesn't seem to be used anywhere in the
> code.  Might it not be better to the 'default' case assert, in case a new type
> is added or eWindowType_java is actually used?

Yeah, I'll do this.

> Since you removed some of the "#if TARGET_CARBON" lines, you may as well remove
> the use of "nsToolkit::OnMacOSX()".

Will do.

Comment 33

[jhp (no longer active)]

Setting blocking-aviary1.0mac properly.

Comment 34

[jhp (no longer active)]

Comment on attachment 155177 [details] [diff] [review]
v2 (diff -w)

Just tried to build Firefox with the 10.2.8 SDK (as it is normally built), and
I got several build breaks.
First, kSimpleWindow is not defined until the 10.3 SDK, even though the
documentation says that it is available in 10.1+.
Second, kWindowDoesNotCycleAttribute is not available until 10.3.  We may have
to check for 10.3 and only enable it then.  Or define it ourselves and just set
it for all versions; maybe 10.1 and 10.2 will just ignore it.  I don't have
either to test on, though.

Comment 35

[Peter Van der Beken [:peterv]]

Attachment: v3 (diff -w)

Comment 36

[Peter Van der Beken [:peterv]]

Attachment: v3

Comment 37

[jhp (no longer active)]

Comment on attachment 155967 [details] [diff] [review]
v3 (diff -w)

+    // XXX Need to special-case for OS X versions below 10.1, because
+    //     kSimpleWindowClass doesn't exist.
+    if (mWindowType == eWindowType_popup &&
+	 nsToolkit::OSXVersion() < MAC_OS_X_VERSION_10_1)
+    {
+      mWindowPtr = ::NewCWindow(nil, &wRect, "\p", false, kWindowSimpleProc,
+				 (WindowRef)-1, false, (long)nsnull);
+    }
+    else
+    {
+      ::CreateNewWindow(windowClass, attributes, &wRect, &mWindowPtr);
+    }

The system requirements for Mozilla on Mac start at OS X 10.1, so you can get
rid of the NewCWindow call.  Otherwise, it looks great.  Thanks.

Comment 38

[Tom Sutherland]

Is it possible someone can post a binary build with this patch?

Comment 39

[Peter Van der Beken [:peterv]]

Hmm, we allow running on OS X below 10.1, we just warn on startup.

Comment 40

[jhp (no longer active)]

Comment on attachment 155967 [details] [diff] [review]
v3 (diff -w)

Asking approval for aviary so we can get it in for PR1.

Comment 41

[David Catmull]

I think the cmd-~ issue can be fixed by handling the
kHICommandRotateWindowsForward command.

Comment 42

[jhp (no longer active)]

The attached patch already fixes the window cycling issue.

Comment 43

[Asa Dotzler [:asa]]

Comment on attachment 155967 [details] [diff] [review]
v3 (diff -w)

a=asa for aviary checkin.

Comment 44

[jhp (no longer active)]

Checked into Aviary, will check into trunk soon.

Comment 45

[Asaf Romano (gone)]

(In reply to comment #44)

We may want it for 1.7.3 as well.

Comment 46

[jhp (no longer active)]

Comment on attachment 155967 [details] [diff] [review]
v3 (diff -w)

Peterv already checked this into the trunk.  Asking for 1.7.3 approval.

Comment 47

[Jo Hermans]

I'm seeing a huge regression in both Seamonkey and Firefox versions of today :
see bug 257546 comment 2 (I thought it was related to bug 257546). Most pulldown
menus are displayed in the top-left corner. Is it related to this path ? I
wasn't visible yesterday, the builds just before this patch.

I'm running Mac Os X 10.2.8

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a3) Gecko/20040831

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.3) Gecko/20040831
Firefox/0.9.1+

Comment 48

[Peter Van der Beken [:peterv]]

File a separate bug on that please. I don't see these regressions in a debug
SeaMonkey build from today on OS X 10.3.5.

Comment 49

[Jo Hermans]

(In reply to comment #48)
> File a separate bug on that please. I don't see these regressions in a debug
> SeaMonkey build from today on OS X 10.3.5.

bug 257647

Comment 50

[Asaf Romano (gone)]

(In reply to comment #49)
> (In reply to comment #48)
> > File a separate bug on that please. I don't see these regressions in a debug
> > SeaMonkey build from today on OS X 10.3.5.
> 
> bug 257647
> 

As i mentioned there, it is not repro on 10.3+.

Can anybody check if fixing "just" the version check to > 10.2 instead of >=
10.2 changes something? (I didn't read the whole patch, so it is nothing more
than a guess).

Comment 51

[chris hofmann]

this needs to be backed out on the aviary and 1.7 branches until we can figure
out how to address  bug 257647

peter/jhpedemonte  can you help?

Comment 52

[Peter Van der Beken [:peterv]]

I'm backing it out. I'll need a 10.2 system to debug to find a fix.

Comment 53

[Peter Van der Beken [:peterv]]

Backed out of the aviary branch. I'm leaving this on the trunk for now. It's not
in 1.7.

Comment 54

[Asaf Romano (gone)]

(In reply to comment #53)

Clearing blocking flag.

Comment 55

[HARUNAGA Hirotoshi]

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

Comment 56

[Dave Chand]

Has this bug been fixed?
The web site http://www.squarefree.com/burningedge/bigger-picture.html
says so, but I still see the hidden window in 0.10pr

Comment 57

[Jo Hermans]

(In reply to comment #56)
> Has this bug been fixed?
> The web site http://www.squarefree.com/burningedge/bigger-picture.html
> says so, but I still see the hidden window in 0.10pr

It was fixed, but backed out again for bug 257647 (see comment 53). That bug has
already a patch, so this one will be checked in again soon.

Comment 58

[HARUNAGA Hirotoshi]

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

Comment 59

[Peter Van der Beken [:peterv]]

Comment on attachment 155967 [details] [diff] [review]
v3 (diff -w)

Asking for branch approval again. This patch on its own caused one regression
for which there is now a patch (in bug 257647). I'm asking for approval of both
patches. I'm not aware of any other regressions because of this patch (it's
been on the trunk for nearly three weeks).

Comment 60

[Asa Dotzler [:asa]]

Comment on attachment 155967 [details] [diff] [review]
v3 (diff -w)

a=asa for aviary checkin.

Comment 61

[jhp (no longer active)]

Checked into aviary branch.  -> FIXED

Comment 62

[Asaf Romano (gone)]

do we want it for 1.7 branch as well?

Comment 63

[Arthur Wiebe]

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

Comment 64

[Phil Ringnalda (:philor)]

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

Comment 65

[Tuukka Tolvanen (sp3000)]

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

Comment 66

[Asa Dotzler [:asa]]

We want this for 1.7.5. This is the single biggest win we can hand our Mac users
at this time.