72313 - focus-related regressions in GTK port

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

There are lots of focus-related regressions in the GTK port due to blizzard's
fix for bug 71266:

 * moving the mouse into a window causes it to be focused (see stack that I'll
attach).  This prevents one from using mozilla with another window on top of it,
which is something that I and many other users expect in many window managers.

 * opening two browser windows that overlap, moving the mouse into the
overlapping area, switching virtual desktops away from these windows, and
switching virtual desktops back to these windows causes these windows to go into
a loop of raising themselves until I move the mouse out of the overlap area

I think these are serious-enough UI problems that this patch should not be in
0.8.1 (which it looks like it will be, judging by the cvs log):

symbolic names:
        MOZILLA_0_8_1_2001031617_BASE: 1.324

I'm using sawfish on RH 7.0 with sloppy-focus.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: stack showing one of the things that causes raise-on-mouseover

Comment 2

[Kevin Buhr]

Let me nominate this bug for the "nuclear-wars-are-fought-over-less" keyword. 
This must be fixed for 0.8.1, or half (or more) of your Linux users will be
demanding instructions on how to downgrade to 0.8.

Comment 3

[Johnny Stenback (:jst)]

I agree, the current behavior is not acceptable.

Comment 4

[Christopher Blizzard (:blizzard)]

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

Comment 5

[Christopher Blizzard (:blizzard)]

I actually might want to move the GetAttention() into a check that makes sure
that the window in question doesn't already have focus.  I'll whip up a patch
and play with it with sloppy focus and see how it does when I get back from
running some errands.

Comment 6

[Christopher Blizzard (:blizzard)]

Also note that you can set user_pref("mozilla.widget.raise-on-setfocus", false);
in your prefs.js file if you don't ever want that behaviour.

Comment 7

[Bradley Baetz]

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

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Why does this change need to affect anything other than calls to
window.focus()?  Couldn't we add a SetFocusAndRaise method to nsIWidget
that calls SetFocus on all the other ports but does the raising on GTK,
and have window.focus() call that instead?  Or could we use the existing
|GetAttention| from window.focus()?  Isn't SetFocus the wrong place to
make this change since it's already used by other code?

Comment 9

[Christopher Blizzard (:blizzard)]

Attachment: fix

Comment 10

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

sr=shaver

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

r=dbaron, although it would be a good idea to add a comment to nsIWidget.h
saying that SetFocus must raise the window (if it doesn't already have focus?)
since correct backwards-compatible DOM behavior requires that cross-platform.

Does this still fix all the DOM problems?  What did window.focus() do in 4.x
for a window that was focused but not raised, and what do we do now?  It still
seems like it might be better to change only what happens from window.focus().

Comment 12

[Christopher Blizzard (:blizzard)]

OK, I did some testing in 4.x because I don't remember 4.x being anywhere this
annoying and it turns out that calling element.focus() does _not_ raise the
window.  You have to explicitly call window.focus() to raise the window.  I'd
rather emulate this behaviour.  I'm going to do some more testing here.

Comment 13

[Christopher Blizzard (:blizzard)]

Attachment: final patch

Comment 14

[Christopher Blizzard (:blizzard)]

Attachment: final patch

Comment 15

[Christopher Blizzard (:blizzard)]

The last patch is correct.  Mozilla can't upload attachments. :(

Anyway, this adds a flag to |nsIWidget::SetFocus| that defaults to PR_FALSE that
indicates whether or not the window should be raised.  I've changed the call
site for SetFocus where window.focus() is called from.  Looking for reviews for
the one line of DOM changes and mac + windows platform checkin buddies to test
the patch there to make sure that it builds OK.

Comment 16

[Christopher Blizzard (:blizzard)]

I have an sr=hyatt on this.

Comment 17

[R.K.Aa.]

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

Comment 18

[Fabian Guisset]

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

Comment 19

[Brendan Eich [:brendan]]

r=[e-mail], this time for sure!

/be

Comment 20

[Christopher Blizzard (:blizzard)]

r=jst on this, too

Comment 21

[Chris McAfee]

ahhhhhhhhhhh!!!!!! please check in the fix for this.

Comment 22

[Colin Blake]

This fix isn't in the 0.81 tree yet, right (as of Monday 3/19)? On my OpenVMS 
(CDE desktop) browser, I can't LOWER a Mozilla window at all. It just stays on 
top regardless!!

Comment 23

[Christopher Blizzard (:blizzard)]

I can't check this in until I get it tested on windows + mac to make sure that I
don't break them when I check it in.  Please test it if you have those
platforms.  I don't.

Comment 24

[Gregory McLean]

Please for the love all things sane get this checked in... I feel like I'm
playing wack-a-mozilla now. minimize, raise, minimize, raise over and over
again... Make it stop!! ;)

