only hop window to the current workspace if the window was previously

2003-01-21  Havoc Pennington  <hp@redhat.com>

	* src/display.c (event_callback): only hop window to the current
	workspace if the window was previously minimized. Should keep
	mozilla from popping windows over to your current workspace.

2003-01-20  Havoc Pennington  <hp@redhat.com>

        Attempt to fix #85916

	* src/keybindings.c (primary_modifier_still_pressed): new function
	(handle_workspace_switch): handle modifier release prior to
	getting the grab
	(do_choose_window): handle modifier release prior to getting the
	grab

	* src/keybindings.c (grab_keyboard): properly return failure
	if the GrabKeyboard doesn't work

3 files changed, 189 insertions, 44 deletions

diff --git a/ChangeLog b/ChangeLog
index e39fe778..58130593 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,22 @@
+2003-01-21  Havoc Pennington  <hp@redhat.com>
+
+	* src/display.c (event_callback): only hop window to the current
+	workspace if the window was previously minimized. Should keep
+	mozilla from popping windows over to your current workspace.
+	
+2003-01-20  Havoc Pennington  <hp@redhat.com>
+
+        Attempt to fix #85916
+	
+	* src/keybindings.c (primary_modifier_still_pressed): new function
+	(handle_workspace_switch): handle modifier release prior to
+	getting the grab
+	(do_choose_window): handle modifier release prior to getting the 
+	grab
+
+	* src/keybindings.c (grab_keyboard): properly return failure
+	if the GrabKeyboard doesn't work
+
 2003-01-19  Havoc Pennington  <hp@pobox.com>
 
 	* configure.in: add note about how this is the unstable branch,
diff --git a/src/display.c b/src/display.c
index 01004123..9dac79f6 100644
--- a/src/display.c
+++ b/src/display.c
@@ -1277,11 +1277,12 @@ event_callback (XEvent   *event,
            display->grab_window == window) ||
           grab_op_is_keyboard (display->grab_op))
         {
-          meta_verbose ("Ending grab op %d on window %s due to button press\n",
-                        display->grab_op,
-                        (display->grab_window ?
-                         display->grab_window->desc : 
-                         "none"));
+          meta_topic (META_DEBUG_WINDOW_OPS,
+                      "Ending grab op %d on window %s due to button press\n",
+                      display->grab_op,
+                      (display->grab_window ?
+                       display->grab_window->desc : 
+                       "none"));
           meta_display_end_grab_op (display,
                                     event->xbutton.time);
         }
