Handle keynav vs. mousenav in mouse and sloppy focus modes. Fixes #167545.

2005-02-21 Elijah Newren <newren@gmail.com> Handle keynav vs. mousenav in mouse and sloppy focus modes. Fixes #167545. * doc/how-to-get-focus-right.txt: Update due to this new method for handling keynav vs. mousenav, plus various other updates that I previously forgot. * src/display.h: (struct _MetaDisplay): add a mouse_mode boolean * src/display.c: (meta_display_open): initialize mouse_mode to true, (event_callback): have EnterNotify and LeaveNotify events set mouse_mode to true when focusing a window * src/keybindings.c: (process_tab_grab): set mouse_mode to false when using alt-tab/alt-esc, (do_choose_window): likewise, (do_handle_move_to_workspace): set mouse_mode to false on move-window-to-workspace-<n> keybindings * src/window.c (idle_calc_showing): if we're in keynav mode while using sloppy or mouse focus, use metacity_sentinel to avoid EnterNotify events being generated from events other than mouse movement. * src/workspace.c (meta_workspace_activate_with_focus): add a FIXME in a potentially duplicate section of code, (meta_workspace_focus_default_window): use the same focus choice as click-to-focus if in keynav mode.
2005-02-22 02:11:25 +00:00 · 2005-02-22 02:11:25 +00:00 · 612507260a
commit 612507260a
parent ad99427b49
7 changed files with 152 additions and 19 deletions
--- a/30
+++ b/30
@ -1,3 +1,33 @@
+2005-02-21  Elijah Newren  <newren@gmail.com>
+
+	Handle keynav vs. mousenav in mouse and sloppy focus modes.  Fixes
+	#167545.
+
+	* doc/how-to-get-focus-right.txt: Update due to this new method
+	for handling keynav vs. mousenav, plus various other updates that
+	I previously forgot.
+
+	* src/display.h: (struct _MetaDisplay): add a mouse_mode boolean
+
+	* src/display.c: (meta_display_open): initialize mouse_mode to
+	true, (event_callback): have EnterNotify and LeaveNotify events
+	set mouse_mode to true when focusing a window
+
+	* src/keybindings.c: (process_tab_grab): set mouse_mode to false
+	when using alt-tab/alt-esc, (do_choose_window): likewise,
+	(do_handle_move_to_workspace): set mouse_mode to false on
+	move-window-to-workspace-<n> keybindings
+
+	* src/window.c (idle_calc_showing): if we're in keynav mode while
+	using sloppy or mouse focus, use metacity_sentinel to avoid
+	EnterNotify events being generated from events other than mouse
+	movement.
+
+	* src/workspace.c (meta_workspace_activate_with_focus): add a
+	FIXME in a potentially duplicate section of code,
+	(meta_workspace_focus_default_window): use the same focus choice
+	as click-to-focus if in keynav mode.
+
 2005-02-20  Elijah Newren  <newren@gmail.com>

 	* src/display.c: (event_callback): Handle _NET_CURRENT_DESKTOP
--- a/doc/how-to-get-focus-right.txt
+++ b/doc/how-to-get-focus-right.txt
@ -79,9 +79,26 @@ to alert the user that there is a window to work with:
 Additionally, the user may decide to use the keyboard instead of the mouse
 to navigate between windows (referred to as "keynav").  This poses no
 problems for click-to-focus (because the same invariant can be
-maintained), but for sloppy and mouse focus it means that EnterNotify
-and LeaveNotify events should be ignored (they can be generated
-without using the mouse, for example, by grabs).
+maintained), but for sloppy and mouse focus it requires extra work to
+attempt to handle the INHERENTLY CONFLICTING CONSTRAINTS.  Metacity does
+this by having a mouse_mode boolean used to determine which of the two
+sets of invariants holds.  This mode is set according to which method was
+most recently used to choose a focus window:
+  1) When receiving EnterNotify/LeaveNotify events from mouse movement, set
+     mouse_mode to TRUE.
+  2) When using keynav to choose a focus window (e.g. alt-tab, alt-esc,
+     move-window-to-workspace keybindings), set mouse_mode to FALSE.
+  3) When handling events that don't choose a focus window but rather need
+     a focus_window chosen for them (e.g. switch-to-workspace keybindings),
+     don't change the mouse_mode and just use the current value.
+Note that grabs present a special case since they can generate EnterNotify
+and LeaveNotify events without using the mouse, thus these events should be
+ignored when the crossing mode is NotifyGrab or NotifyUngrab.  THIS
+MOUSENAV/KEYNAV MODERATION METHOD IS NOT PERFECT--there are corner cases
+when trying to mix-and-match between mousenav and keynav simultaneously
+that cause problems; but it appears to be the most reasonable tradeoff and
+works well in most cases, especially if the user sticks to just mousenav
+for a long time or just keynav for a long time.

 Finally, windows of type WM_DOCK or WM_DESKTOP (e.g. the desktop and
 the panel) present a special case, at least partially due to the lack