Comment 25

[Christopher Blizzard (:blizzard)]

Did you miss my comment or something?

Comment 26

[Boris Zbarsky [:bzbarsky]]

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

Comment 27

[Christopher Blizzard (:blizzard)]

Checked into the tip and the 0.8.1 branch.

Comment 28

[Christopher Blizzard (:blizzard)]

Argh!  I am so lame.  I forgot to put the check in for window focus for sloppy
focus no raise users.

Index: nsWindow.cpp
===================================================================
RCS file: /cvsroot/mozilla/widget/src/gtk/nsWindow.cpp,v
retrieving revision 1.325
diff -u -r1.325 nsWindow.cpp
--- nsWindow.cpp        2001/03/19 17:55:43     1.325
+++ nsWindow.cpp        2001/03/19 19:41:54
@@ -1089,8 +1089,10 @@
   if (top_mozarea)
     toplevel = gtk_widget_get_toplevel(top_mozarea);
 
-  // map the window if the pref says to
-  if (gRaiseWindows && aRaise)
+  // map the window if the pref says to and neither the mozarea or its
+  // toplevel window has focus
+  if (gRaiseWindows && aRaise && toplevel && top_mozarea &&
+      (!GTK_WIDGET_HAS_FOCUS(top_mozarea) && !GTK_WIDGET_HAS_FOCUS(toplevel)))
     GetAttention();
 
 #ifdef DEBUG_FOCUS

Comment 29

[Colin Blake]

I was just about to report that LOWER still didn't work, but then I saw your 
change to widget/src/gtk/nsWindow.cpp. With this fix in, LOWER works again on 
OpenVMS.

Comment 30

[Boris Zbarsky [:bzbarsky]]

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

Comment 31

[lidl]

With the last patch to the nsWindow.cpp file installed locally, the browswer
once again works as expected, at least for me, on BSD/OS (which uses the GTK
inteface).

Thanks!

Comment 32

[Colin Phillips]

The lastest build is still rather screwy.  In this mornings build (2001031910)
as soon as I mouse over a Mozilla window that window takes focus.  I am using
FocusFollowsMouse, but I also have a 750 ms delay before a window should take
focus.  Mozilla is not following the second rule, the moment the first mouse
movement is captured over top of a Mozilla window, that window will come to the
front (it wont wait the 750 ms delay like its supposed to).  Rrrrrr

Comment 33

[Christopher Blizzard (:blizzard)]

OK, that patch is in.  I think this is finally fixed for real.

Comment 34

[R.K.Aa.]

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

Comment 35

[Boris Zbarsky [:bzbarsky]]

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

Comment 36

[Boris Zbarsky [:bzbarsky]]

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

Comment 37

[Boris Zbarsky [:bzbarsky]]

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

Comment 38

[Boris Zbarsky [:bzbarsky]]

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

Comment 39

[Boris Zbarsky [:bzbarsky]]

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

Comment 40

[Doron Rosenberg (IBM)]

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

Comment 41

[Boris Zbarsky [:bzbarsky]]

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

Comment 42

[R.K.Aa.]

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

Comment 43

[R.K.Aa.]

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