@@ -1634,12 +1635,20 @@ event_callback (XEvent   *event,
       /* if frame was receiver it's some malicious send event or something */
       else if (!frame_was_receiver && window)        
         {
+          meta_verbose ("MapRequest on %s mapped = %d minimized = %d\n",
+                        window->desc, window->mapped, window->minimized);
           if (window->minimized)
-            meta_window_unminimize (window);
-	  if (!meta_workspace_contains_window (window->screen->active_workspace,
-					       window))
-	    meta_window_change_workspace (window,
-					  window->screen->active_workspace);
+            {
+              meta_window_unminimize (window);
+              if (!meta_workspace_contains_window (window->screen->active_workspace,
+                                                   window))
+                {
+                  meta_verbose ("Changing workspace due to MapRequest mapped = %d minimized = %d\n",
+                                window->mapped, window->minimized);
+                  meta_window_change_workspace (window,
+                                                window->screen->active_workspace);
+                }
+            }
         }
       break;
     case ReparentNotify:
@@ -2312,6 +2321,9 @@ meta_spew_event (MetaDisplay *display,
       break;
     case MapRequest:
       name = "MapRequest";
+      extra = g_strdup_printf ("window: 0x%lx parent: 0x%lx\n",
+                               event->xmaprequest.window,
+                               event->xmaprequest.parent);
       break;
     case ReparentNotify:
       name = "ReparentNotify";
@@ -2684,7 +2696,14 @@ meta_display_set_grab_op_cursor (MetaDisplay *display,
         {
           display->grab_have_pointer = TRUE;
           meta_topic (META_DEBUG_WINDOW_OPS,
-                      "XGrabPointer() returned GrabSuccess\n");
+                      "XGrabPointer() returned GrabSuccess time 0x%lu\n",
+                      timestamp);
+        }
+      else
+        {
+          meta_topic (META_DEBUG_WINDOW_OPS,
+                      "XGrabPointer() failed time 0x%lu\n",
+                      timestamp);
         }
       meta_error_trap_pop (display, TRUE);
     }
@@ -2872,7 +2891,9 @@ void
 meta_display_end_grab_op (MetaDisplay *display,
                           Time         timestamp)
 {
-  meta_verbose ("Ending grab op %d at time %ld\n", display->grab_op, timestamp);
+  meta_topic (META_DEBUG_WINDOW_OPS,
+              "Ending grab op %d at time %lu\n", display->grab_op,
+              (unsigned long) timestamp);
   
   if (display->grab_op == META_GRAB_OP_NONE)
     return;
@@ -2903,7 +2924,7 @@ meta_display_end_grab_op (MetaDisplay *display,
   if (display->grab_have_keyboard)
     {
       meta_topic (META_DEBUG_WINDOW_OPS,
-                  "Ungrabbing all keys\n");
+                  "Ungrabbing all keys timestamp %lu\n", timestamp);
       if (display->grab_window)
         meta_window_ungrab_all_keys (display->grab_window);
       else
diff --git a/src/keybindings.c b/src/keybindings.c
index 658d5f7f..fbc9de46 100644
--- a/src/keybindings.c
+++ b/src/keybindings.c
@@ -1055,25 +1055,55 @@ meta_window_ungrab_keys (MetaWindow  *window)
     }
 }
 
+#ifdef WITH_VERBOSE_MODE
+static const char*
+grab_status_to_string (int status)
+{
+  switch (status)
+    {
+    case AlreadyGrabbed:
+      return "AlreadyGrabbed";
+    case GrabSuccess:
+      return "GrabSuccess";
+    case GrabNotViewable:
+      return "GrabNotViewable";
+    case GrabFrozen:
+      return "GrabFrozen";
+    case GrabInvalidTime:
+      return "GrabInvalidTime";
+    default:
+      return "(unknown)";
+    }
+}
+#endif /* WITH_VERBOSE_MODE */
+
 static gboolean
 grab_keyboard (MetaDisplay *display,
                Window       xwindow)
 {
   int result;
-
+  int grab_status;
+  Time timestamp;
+  
   /* Grab the keyboard, so we get key releases and all key
    * presses
    */
   meta_error_trap_push_with_return (display);
 
-  if (XGrabKeyboard (display->xdisplay,
-                     xwindow, True,
-                     GrabModeAsync, GrabModeAsync,
-                     meta_display_get_current_time (display)) != GrabSuccess)
+  timestamp = meta_display_get_current_time (display);
+  grab_status = XGrabKeyboard (display->xdisplay,
+                               xwindow, True,
+                               GrabModeAsync, GrabModeAsync,
+                               timestamp);
+  
+  if (grab_status != GrabSuccess)
     {
       meta_error_trap_pop_with_return (display, TRUE);
       meta_topic (META_DEBUG_KEYBINDINGS,
-                  "XGrabKeyboard() returned failure\n");
+                  "XGrabKeyboard() returned failure status %s time %lu\n",
+                  grab_status_to_string (grab_status),
+                  (unsigned long) timestamp);
+      return FALSE;
     }
   else
     {
@@ -1274,10 +1304,9 @@ is_specific_modifier (MetaDisplay *display,
   return retval;
 }
 
-static gboolean
-keycode_is_primary_modifier (MetaDisplay *display,
-                             unsigned int keycode,
-                             unsigned int entire_binding_mask)
+static unsigned int
+get_primary_modifier (MetaDisplay *display,
+                      unsigned int entire_binding_mask)
 {
   /* The idea here is to see if the "main" modifier
    * for Alt+Tab has been pressed/released. So if the binding
@@ -1290,20 +1319,62 @@ keycode_is_primary_modifier (MetaDisplay *display,
                            ShiftMask, LockMask };
 
   int i;
-
-  meta_topic (META_DEBUG_KEYBINDINGS,
-              "Checking whether code 0x%x is the primary modifier of mask 0x%x\n",
-              keycode, entire_binding_mask);
   
   i = 0;
   while (i < (int) G_N_ELEMENTS (masks))
     {
       if (entire_binding_mask & masks[i])
-        return is_specific_modifier (display, keycode, masks[i]);
+        return masks[i];
       ++i;
     }
 
-  return FALSE;
+  return 0;
+}
+
+static gboolean
+keycode_is_primary_modifier (MetaDisplay *display,
+                             unsigned int keycode,
+                             unsigned int entire_binding_mask)
+{
+  unsigned int primary_modifier;
+
+  meta_topic (META_DEBUG_KEYBINDINGS,
+              "Checking whether code 0x%x is the primary modifier of mask 0x%x\n",
+              keycode, entire_binding_mask);
+  
+  primary_modifier = get_primary_modifier (display, entire_binding_mask);
+  if (primary_modifier != 0)
+    return is_specific_modifier (display, keycode, primary_modifier);
+  else
+    return FALSE;
+}
+
+static gboolean
+primary_modifier_still_pressed (MetaDisplay *display,
+                                unsigned int entire_binding_mask)
+{
+  unsigned int primary_modifier;
+  int x, y, root_x, root_y;
+  Window root, child;
+  guint mask;
+  
+  primary_modifier = get_primary_modifier (display, entire_binding_mask);
+  
+  XQueryPointer (display->xdisplay,
+                 display->no_focus_window, /* some random window */
+                 &root, &child,
+                 &root_x, &root_y,
+                 &x, &y,
+                 &mask);
+
+  meta_topic (META_DEBUG_KEYBINDINGS,
+              "Primary modifier 0x%x full grab mask 0x%x current state 0x%x\n",
+              primary_modifier, entire_binding_mask, mask);
+  
+  if ((mask & primary_modifier) == 0)
+    return FALSE;
+  else
+    return TRUE;
 }
 
 static const MetaKeyHandler*
@@ -2558,15 +2629,30 @@ do_choose_window (MetaDisplay    *display,
                                            binding->mask,
                                            event->xkey.time,
                                            0, 0))
