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.

	(change will not be in 2.2.0)

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

        Attempt to fix #85916 (change will not be in 2.2.0)

	* 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, 191 insertions, 44 deletions

diff --git a/ChangeLog b/ChangeLog
index d668a463..3b198d12 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,24 @@
+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.
+
+	(change will not be in 2.2.0)
+	
+2003-01-20  Havoc Pennington  <hp@redhat.com>
+
+        Attempt to fix #85916 (change will not be in 2.2.0)
+	
+	* 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-20  Havoc Pennington  <hp@pobox.com>
 
 	* NEWS: update
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);
+        }
     }
 }