119586 - FrontApplicationWindow determined incorrectly

Status: NEW

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Jonathan Brecher]

From Bugzilla Helper:
User-Agent: Mozilla/4.75 [en] (Windows NT 5.0; U)
BuildID:    20020109

We are developing a plugin that creates and manages its own floating windows 
(toolbars).  Works great -- except when it doesn't.

There are about a dozen places in the code where ::FrontWindow() is called 
directly.  Since we have created our floating windows (which always are in front 
of all non-floating windows), these calls generally return a reference to our 
window.  That then confuses Mozilla, which was expecting one of its own windows.

The specific case I found this with is in 
nsMacMessagePump::GetFrontApplicationWindow(), called from 
nsMacMessagePump::DoKey().  When my floating window is returned from this 
function, DispatchOSEventToRaptor() fails to find any WindowEventSink, and the 
key event is never sent to my plugin.  Bummer.

Does Mozilla ever create a floating window?  It doesn't look that way, but I'm 
not positive.  If not, I suspect that all the calls to ::FrontWindow() should be 
replaced with calls to FrontNonFloatingWindow().  That may or not be a complete 
solution, but it would definitely be an improvement.

The "complete" solution would be to tag every window created by Mozilla (using 
SetWRefCon, or SetWindowProperty) with some value you can recognize, and make 
sure that you always get the front *Mozilla* window when you're looking for 
one...

I'm not sure how widespread this problem is.  It may affect things in other ways 
(like, I wonder if one of my other bugs might be due to your disposing a window 
out from under me?).  I'll enter them as separate bugs as I find them, but the 
Front(NonFloating)Window issue described above is definitely a problem and 
blocking me from identifying the others.

In an attempt to fend off a "plugins shouldn't be creating their own floating 
windows" argument, I'll point out that this scheme worked well with Netscape 
4.x...

Reproducible: Always

Expected Results:  Plugins with floating windows should get key events and be 
able to deal with them accordingly (among other things -- see above).

I can provide an in-development version of our plugin if necessary.

Comment 1

[Jonathan Brecher]

I hate to pester, but I entered this bug two weeks ago and I don't know if 
anyone's even looked at it.  This is an absolute blocker for us.  We definitely 
can't ship our plugin until this gets fixed.  This issue also blocks us from 
doing large chunks of development as well.  Help?

Comment 2

[Robert John Churchill]

From MacWindows.h:

The Mac OS 8.5 Window Manager introduced the FrontNonFloatingWindow API, which
was designed to return the window that should be considered active by the
application. With the advent of window groups, it is now possible to have a
window that looks active (is highlighted, and accepts keyboard input) but to
have other non-floating windows grouped above the active window. The
ActiveNonFloatingWindow API returns the active window regardless of where it is
positioned in the z-order. Most code that currently uses FrontNonFloatingWindow
or GetFrontWindowOfClass(kDocumentClass) to get the active window should use
ActiveNonFloatingWindow instead.


which in semi-pseudo-code seems to mean:
[at least, for Mac Widget which has nsToolkit::OnMacOSX()]


#if TARGET_CARBON
  if (nsToolkit::OnMacOSX())
  {
      // ActiveNonFloatingWindow is not available in CarbonLib 1.x,
      // but is available on Mac OS X version 10.0 or later

      ::ActiveNonFloatingWindow
  }
  else
#else
    // Non-Carbon CFM: in WindowsLib 8.5 and later,
    // CarbonLib 1.0 and later,
    // and Mac OS X version 10.0 or later

    ::FrontNonFloatingWindow
#endif

Comment 3

[saari (gone)]

joki isn't the right owner for this, but I think rjc is since he is mucking in
the floater code. Sorry Robert :-)

Comment 4

[saari (gone)]

taking as I have a similar issue that I'm dealing with on OSX too

Comment 5

[saari (gone)]

really this time...

Comment 6

[Mike Pinkerton (not reading bugmail)]

we already tag any window that we create with SetWindowProperty and stash the
top level widget in there. shouldn't be tough to hack from there...

Comment 7

[Simon Fraser [no longer active]]

It would seem to me that putting a floating window into the window list of
another application is asking for trouble. Most Classic/Carbon apps would be
broken by this, since ::FrontWindow() will change. Isn't there a better way?

Comment 8

[Jonathan Brecher]