-        {      
-          meta_ui_tab_popup_select (screen->tab_popup,
-                                    (MetaTabEntryKey) initial_selection->xwindow);
-          
-          if (show_popup)
-            meta_ui_tab_popup_set_showing (screen->tab_popup,
-                                           TRUE);
+        {
+          if (!primary_modifier_still_pressed (display,
+                                               binding->mask))
+            {
+              /* This handles a race where modifier might be released
+               * 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",
+                          initial_selection->desc);
+              meta_display_end_grab_op (display, event->xkey.time);
+              meta_window_activate (initial_selection, event->xkey.time);
+            }
           else
-            meta_window_raise (initial_selection);
+            {
+              meta_ui_tab_popup_select (screen->tab_popup,
+                                        (MetaTabEntryKey) initial_selection->xwindow);
+              
+              if (show_popup)
+                meta_ui_tab_popup_set_showing (screen->tab_popup,
+                                               TRUE);
+              else
+                meta_window_raise (initial_selection);
+            }
         }
     }
 }
@@ -2882,6 +2968,7 @@ handle_workspace_switch  (MetaDisplay    *display,
                           MetaKeyBinding *binding)
 {
   int motion;
+  unsigned int grab_mask;
      
   motion = GPOINTER_TO_INT (binding->handler->data);
 
@@ -2890,31 +2977,49 @@ handle_workspace_switch  (MetaDisplay    *display,
   meta_topic (META_DEBUG_KEYBINDINGS,
               "Starting tab between workspaces, showing popup\n");
 
+  /* FIXME should we use binding->mask ? */
+  grab_mask = event->xkey.state & ~(display->ignored_modifier_mask);
+  
   if (meta_display_begin_grab_op (display,
                                   screen,
                                   NULL,
                                   META_GRAB_OP_KEYBOARD_WORKSPACE_SWITCHING,
                                   FALSE,
                                   0,
-                                  event->xkey.state & ~(display->ignored_modifier_mask),
+                                  grab_mask,
                                   event->xkey.time,
                                   0, 0))
     {
       MetaWorkspace *next;
-
+      gboolean grabbed_before_release;
+      
       next = meta_workspace_get_neighbor (screen->active_workspace, motion);
       g_assert (next); 
 
+      grabbed_before_release = primary_modifier_still_pressed (display, grab_mask);
       
       meta_topic (META_DEBUG_KEYBINDINGS,
 		  "Activating target workspace\n");
+
+      if (!grabbed_before_release)
+        {
+          /* end the grab right away, modifier possibly released
+           * before we could establish the grab and receive the
+           * release event. Must end grab before we can switch
+           * spaces.
+           */
+          meta_display_end_grab_op (display, event->xkey.time);
+        }
       
       switch_to_workspace (display, next);
-      
-      meta_ui_tab_popup_select (screen->tab_popup, (MetaTabEntryKey) next);
-      
-      /* only after selecting proper space */
-      meta_ui_tab_popup_set_showing (screen->tab_popup, TRUE);
+
+      if (grabbed_before_release)
+        {
+          meta_ui_tab_popup_select (screen->tab_popup, (MetaTabEntryKey) next);
+          
+          /* only after selecting proper space */
+          meta_ui_tab_popup_set_showing (screen->tab_popup, TRUE);
+        }
     }
 }