@ -103,7 +120,8 @@ To read more about the bugs that inspired these choices:
    Also, the EWMH spec, especially the parts relating to _NET_WM_USER_TIME
  - Modal vs. non-modal dialogs that get denied focus when mapped
    http://bugzilla.gnome.org/show_bug.cgi?id=151996
-  - Ignoring EnterNotify and LeaveNotify events during keynav
+  - Mousenav vs. Keynav in mouse and sloppy focus modes
+    http://bugzilla.gnome.org/show_bug.cgi?id=167545
    http://bugzilla.gnome.org/show_bug.cgi?id=101190
  - Not focusing panels
    http://bugzilla.gnome.org/show_bug.cgi?id=160470
@ -121,6 +139,7 @@ the ones I'm the most familiar with):
  bug  95747  should ignore EnterNotify events with NotifyInferior detail set
  bug  97635  sticky windows always keep focus when switching workspaces
  bug 102665  a window unminimized from the tasklist should be focused
+  bug 107347  focus windows that manually position themselves too
  bug 108643  focus in MRU order instead of stack order
  bug 110970  moving a window to another workspace loses focus
  bug 112031  closing a dialog can result in a strange focus window
@ -128,6 +147,7 @@ the ones I'm the most familiar with):
  bug 120100  panel shouldn't be focused after workspace applet usage
  bug 123803  need final EnterNotify after workspace switch (see also 124798)
  bug 124981  focus clicked window in pager only if on current workspace
+  bug 125492  catch the xserver unfocusing everything and fix its braindeadedness
  bug 128200  focus correct window on libwnck window minimize (see 107681 too)
  bug 131582  fix race condition on window minimize/close
  bug 133120  wrong window focused when changing workspaces
@ -148,6 +168,18 @@ the ones I'm the most familiar with):
  bug 151990  prevent focus inconsistencies by only providing one focus method
  bug 151996  modal dialogs denied focus should not be lowered
  bug 152000  fix race on window close followed by rapid mouse movement
+  bug 152004  ways to handle new window versus mouse invariants
+  bug 153220  catch the root window getting focus and reset to default window
+  bug 157360  focus parents of dismissed transient windows in preference to
+              the window that most recently had focus
+  bug 159257  focus the desktop when showing it
+  bug 160470  don't focus panels on click
+  bug 163450  correct highlighting in workspace switcher popup
+  bug 164716  refuse to focus a window with a modal transient, and focus
+              the transient instead
+  bug 166524  avoid new windows being obscured by the focus window
+  bug 167545  mousenav vs. keynav in mouse and sloppy focus modes
+  <a massive heap of bugs relating to focus stealing prevention...>


 Addendum on sloppy and mouse focus
@ -156,7 +188,7 @@ Addendum on sloppy and mouse focus

  1) Keynav doesn't maintain the same invariants as mouse navigation
     for these focus modes; switching back and forth between
-     navigation methods, therefore, may appear to have
+     navigation methods, therefore, may have or appear to have
     inconsistencies.  Examples:
     a) If the user uses Alt-Tab to change the window with focus, then
        starts to move the mouse, at that moment the window where the
@ -182,6 +214,15 @@ Addendum on sloppy and mouse focus
        containing the menu) but is one of those hard-to-get-right
        keynav and mouse focus mixture cases.  (See bug 101190 for
        more details)