>Isn't there a better way?

In short, no.  We only need a few square inches of drawing space in the window,
but the user could readily have have a dozen toolbars/info windows open at a
time, with information about what is drawn in that space.  All of which *might*
be possible to workaround if we only had one plugin instance in a page, but we
often want several.  So, no, there really isn't a better way.

>Most Classic/Carbon apps would be broken by this, since ::FrontWindow() will
change.

Sure, those apps that depended on FrontWindow.  Of course, Netscape 4.x didn't.
 Neither does IE, apparently.  Neither does any of Netscape/IE/Mozilla on Windows.

Besides, you really want to change this yourself, anyway.  The places where
Mozilla is currently calling FrontWindow(), it doesn't want to be.  You're
really looking for FrontNonFloatingWindow() in all of those cases even now.  The
problem is that you're using FrontWindow to do the looking...

Comment 9

[Simon Fraser [no longer active]]

So what happens when we add floating window support to XUL, and we have our own
floating windows?

Comment 10

[saari (gone)]

We need to fix it to look for the custom mozilla tagged window as the event
target. If the window for the event is not ours, we assume it is a plugin and
dump it into the event system via the last focused widget, which in this case
should be the plugin as long as we also don't update the target with windows
that are not ours.

Anyone see problems with that? 

Comment 11

[Robert John Churchill]

>Isn't there a better way?

You could have your plugin, on Mac, talk to an external application which
managed its own windows in its own application space.  Just a thought.  :)

Comment 12

[Robert John Churchill]

> So what happens when we add floating window support to XUL,
> and we have our own floating windows?


Bug # 124313 is a good example...  I don't plan to try and get it in for Moz 1.0
as there was at least one focus issue resulting from adding floating windows in
the app which saari said he'd look at when given time.

Comment 13

[Jonathan Brecher]

>You could have your plugin, on Mac, talk to an external application which
>managed its own windows in its own application space.  Just a thought.  :)

I stand corrected  :-)

>So what happens when we add floating window support to XUL, and we have our own
>floating windows?

Well, if you fix this bug now, you won't have problems then.  If you don't fix
it now, you *will* have problems then.

Maybe it's worth looking at the specific places where Mozilla is calling
FrontWindow in the current sources:

a)  Two calls each in /plugin/oji/MRJ/plugin/Source/AltWindowHandling.cpp and
BackwardAdapter.cpp:  These calls already check that the window is owned
appropriately.  So at worst they'll do nothing.  I'm not sure when these
functions get executed.

b)  Three calls in PlaceBehind functions in
/plugin/oji/MRJ/plugin/Source/TopLevelFrame.cpp, /widget/src/mac/nsMacWindow.cpp
and /widget/src/cocoa/nsCocoaWindow.cpp.  This is used to adjust window
ordering, and should be changed to look for non-floating windows (or probably
better, windows of the same class).

c)  Two calls in /xpinstall/ that I don't care about because the installer isn't
going to be running plugins anyway

d)  One call in /gfx/src/mac/nsGfxUtils.h that simply checks for the presence of
a window and should be fine.

e)  One call in /webshell/tests/viewer/nsMacMain.cpp that is specifically
looking for a browser window and should changed to always ignore floaters

f)  One call in HandleMouseDownEvent in /widget/src/mac/nsMacEventHandler.cpp. 
This is definitely wrong in the presence of floaters.  I think you want
FrontWindowOfClass(GetWindowClass(whichWindow)), if I follow the logic right. 
I've read the comment several times and I still don't quite follow what this
code is trying to do.

g)  Five calls passed as a parameter to ZoomWindow in
/widget/src/mac/nsMacEventHandler.cpp (x1) and nsMacWindow.cpp (x2) and
/widget/src/cocoa/nsCocoaWindow.mm (x2).  Definitely wrong.  Should be changed
to FrontNonFloatingWindow

and finally...
h)  One call in GetFrontApplicationWindow() in
/widget/src/mac/nsMacMessagePump.cpp, which is the only issue that might
possibly be specific to our case.  This function is in turn called from four
places:  DoMouseUp, DoMouseMove, DoKey, and DispatchMenuCommandToRaptor.  The
first two are ok for our purposes (we can eat the events before you see them),
although it wouldn't hurt to make sure that the Application window really is
owned by Mozilla.  The third one is critical, and is definitely wrong.  Keyboard
events should go to the GetUserFocusWindow, not the FrontWindow.  That is going
to cause headaches even for Mozilla-created windows.  The fourth one should be
changed to look only for Mozilla-created windows, where a Raptor might possibly
exist.

