Open Mozilla with one navigator window and one mailnews window 1) Use Alt+Tab to focus the navigator window. Use Ctrl+2 to switch to the mailnews window. Notice the mailnews window raises without focus. 2) Use Alt+Tab to focus the mailnews window. Use Ctrl+1 to switch to the navigator window. Notice the navigator window raises without focus.

Here is the log of nsWindow::SetFocus() (widget/src/gtk2) Suppose window[0x8aae6a8] is active. Press Ctrl+1 to switch to window[0x8757d28] We will get SetFocus [0x8757d28] grabbing focus for the toplevel [0x8757d28] Press Ctrl+2 to switch back We will get SetFocus [0x8aae6a8] already have focus [0x8aae6a8] A possible fix is change gtk_widget_grab_focus(owningWidget); to gtk_window_present((GtkWindow*)owningWindow->mShell); in nsWindow::SetFocus() Because gtk_widget_grab_focus doesn't work at all. I also found following comments. void nsWindow::OnContainerFocusInEvent(GtkWidget *aWidget, GdkEventFocus *aEvent) { LOGFOCUS(("OnContainerFocusInEvent [%p]\n", (void *)this)); // Return if someone has blocked events for this widget. This will // happen if someone has called gtk_widget_grab_focus() from // nsWindow::SetFocus() and will prevent recursion. if (mContainerBlockFocus) { printf("Container focus is blocked [%p]\n", (void *)this); return; } But nsWindow::OnContainerFocusInEvent is never invoked by calling gtk_widget_grab_focus(). Next, in nsWindow::SetFocus() after gtk_widget_grab_focus(), DispatchActivateEvent(); is never called either. Because mActivatePending is set PR_TRUE in nsWindow::OnContainerFocusInEvent() gtk_widget_grab_focus(owningWidget); owningWindow->mContainerBlockFocus = PR_FALSE; DispatchGotFocusEvent(); // unset the activate flag if (owningWindow->mActivatePending) { owningWindow->mActivatePending = PR_FALSE; DispatchActivateEvent(); }

see bug 202145

it is a possible fix if change gtk_widget_grab_focus(owningWidget); to gtk_window_present((GtkWindow*)owningWindow->mShell); in widget/src/gtk2/nsWindow.cpp

bryner, can you review it? Thanks.

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

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

Does GTK1 have the same problem?

yes, it works the same in both gtk1 and 2 see bug 202145 -- blizzard consider this a feature.

What I meant to say, was do we need the same fix for GTK1 as well?

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

(In reply to comment #9) > What I meant to say, was do we need the same fix for GTK1 as well? I wonder how many people are still building with GTK1. Firefox requires GTK2 now. I don't know how to fix this for GTK1 since GTK1 doesn't have gtk_window_present or a method to do so. I'm sure it's a bug not a feature. Consider we have a navigator window A with 2 or more tabs in workspace 1, and another navigator window B with only 1 tab in workspace 2. If we press CTRL+Q within the navigator window B. A "Confirm close" dialog will be popped up in workspace 1 but we could not notify it. It's bad as we're going to press CTRL+Q time and time again with no responses. It also blocks Bug 283103, Bug 260560 which are well-known security issues. From gtk2 documents: gtk_window_present () void gtk_window_present (GtkWindow *window); Presents a window to the user. This may mean raising the window in the stacking order, deiconifying it, moving it to the current desktop, and/or giving it the keyboard focus, possibly dependent on the user's platform, window manager, and preferences. If window is hidden, this function calls gtk_widget_show() as well. This function should be used when the user tries to open a window that's already open. Say for example the preferences dialog is currently open, and the user chooses Preferences from the menu a second time; use gtk_window_present() to move the already-open dialog where the user can see it.

I'm worried that this might make us start popping windows to the top unexpectedly. Did gtk_widget_grab_focus() grab the global focus and start directing all keystrokes there? Or did it just set the focus for the window it's in? Shouldn't we be raising windows only if aRaise is true in nsWindow::SetFocus? Why are the changes to navigator.js and tabbrowser.xml needed?

(In reply to comment #12) > I'm worried that this might make us start popping windows to the top > unexpectedly. Did gtk_widget_grab_focus() grab the global focus and start > directing all keystrokes there? Or did it just set the focus for the window > it's in? From GTK's manual, gtk_widget_grab_focus () void gtk_widget_grab_focus (GtkWidget *widget); Causes widget to have the keyboard focus for the GtkWindow it's inside. widget must be a focusable widget, such as a GtkEntry; something like GtkFrame won't work. (More precisely, it must have the GTK_CAN_FOCUS flag set.) So I think it can't grab the global focus. It should not be used in nsWindow::SetFocus. > Shouldn't we be raising windows only if aRaise is true in nsWindow::SetFocus? I don't think it's necessary. Can a window be focused but not showing? I think we should ignore aRaise. We should also remove GetAttention(-1) from nsWindow::SetFocus, gtk_window_present does the same job. I checked other platforms' nsWindow.cpp, aRaise is ignored. > Why are the changes to navigator.js and tabbrowser.xml needed? Make sure the "Confirm close" window get focus. Thanks for your comment!

> Can a window be focused but not showing? Sure. Lowered windows have focus all the time with sloppy focus.

If a web page calls focus() on one of its controls (e.g. when google.com loads it focuses the search box), will that cause the window to come to the front now? If changes were needed in navigator.js and tabbrowser.xml, what makes you think those are the only changes needed?

More comfortable patch. 1) keep gtk_widget_grab_focus(owningWidget); 2) test aRaise 3) split changes of navigator.js and tabbrowser.xml to another bug

(In reply to comment #15) > If a web page calls focus() on one of its controls (e.g. when google.com loads > it focuses the search box), will that cause the window to come to the front > now? No, set focus to a control won't bring the window up. window.focus() can bring it up. Let me clarify what will change with this patch. Without it: GetAttention(-1) raises the window, but it doesn't have focus. The old window under it still has focus. (It can cause a security hole.) With it: The window is raised with focus. It doesn't change the behaviour of whether the window is brought to front. > If changes were needed in navigator.js and tabbrowser.xml, what makes you think > those are the only changes needed? No, I don't think they're the only changes needed. A customer complained about the dialog, so I fixed it. Now I decide to split the patch, since they should be different bugs.

Checking in nsWindow.cpp; /cvsroot/mozilla/widget/src/gtk2/nsWindow.cpp,v <-- nsWindow.cpp new revision: 1.153; previous revision: 1.152 done

This caused bug 329675.

This also seems to have caused bug 330006 and bug 324348, and I really don't like the whole idea; see bug 330006 comment 0.

Please address the issues in bug 330006, or I'll back this out next week.

(In reply to comment #21) > Please address the issues in bug 330006, or I'll back this out next week. > I'm ok to back it out if the issue in bug 330006 comment 4 is addressed. I'm wondering which is really a problem for user, bug 330006 or bug 330006 comment 4 plus this bug and bug 283103.

(In reply to comment #22) > I'm ok to back it out if the issue in bug 330006 comment 4 is addressed. That's a modality bug, not a focus bug, and certainly shouldn't be solved by complicating low-level focus code with extraneous raises.

Also, I think this bug, as stated, is invalid. If you don't like your window manager focus preferences, change them.