+     d) Similar to (c), moving the mouse off the menu doesn't immediately
+        focus the window that the mouse goes over, due to an application
+        grab (we couldn't change this and wouldn't want to, but
+        technically it does break the invariant).
+     e) If mouse_mode is off and the user does something to cause focus to
+        change (e.g. switch workspaces, close or minimize a window, etc.)
+        and simultaneously tries to move the mouse, the choice of which
+        window to focus is inherently race-y.  (You probably can't satisfy
+        both keynav and mousenav invariants simultaneously...)
  2) The sloppy/mouse invariants are often not strictly maintained;
     for example, we provide an exception to the invariant for newly
     mapped windows.  (Most find that not allowing this exception is
--- a/src/display.c
+++ b/src/display.c
@ -343,6 +343,8 @@ meta_display_open (const char *name)
  display->expected_focus_window = NULL;
  display->grab_old_window_stacking = NULL;

+  display->mouse_mode = TRUE; /* Only relevant for mouse or sloppy focus */
+
 #ifdef HAVE_XSYNC
  display->grab_sync_request_alarm = None;
 #endif
@ -1785,10 +1787,15 @@ event_callback (XEvent   *event,
                  window->type != META_WINDOW_DESKTOP)
                {
                  meta_topic (META_DEBUG_FOCUS,
-                              "Focusing %s due to enter notify with serial %lu\n",
-                              window->desc, event->xany.serial);
+                              "Focusing %s due to enter notify with serial %lu "
+                              "at time %lu, and setting display->mouse_mode to "
+                              "TRUE.\n",
+                              window->desc, 
+                              event->xany.serial,
+                              event->xcrossing.time);

-		  meta_window_focus (window, event->xcrossing.time);
+                  display->mouse_mode = TRUE;
+                  meta_window_focus (window, event->xcrossing.time);

 		  /* stop ignoring stuff */
 		  reset_ignores (display);
@ -1822,16 +1829,28 @@ event_callback (XEvent   *event,
          switch (meta_prefs_get_focus_mode ())
            {
            case META_FOCUS_MODE_MOUSE:
-              if (window == display->expected_focus_window &&
-                  (window->frame == NULL || frame_was_receiver) &&
+              if ((window->frame == NULL || frame_was_receiver) &&
 		  event->xcrossing.mode != NotifyGrab && 
 		  event->xcrossing.mode != NotifyUngrab &&
 		  event->xcrossing.detail != NotifyInferior)
                {
-                  meta_verbose ("Unsetting focus from %s due to LeaveNotify\n",
-                                window->desc);
-                  meta_display_focus_the_no_focus_window (display, 
-                                                          event->xcrossing.time);
+                  if (window == display->expected_focus_window)
+                    {
+                      meta_topic (META_DEBUG_FOCUS,
+                                  "Unsetting focus from %s due to LeaveNotify\n",
+                                  window->desc);
+                      meta_display_focus_the_no_focus_window (display, 
+                                                              event->xcrossing.time);
+                    }
+                  if (window->type != META_WINDOW_DOCK &&
+                      window->type != META_WINDOW_DESKTOP)
+                    {
+                      meta_topic (META_DEBUG_FOCUS,
+                                  "Setting display->mouse_mode to TRUE due to "
+                                  "LeaveNotify at time %lu.\n",
+                                  event->xcrossing.time);
+                      display->mouse_mode = TRUE;
+                    }
                }
              break;
            case META_FOCUS_MODE_SLOPPY:
--- a/src/display.h
+++ b/src/display.h
@ -204,6 +204,11 @@ struct _MetaDisplay
  /* last user interaction time in any app */
  Time last_user_time;

+  /* whether we're using mousenav (only relevant for sloppy&mouse focus modes;
+   * !mouse_mode means "keynav mode")
+   */
+  guint mouse_mode : 1;
+
  guint static_gravity_works : 1;
  
  /*< private-ish >*/
--- a/src/keybindings.c
+++ b/src/keybindings.c
@ -2378,8 +2378,10 @@ process_tab_grab (MetaDisplay *display,
          meta_topic (META_DEBUG_KEYBINDINGS,
                      "Activating target window\n");

-          meta_topic (META_DEBUG_FOCUS, "Activating %s due to tab popup selection\n",
+          meta_topic (META_DEBUG_FOCUS, "Activating %s due to tab popup "
+                      "selection and turning mouse_mode off\n",
                      target_window->desc);
+          display->mouse_mode = FALSE;
          meta_window_activate (target_window, event->xkey.time);

          meta_topic (META_DEBUG_KEYBINDINGS,
@ -2993,8 +2995,11 @@ do_choose_window (MetaDisplay    *display,
          /* If no modifiers, we can't do the "hold down modifier to keep
           * moving" thing, so we just instaswitch by one window.
           */
-          meta_topic (META_DEBUG_FOCUS, "Activating %s due to switch/cycle windows with no modifiers\n",
+          meta_topic (META_DEBUG_FOCUS,
+                      "Activating %s and turning off mouse_mode due to "
+                      "switch/cycle windows with no modifiers\n",
                      initial_selection->desc);
+          display->mouse_mode = FALSE;
          meta_window_activate (initial_selection, event->xkey.time);
        }
      else if (meta_display_begin_grab_op (display,
@ -3017,9 +3022,13 @@ do_choose_window (MetaDisplay    *display,
               * before we establish the grab. must end grab
               * prior to trying to focus a window.
               */
-              meta_topic (META_DEBUG_FOCUS, "Ending grab and activating %s due to switch/cycle windows where modifier was released prior to grab\n",
+              meta_topic (META_DEBUG_FOCUS, 
+                          "Ending grab, activating %s, and turning off "
+                          "mouse_mode due to switch/cycle windows where "
+                          "modifier was released prior to grab\n",
                          initial_selection->desc);
              meta_display_end_grab_op (display, event->xkey.time);
+              display->mouse_mode = FALSE;
              meta_window_activate (initial_selection, event->xkey.time);
            }
          else
@ -3275,7 +3284,15 @@ do_handle_move_to_workspace  (MetaDisplay    *display,
      /* Activate second, so the window is never unmapped */
      meta_window_change_workspace (window, workspace);
      if (flip)
-        meta_workspace_activate_with_focus (workspace, window, event->xkey.time);
+        {
+          meta_topic (META_DEBUG_FOCUS,
+                      "Resetting mouse_mode to FALSE due to "
+                      "do_handle_move_to_workspace() call with flip set.\n");
+          workspace->screen->display->mouse_mode = FALSE;
+          meta_workspace_activate_with_focus (workspace,
+                                              window,
+                                              event->xkey.time);
+        }
    }
  else
    {
--- a/src/window.c
+++ b/src/window.c
@ -1471,6 +1471,25 @@ idle_calc_showing (gpointer data)
      tmp = tmp->next;
    }

+  if (meta_prefs_get_focus_mode () != META_FOCUS_MODE_CLICK)
+    {
+      /* When display->mouse_mode is false, we want to ignore
+       * EnterNotify events unless they come from mouse motion.  To do
+       * that, we set a sentinel property on the root window if we're
+       * not in mouse_mode.
+       */
+      tmp = should_show;
+      while (tmp != NULL)
+        {
+          MetaWindow *window = tmp->data;
+          
+          if (!window->display->mouse_mode)
+            meta_display_increment_focus_sentinel (window->display);
+
+          tmp = tmp->next;
+        }
+    }
+
  g_slist_free (copy);

  g_slist_free (unplaced);
--- a/src/workspace.c
+++ b/src/workspace.c
@ -317,6 +317,7 @@ meta_workspace_activate_with_focus (MetaWorkspace *workspace,
  meta_workspace_queue_calc_showing (old);
  meta_workspace_queue_calc_showing (workspace);

+  /* FIXME: Why do we need this?!?  Isn't it handled in the lines above? */
  if (move_window)
      /* Removes window from other spaces */
      meta_window_change_workspace (move_window, workspace);
@ -800,7 +801,8 @@ meta_workspace_focus_default_window (MetaWorkspace *workspace,
    }


-  if (meta_prefs_get_focus_mode () == META_FOCUS_MODE_CLICK)
+  if (meta_prefs_get_focus_mode () == META_FOCUS_MODE_CLICK ||
+      !workspace->screen->display->mouse_mode)
    focus_ancestor_or_mru_window (workspace, not_this_one, timestamp);
  else
    {