Phew.  Does that cover it?  As long as you already have a mechanism for
identifying Mozilla-created windows, I think that everything here is simple and
non-controversial, and eliminates several bugs that will likely get in your way
as soon as you create floaters of your own.  Sounds like a win.  No?

Thanks!

Comment 14

[Robert John Churchill]

g)  Five calls passed as a parameter to ZoomWindow in
/widget/src/mac/nsMacEventHandler.cpp (x1) and nsMacWindow.cpp (x2) and
/widget/src/cocoa/nsCocoaWindow.mm (x2).  Definitely wrong.  Should be changed
to FrontNonFloatingWindow


Careful! Just switching from FrontWindow() to FrontNonFloatingWindow() is NOT
always the appropriate answer if we ever want Mozilla to work with its own
floating windows.

Comment 15

[Jonathan Brecher]

Um... Do you have a specific case in mind where it wouldn't be appropriate?  
ZoomWindow can't be called for floating windows, and will always want to ignore 
floating windows if present.

(And as of MacOS 8.5, Apple seems to be recommending to use ZoomWindowIdeal 
instead, which would avoid this particular sub-issue anyway)

Comment 16

[Marek Z. Jeziorek]

Minusing to topembed- and adding embed as per edt subteam.  Saari, could you
explain why you topembed'ed this bug?

Comment 17

[saari (gone)]

Personal request from plugin developer, and a definite architecture issue for
us. Falls into the Good Thing category, but not currently blocking any embeddors
that I know of

Comment 18

[Greg Hayes]

Is this the same reason why my keyboard entry into sherlock (amongst others)
gets lost when Mozilla is open? 

Comment 19

[Simon Fraser [no longer active]]

Greg: that would be bug 153067. If you can shed more light there, it would be
appreciated.

Comment 20

[rob webb]

From: "Support" <[e-mail]>
Date: Mon Jan 20, 2003  5:37:01 AM US/Pacific
To: "Robert Webb" <[e-mail]>
Subject: RE: {Talisma#063-759} ChemDraw Ultra for Mac

Thanks for your feedback.  Please know that it is not that we don't want to support newer 
versions of Netscape, IE, etc., but there are problems that are preventing it so far.  
Netscape 6 and above have a bug that has prevented us from releasing a compatible 
Plugin:

FAQ's > ChemDraw FAQ's > PLUGIN for Mac FAQ's > Solution 


Question: I'd like to use the ChemDraw Plugin with Mozilla or Netscape 7 for the Mac, but 
it doesn't work.

Solution: Please see Netscape/Mozilla's bug tracking website at: http://
bugzilla.mozilla.org/show_bug.cgi?id=119586 and please send an e-mail to the Mozilla 
developers asking that this bug, reported Jan 11, 2002, be fixed so that we can consider 
producing a plugin for these browsers. 

Comment 21

[Jonathan Brecher]

This exchange was just posted to the Carbon-development list:

At 10:14 AM -0400 5/12/03, Eric Schlegel wrote:
>On Monday, May 12, 2003, at 06:25  AM, Yaron Tadmor wrote:
>
>> If I want to know what is the currently active window (the one with 
>> the focus), should I use FrontWindow() or IsWindowActive() ?
>
>You should use ActiveNonFloatingWindow
>
>On Monday, May 12, 2003, at 06:51  AM, Trygve Inda wrote:
>
>> The currently active window is returned by FrontWindow().
>
>Sorry, that's incorrect. FrontWindow returns the front window in the 
>window list. In many apps that's not the active window; it could be a 
>help tag window or a floating window. Even if your app doesn't use help 
>tags or floating windows, the operating system may create such windows 
>in your app on your behalf; for example, the Text Services Manager may 
>create a bottomline input window for text input into your app, and that 
>window would be returned by FrontWindow.
>
>Always use ActiveNonFloatingWindow to find the active window.
>
>-eric

Comment 22

[Paul Wyskoczka]

adt: nsbeta1-

Comment 23

[Simon Paquet [:sipaq]]

Does not qualify as a blocker.