focus-related regressions in GTK port

RESOLVED FIXED

Status

()

Core
XUL
--
critical
RESOLVED FIXED
17 years ago
6 years ago

People

(Reporter: dbaron, Assigned: blizzard)

Tracking

({regression})

Trunk
x86
Linux
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: critical to 0.8.1)

Attachments

(4 attachments)

(Reporter)

Description

17 years ago
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.
(Reporter)

Updated

17 years ago
Keywords: mozilla0.8.1, nsdogfood, regression
(Reporter)

Comment 1

17 years ago
Created attachment 27982 [details]
stack showing one of the things that causes raise-on-mouseover

Comment 2

17 years ago
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.
I agree, the current behavior is not acceptable.

Updated

17 years ago
Whiteboard: critical to 0.8.1
(Assignee)

Comment 4

17 years ago
*** Bug 72314 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 5

17 years ago
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.
(Assignee)

Comment 6

17 years ago
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

17 years ago
*** Bug 72337 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 8

17 years ago
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?
(Assignee)

Comment 9

17 years ago
Created attachment 28000 [details] [diff] [review]
fix
sr=shaver
(Reporter)

Comment 11

17 years ago
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().
(Assignee)

Comment 12

17 years ago
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.
(Assignee)

Comment 13

17 years ago
Created attachment 28008 [details] [diff] [review]
final patch
(Assignee)

Comment 14

17 years ago
Created attachment 28009 [details] [diff] [review]
final patch
(Assignee)

Comment 15

17 years ago
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.
Status: NEW → ASSIGNED
(Assignee)

Comment 16

17 years ago
I have an sr=hyatt on this.

Comment 17

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

Comment 18

17 years ago
*** Bug 72385 has been marked as a duplicate of this bug. ***
r=brendan@mozilla.org, this time for sure!

/be
(Assignee)

Comment 20

17 years ago
r=jst on this, too

Comment 21

17 years ago
ahhhhhhhhhhh!!!!!! please check in the fix for this.

Comment 22

17 years ago
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!!
(Assignee)

Comment 23

17 years ago
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

17 years ago
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!! ;)
(Assignee)

Comment 25

17 years ago
Did you miss my comment or something?
*** Bug 72435 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 27

17 years ago
Checked into the tip and the 0.8.1 branch.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Assignee)

Comment 28

17 years ago
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 29

17 years ago
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.
*** Bug 72504 has been marked as a duplicate of this bug. ***

Comment 31

17 years ago
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

17 years ago
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
(Assignee)

Updated

17 years ago
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED
(Assignee)

Comment 33

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

Comment 34

17 years ago
*** Bug 72545 has been marked as a duplicate of this bug. ***
*** Bug 72598 has been marked as a duplicate of this bug. ***
*** Bug 72643 has been marked as a duplicate of this bug. ***
*** Bug 72661 has been marked as a duplicate of this bug. ***
*** Bug 72728 has been marked as a duplicate of this bug. ***
*** Bug 72765 has been marked as a duplicate of this bug. ***

Updated

17 years ago
Keywords: mostfreq
*** Bug 73017 has been marked as a duplicate of this bug. ***
*** Bug 72992 has been marked as a duplicate of this bug. ***

Comment 42

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

Comment 43

17 years ago
*** Bug 73290 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.