remove XSync, error traps already do that

2001-08-19  Havoc Pennington  <hp@pobox.com>

	* src/display.c (meta_display_grab_window_buttons): remove XSync,
	error traps already do that
	(meta_display_grab_window_buttons): implement

	* src/keybindings.c:
	src/display.c: wire up the tab window, it rulez!

ChangeLog

@@ -1,3 +1,12 @@
+2001-08-19  Havoc Pennington  <hp@pobox.com>
+
+	* src/display.c (meta_display_grab_window_buttons): remove XSync,
+	error traps already do that
+	(meta_display_grab_window_buttons): implement
+
+	* src/keybindings.c: 
+	src/display.c: wire up the tab window, it rulez!
+
 2001-08-19  Havoc Pennington  <hp@pobox.com>
 
 	* src/tabpopup.c: add prototype thingy to display windows we're

README

@@ -194,9 +194,12 @@ METACITY BUGS, NON-FEATURES, AND CAVEATS
 
  - Should support click-to-focus as an option.
 
- - Windows has a neat way of implementing Alt-Tab for 
+ - There is some sort of race condition with Alt-Tab that can
-   window cycling that I would like to copy. (The little 
+   sometimes result in the focus not moving.
-   popup window thing.)
+
+ - The focus rectangle around the icons in the Alt-Tab popup
+   is ugly, should use a GTK-like focus rectangle, or maybe
+   make it look selected, not sure.
 
  - Should Metacity support flipping in right-to-left locales?
    I don't know what window managers look like in a right-to-left

src/common.h

@@ -67,9 +67,13 @@ typedef void (* MetaWindowMenuFunc) (MetaWindowMenu *menu,
                                      int             workspace,
                                      gpointer        data);
 
+/* when changing this enum, there are various switch statements
+ * you have to update
+ */
 typedef enum
 {
   META_GRAB_OP_NONE,
+
   /* Mouse ops */
   META_GRAB_OP_MOVING,
   META_GRAB_OP_RESIZING_SE,
@@ -80,6 +84,7 @@ typedef enum
   META_GRAB_OP_RESIZING_NW,
   META_GRAB_OP_RESIZING_W,
   META_GRAB_OP_RESIZING_E,
+
   /* Keyboard ops */
   META_GRAB_OP_KEYBOARD_MOVING,
   META_GRAB_OP_KEYBOARD_RESIZING_UNKNOWN,
@@ -87,6 +92,13 @@ typedef enum
   META_GRAB_OP_KEYBOARD_RESIZING_N,
   META_GRAB_OP_KEYBOARD_RESIZING_W,
   META_GRAB_OP_KEYBOARD_RESIZING_E,
+  META_GRAB_OP_KEYBOARD_RESIZING_SE,
+  META_GRAB_OP_KEYBOARD_RESIZING_NE,
+  META_GRAB_OP_KEYBOARD_RESIZING_SW,
+  META_GRAB_OP_KEYBOARD_RESIZING_NW,
+
+  META_GRAB_OP_KEYBOARD_TABBING,
+  
   /* Frame button ops */
   META_GRAB_OP_CLICKING_MINIMIZE,
   META_GRAB_OP_CLICKING_MAXIMIZE,

src/display.c

@@ -592,6 +592,11 @@ grab_op_is_keyboard (MetaGrabOp op)
     case META_GRAB_OP_KEYBOARD_RESIZING_N:
     case META_GRAB_OP_KEYBOARD_RESIZING_W:
     case META_GRAB_OP_KEYBOARD_RESIZING_E:
+    case META_GRAB_OP_KEYBOARD_RESIZING_SE:
+    case META_GRAB_OP_KEYBOARD_RESIZING_NE:
+    case META_GRAB_OP_KEYBOARD_RESIZING_SW:
+    case META_GRAB_OP_KEYBOARD_RESIZING_NW:
+    case META_GRAB_OP_KEYBOARD_TABBING:
       return TRUE;
       break;
 
@@ -1240,6 +1245,7 @@ meta_spew_event (MetaDisplay *display,
           name = "MappingNotify";
           break;
         default:
+          meta_verbose ("Unknown event type %d\n", event->xany.type);
           name = "Unknown event type";
           break;
         }
@@ -1481,27 +1487,16 @@ meta_display_begin_grab_op (MetaDisplay *display,
       return FALSE;
     }
 
-  switch (op)
+  if (grab_op_is_keyboard (op))
     {
-    case META_GRAB_OP_KEYBOARD_MOVING:
-    case META_GRAB_OP_KEYBOARD_RESIZING_UNKNOWN:
-    case META_GRAB_OP_KEYBOARD_RESIZING_S:
-    case META_GRAB_OP_KEYBOARD_RESIZING_N:
-    case META_GRAB_OP_KEYBOARD_RESIZING_W:
-    case META_GRAB_OP_KEYBOARD_RESIZING_E:
       if (meta_window_grab_all_keys (window))
         display->grab_have_keyboard = TRUE;
       
       if (!display->grab_have_keyboard)
         {
-          meta_verbose ("XGrabKeyboard() failed\n");
+          meta_verbose ("grabbing all keys failed\n");
           return FALSE;
         }
-      break;
-
-    default:
-      /* non-keyboard grab ops */
-      break;
     }
   
   display->grab_op = op;
@@ -1520,6 +1515,10 @@ meta_display_begin_grab_op (MetaDisplay *display,
   g_assert (display->grab_window != NULL);
   g_assert (display->grab_op != META_GRAB_OP_NONE);
 
+  /* Do this last, after everything is set up. */
+  if (op == META_GRAB_OP_KEYBOARD_TABBING)
+    meta_screen_ensure_tab_popup (window->screen);
+  
   return TRUE;
 }
 
@@ -1530,6 +1529,12 @@ meta_display_end_grab_op (MetaDisplay *display,
   if (display->grab_op == META_GRAB_OP_NONE)
     return;
 
+  if (display->grab_op == META_GRAB_OP_KEYBOARD_TABBING)
+    {
+      meta_ui_tab_popup_free (display->grab_window->screen->tab_popup);
+      display->grab_window->screen->tab_popup = NULL;
+    }
+  
   if (display->grab_have_pointer)
     XUngrabPointer (display->xdisplay, timestamp);    
 
@@ -1549,6 +1554,11 @@ meta_display_grab_window_buttons (MetaDisplay *display,
    */
   meta_verbose ("Grabbing window buttons for 0x%lx\n", xwindow);
 
+  /* FIXME If we ignored errors here instead of spewing, we could
+   * put one big error trap around the loop and avoid a bunch of
+   * XSync()
+   */
+  
   {
     int i = 1;
     while (i < 4)
@@ -1562,11 +1572,10 @@ meta_display_grab_window_buttons (MetaDisplay *display,
                      PointerMotionMask | PointerMotionHintMask,
                      GrabModeAsync, GrabModeAsync,
                      False, None);
-        XSync (display->xdisplay, False);
         result = meta_error_trap_pop (display);
 
         if (result != Success)
-          meta_warning ("Failed to grab button %d with Mod1Mask for frame 0x%lx error code %d\n",
+          meta_warning ("Failed to grab button %d with Mod1Mask for window 0x%lx error code %d\n",
                         i, xwindow, result);
         
 
@@ -1582,11 +1591,10 @@ meta_display_grab_window_buttons (MetaDisplay *display,
                                   PointerMotionMask | PointerMotionHintMask,
                                   GrabModeAsync, GrabModeAsync,
                                   False, None);
-            XSync (display->xdisplay, False);
             result = meta_error_trap_pop (display);
             
             if (result != Success)
-              meta_warning ("Failed to grab button %d with ControlMask for frame 0x%lx error code %d\n",
+              meta_warning ("Failed to grab button %d with ControlMask for window 0x%lx error code %d\n",
                             i, xwindow, result);
           }
         
@@ -1599,7 +1607,32 @@ void
 meta_display_ungrab_window_buttons  (MetaDisplay *display,
                                      Window       xwindow)
 {
+  /* FIXME If we ignored errors here instead of spewing, we could
+   * put one big error trap around the loop and avoid a bunch of
+   * XSync()
+   */
+  int i = 1;
+  while (i < 4)
+    {
+      int result;
       
+      meta_error_trap_push (display);
+      XUngrabButton (display->xdisplay, i, Mod1Mask, xwindow);
+      result = meta_error_trap_pop (display);
 
+      if (result != Success)
+        meta_warning ("Failed to ungrab button %d with Mod1Mask for window 0x%lx error code %d\n",
+                      i, xwindow, result);
+
+      meta_error_trap_push (display);
+      XUngrabButton (display->xdisplay, i, ControlMask, xwindow);
+      result = meta_error_trap_pop (display);
+      
+      if (result != Success)
+        meta_warning ("Failed to ungrab button %d with ControlMask for window 0x%lx error code %d\n",
+                      i, xwindow, result);
+      
+      ++i;
+    }
 }
 

src/keybindings.c

@@ -66,6 +66,16 @@ static void handle_workspace_right     (MetaDisplay *display,
                                         XEvent      *event,
                                         gpointer     data);
 
+static gboolean process_keyboard_move_grab (MetaDisplay *display,
+                                            MetaWindow  *window,
+                                            XEvent      *event,
+                                            KeySym       keysym);
+
+static gboolean process_tab_grab           (MetaDisplay *display,
+                                            MetaWindow  *window,
+                                            XEvent      *event,
+                                            KeySym       keysym);
+
 typedef struct _MetaKeyBinding MetaKeyBinding;
 
 struct _MetaKeyBinding
@@ -281,6 +291,26 @@ meta_window_grab_all_keys (MetaWindow  *window)
     }
   else
     {
+      /* Also grab the keyboard, so we get key releases and all key
+       * presses
+       */
+       meta_error_trap_push (window->display);
+       /* FIXME CurrentTime bogus */
+       XGrabKeyboard (window->display->xdisplay,
+                      grabwindow, True,
+                      GrabModeAsync, GrabModeAsync,
+                      CurrentTime);
+       
+       result = meta_error_trap_pop (window->display);
+       if (result != Success)
+         {
+           meta_verbose ("XGrabKeyboard() failed for window %s\n",
+                         window->desc);
+           return FALSE;
+         }
+       
+       meta_verbose ("Grabbed all keys on window %s\n", window->desc);
+       
        window->keys_grabbed = FALSE;
        window->all_keys_grabbed = TRUE;
        window->grab_on_frame = window->frame != NULL;
@@ -302,6 +332,8 @@ meta_window_ungrab_all_keys (MetaWindow  *window)
       XUngrabKey (window->display->xdisplay,
                   AnyKey, AnyModifier,
                   grabwindow);
+      /* FIXME CurrentTime bogus */
+      XUngrabKeyboard (window->display->xdisplay, CurrentTime);
       meta_error_trap_pop (window->display);
       
       window->grab_on_frame = FALSE;
@@ -328,9 +360,41 @@ is_modifier (MetaDisplay *display,
   
   map_size = 8 * mod_keymap->max_keypermod;
   i = 0;
-	while (i < map_size) {
+  while (i < map_size)
+    {
+      if (keycode == mod_keymap->modifiermap[i])
+        {
+          retval = TRUE;
+          break;
+        }
+      ++i;
+    }
   
-		if (keycode == mod_keymap->modifiermap[i]) {
+  XFreeModifiermap (mod_keymap);
+  
+  return retval;
+}
+
+#define MOD1_INDEX 3 /* shift, lock, control, mod1 */
+static gboolean 
+is_mod1_key (MetaDisplay *display,
+             unsigned int keycode)
+{
+  int i;
+  int end;
+  XModifierKeymap *mod_keymap;
+  gboolean retval = FALSE;
+  
+  /* FIXME this is ass-slow, cache the modmap */
+  
+  mod_keymap = XGetModifierMapping (display->xdisplay);
+  
+  end = (MOD1_INDEX + 1) * mod_keymap->max_keypermod;
+  i = MOD1_INDEX * mod_keymap->max_keypermod;
+  while (i < end)
+    {
+      if (keycode == mod_keymap->modifiermap[i])
+        {
           retval = TRUE;
           break;
         }
@@ -384,43 +448,75 @@ meta_display_process_key_event (MetaDisplay *display,
                 XKeysymToString (keysym), event->xkey.state,
                 window ? window->desc : "(no window)");
 
-  if (window == NULL || !window->all_keys_grabbed)
+  if (display->grab_op == META_GRAB_OP_NONE &&
+      (window == NULL || !window->all_keys_grabbed))
     {
       /* Do the normal keybindings */
       process_event (screen_bindings, display, NULL, event, keysym);
 
       if (window)
         process_event (window_bindings, display, window, event, keysym);
+
       return;
     }
   
+  if (display->grab_op == META_GRAB_OP_NONE)
+    return;    
+
   /* If we get here we have a global grab, because
    * we're in some special keyboard mode such as window move
    * mode.
    */
   
-  if (display->grab_op == META_GRAB_OP_NONE)
-    return;
-    
-  /* don't end grabs on modifier key presses */
-  if (is_modifier (display, event->xkey.keycode))
-    return;
-  
   handled = FALSE;
 
-  if (display->grab_op == META_GRAB_OP_KEYBOARD_MOVING &&
+  if (window == display->grab_window)
-      display->grab_window == window)
     {
+      switch (display->grab_op)
+        {
+        case META_GRAB_OP_KEYBOARD_MOVING:
+          meta_verbose ("Processing event for keyboard move\n");
+          handled = process_keyboard_move_grab (display, window, event, keysym);
+          break;
+        case META_GRAB_OP_KEYBOARD_TABBING:
+          meta_verbose ("Processing event for keyboard tabbing\n");
+          handled = process_tab_grab (display, window, event, keysym);
+          break;
+        default:
+          break;
+        }
+    }
+  
+  /* end grab if a key that isn't used gets pressed */
+  if (!handled)
+    {
+      meta_verbose ("Ending grab op %d on key event sym %s\n",
+                    display->grab_op, XKeysymToString (keysym));
+      meta_display_end_grab_op (display, event->xkey.time);
+    }
+}
+
+static gboolean
+process_keyboard_move_grab (MetaDisplay *display,
+                            MetaWindow  *window,
+                            XEvent      *event,
+                            KeySym       keysym)
+{
+  gboolean handled;
   int x, y;
   int incr;
   gboolean smart_snap;
   int edge;
   
-      if (event->type == KeyRelease)
+  handled = FALSE;
-        return; /* don't care about releases */
 
-      if (window == NULL)
+  /* don't care about releases, but eat them, don't end grab */
-        meta_bug ("NULL window while doing keyboard grab op\n");
+  if (event->type == KeyRelease)
+    return TRUE;
+
+  /* don't end grab on modifier key presses */
+  if (is_modifier (display, event->xkey.keycode))
+    return TRUE;
 
   meta_window_get_position (window, &x, &y);
 
@@ -498,15 +594,82 @@ meta_display_process_key_event (MetaDisplay *display,
 
   if (handled)
     meta_window_move (window, x, y);
+
+  return handled;
+}
+
+static gboolean
+process_tab_grab (MetaDisplay *display,
+                  MetaWindow  *window,
+                  XEvent      *event,
+                  KeySym       keysym)
+{
+  MetaScreen *screen;
+
+  window = NULL; /* be sure we don't use this, it's irrelevant */
+
+  screen = display->grab_window->screen;
+
+  g_return_val_if_fail (screen->tab_popup != NULL, FALSE);
+  
+  if (event->type == KeyRelease &&
+      is_mod1_key (display, event->xkey.keycode))
+    {
+      /* We're done, move to the new window. */
+      Window target_xwindow;
+      MetaWindow *target_window;
+
+      target_xwindow =
+        meta_ui_tab_popup_get_selected (screen->tab_popup);
+      target_window =
+        meta_display_lookup_x_window (display, target_xwindow);
+
+      meta_verbose ("Ending tab operation, Mod1 released\n");
+      
+      if (target_window)
+        {
+          meta_verbose ("Ending grab early so we can focus the target window\n");
+          meta_display_end_grab_op (display, event->xkey.time);
+
+          meta_verbose ("Focusing target window\n");
+          meta_window_raise (target_window);
+          meta_window_focus (target_window, event->xkey.time);
+
+          return TRUE; /* we already ended the grab */
         }
       
-  /* end grab if a key that isn't used gets pressed */
+      return FALSE; /* end grab */
-  if (!handled)
-    {
-      meta_verbose ("Ending grab op %d on key press event sym %s\n",
-                    display->grab_op, XKeysymToString (keysym));
-      meta_display_end_grab_op (display, event->xkey.time);
     }
+  
+  /* don't care about other releases, but eat them, don't end grab */
+  if (event->type == KeyRelease)
+    return TRUE;
+
+  /* don't end grab on modifier key presses */
+  if (is_modifier (display, event->xkey.keycode))
+    return TRUE;
+
+  switch (keysym)
+    {
+    case XK_ISO_Left_Tab:
+    case XK_Tab:
+      if (event->xkey.state & ShiftMask)
+        meta_ui_tab_popup_backward (screen->tab_popup);
+      else
+        meta_ui_tab_popup_forward (screen->tab_popup);
+
+      /* continue grab */
+      meta_verbose ("Tab key pressed, moving tab focus in popup\n");
+      return TRUE;
+      break;
+
+    default:
+      break;
+    }
+
+  /* end grab */
+  meta_verbose ("Ending tabbing, uninteresting key pressed\n");
+  return FALSE;
 }
 
 static void
@@ -694,8 +857,22 @@ handle_tab_forward (MetaDisplay *display,
 
   if (window)
     {
-      meta_window_raise (window);
+      meta_verbose ("Starting tab forward, showing popup\n");
-      meta_window_focus (window, event->xkey.time);
+
+      meta_display_begin_grab_op (window->display,
+                                  display->focus_window ?
+                                  display->focus_window : window,
+                                  META_GRAB_OP_KEYBOARD_TABBING,
+                                  FALSE,
+                                  0, 0,
+                                  event->xkey.time,
+                                  0, 0);
+
+      meta_ui_tab_popup_select (window->screen->tab_popup,
+                                window->xwindow);
+      /* only after selecting proper window */
+      meta_ui_tab_popup_set_showing (window->screen->tab_popup,
+                                     TRUE);
     }
 }
 
@@ -738,8 +915,22 @@ handle_tab_backward (MetaDisplay *display,
 
   if (window)
     {
-      meta_window_raise (window);
+      meta_verbose ("Starting tab backward, showing popup\n");
-      meta_window_focus (window, event->xkey.time);
+      
+      meta_display_begin_grab_op (window->display,
+                                  display->focus_window ?
+                                  display->focus_window : window,
+                                  META_GRAB_OP_KEYBOARD_TABBING,
+                                  FALSE,
+                                  0, 0,
+                                  event->xkey.time,
+                                  0, 0);
+
+      meta_ui_tab_popup_select (window->screen->tab_popup,
+                                window->xwindow);
+      /* only after selecting proper window */
+      meta_ui_tab_popup_set_showing (window->screen->tab_popup,
+                                     TRUE);
     }
 }
 

src/screen.c

@@ -198,6 +198,8 @@ meta_screen_new (MetaDisplay *display,
   screen->ui = meta_ui_new (screen->display->xdisplay,
                             screen->xscreen);
 
+  screen->tab_popup = NULL;
+  
   screen->stack = meta_stack_new (screen);
   
   meta_verbose ("Added screen %d ('%s') root 0x%lx\n",
@@ -423,3 +425,45 @@ meta_screen_set_cursor (MetaScreen *screen,
   XDefineCursor (screen->display->xdisplay, screen->xroot, xcursor);
   XFreeCursor (screen->display->xdisplay, xcursor);
 }
+
+void
+meta_screen_ensure_tab_popup (MetaScreen *screen)
+{
+  MetaTabEntry *entries;
+  GSList *tab_list;
+  GSList *tmp;
+  int len;
+  int i;
+  
+  if (screen->tab_popup)
+    return;
+
+  tab_list = meta_stack_get_tab_list (screen->stack);
+  len = g_slist_length (tab_list);
+
+  entries = g_new (MetaTabEntry, len + 1);
+  entries[len].xwindow = None;
+  entries[len].title = NULL;
+  entries[len].icon = NULL;
+  
+  i = 0;
+  tmp = tab_list;
+  while (i < len)
+    {
+      MetaWindow *window;
+
+      window = tmp->data;
+      
+      entries[i].xwindow = window->xwindow;
+      entries[i].title = window->title;
+      entries[i].icon = window->icon;
+      
+      ++i;
+      tmp = tmp->next;
+    }
+  
+  screen->tab_popup = meta_ui_tab_popup_new (entries);
+  g_free (entries);
+
+  /* don't show tab popup, since proper window isn't selected yet */
+}

src/screen.h

@@ -39,6 +39,7 @@ struct _MetaScreen
   int width;
   int height;
   MetaUI *ui;
+  MetaTabPopup *tab_popup;
   
   MetaWorkspace *active_workspace;
 
@@ -63,6 +64,8 @@ int           meta_screen_get_n_workspaces    (MetaScreen                 *scree
 void          meta_screen_set_cursor          (MetaScreen                 *screen,
                                                MetaCursor                  cursor);
 
+void          meta_screen_ensure_tab_popup    (MetaScreen                 *screen);
+
 #endif
 
 

src/stack.c

@@ -902,6 +902,41 @@ meta_stack_get_tab_next (MetaStack  *stack,
     return find_tab_forward (stack, NULL, -1);
 }
 
+GSList*
+meta_stack_get_tab_list (MetaStack *stack)
+{
+  GSList *list;
+  int i;
+  
+  list = NULL;
+  
+  i = 0;
+  while (i < stack->windows->len)
+    {
+      MetaWindow *window;
+      MetaWorkspace *workspace;
+      
+      window = meta_display_lookup_x_window (stack->screen->display,
+                                             GET_XWINDOW (stack, i));
+
+      if (window)
+        workspace = window->screen->active_workspace;
+      else
+        workspace = NULL;
+      
+      if (window && IN_TAB_CHAIN (window) &&
+          (workspace == NULL ||
+           meta_workspace_contains_window (workspace, window)))
+        list = g_slist_prepend (list, window);
+
+      ++i;
+    }
+
+  list = g_slist_reverse (list);
+
+  return list;
+}
+
 int
 meta_stack_windows_cmp  (MetaStack  *stack,
                          MetaWindow *window_a,

src/stack.h

@@ -95,6 +95,7 @@ MetaWindow* meta_stack_get_below  (MetaStack  *stack,
 MetaWindow* meta_stack_get_tab_next (MetaStack  *stack,
                                      MetaWindow *window,
                                      gboolean    backward);
+GSList*     meta_stack_get_tab_list (MetaStack  *stack);
 /* -1 if a < b, etc. */
 int         meta_stack_windows_cmp  (MetaStack  *stack,
                                      MetaWindow *window_a,

src/tabpopup.c

@@ -40,6 +40,7 @@ struct _MetaTabPopup
   GtkWidget *label;
   GList *current;
   GList *entries;
+  GtkWidget *current_selected_widget;
 };
 
 MetaTabPopup*
@@ -52,11 +53,18 @@ meta_ui_tab_popup_new (const MetaTabEntry *entries)
   int height;
   GtkWidget *table;
   GList *tmp;
+  GtkWidget *frame;
   
   popup = g_new (MetaTabPopup, 1);
   popup->window = gtk_window_new (GTK_WINDOW_POPUP);
+  gtk_window_set_position (GTK_WINDOW (popup->window),
+                           GTK_WIN_POS_CENTER_ALWAYS);
+  /* enable resizing, to get never-shrink behavior */
+  gtk_window_set_resizable (GTK_WINDOW (popup->window),
+                            TRUE);
   popup->current = NULL;
   popup->entries = NULL;
+  popup->current_selected_widget = NULL;
   
   tab_entries = NULL;
   i = 0;
@@ -85,32 +93,51 @@ meta_ui_tab_popup_new (const MetaTabEntry *entries)
 
   table = gtk_table_new (height + 1, width, FALSE);
   
+  frame = gtk_frame_new (NULL);
+  gtk_frame_set_shadow_type (GTK_FRAME (frame), GTK_SHADOW_OUT);
+  gtk_container_set_border_width (GTK_CONTAINER (table), 1);
+  gtk_container_add (GTK_CONTAINER (popup->window),
+                     frame);
+  gtk_container_add (GTK_CONTAINER (frame),
+                     table);
+
+  
   popup->label = gtk_label_new ("");
   
   gtk_table_attach (GTK_TABLE (table),
                     popup->label,
-                    0, width,   height - 1, height,
+                    0, width,              height, height + 1,
-                    0,          0,
+                    GTK_EXPAND | GTK_FILL, GTK_EXPAND | GTK_FILL,
-                    0,          0);
+                    0,                     2);
+  
 
-  left = 0;
-  right = 1;
   top = 0;
   bottom = 1;
   tmp = popup->entries;
   
   while (tmp && top < height)
     {      
+      left = 0;
+      right = 1;
+
       while (tmp && left < width)
         {
           GtkWidget *image;
+          GtkWidget *highlight;
+          
           TabEntry *te;
 
           te = tmp->data;
 
+          highlight = gtk_frame_new (NULL);
+          gtk_frame_set_shadow_type (GTK_FRAME (highlight),
+                                     GTK_SHADOW_NONE);
           image = gtk_image_new_from_pixbuf (te->icon);
+          gtk_misc_set_padding (GTK_MISC (image), 3, 3);
           
-          te->widget = image;
+          gtk_container_add (GTK_CONTAINER (highlight), image);
+          
+          te->widget = highlight;
 
           gtk_table_attach (GTK_TABLE (table),
                             te->widget,
@@ -121,9 +148,11 @@ meta_ui_tab_popup_new (const MetaTabEntry *entries)
           tmp = tmp->next;
           
           ++left;
+          ++right;
         }
       
       ++top;
+      ++bottom;
     }
       
   return popup;
@@ -169,7 +198,16 @@ static void
 display_entry (MetaTabPopup *popup,
                TabEntry     *te)
 {
+  if (popup->current_selected_widget)
+    {
+      gtk_frame_set_shadow_type (GTK_FRAME (popup->current_selected_widget),
+                                 GTK_SHADOW_NONE);
+    }
+  
   gtk_label_set_text (GTK_LABEL (popup->label), te->title);
+  gtk_frame_set_shadow_type (GTK_FRAME (te->widget),
+                             GTK_SHADOW_ETCHED_IN);
+  popup->current_selected_widget = te->widget;
 }
 
 void

src/ui.c

@@ -345,3 +345,12 @@ meta_ui_pop_delay_exposes  (MetaUI *ui)
   meta_frames_pop_delay_exposes (ui->frames);
 }
 
+GdkPixbuf*
+meta_ui_get_default_window_icon (MetaUI *ui)
+{
+  /* FIXME */
+  return gtk_widget_render_icon (GTK_WIDGET (ui->frames),
+                                 GTK_STOCK_NEW,
+                                 GTK_ICON_SIZE_LARGE_TOOLBAR,
+                                 NULL);
+}

src/ui.h

@@ -117,6 +117,8 @@ GdkPixbuf* meta_gdk_pixbuf_get_from_window (GdkPixbuf   *dest,
 void      meta_ui_push_delay_exposes (MetaUI *ui);
 void      meta_ui_pop_delay_exposes  (MetaUI *ui);
 
+GdkPixbuf* meta_ui_get_default_window_icon (MetaUI *ui);
+
 typedef struct _MetaTabEntry MetaTabEntry;
 typedef struct _MetaTabPopup MetaTabPopup;
 

src/window.c

@@ -653,6 +653,9 @@ meta_window_free (MetaWindow  *window)
   
   meta_error_trap_pop (window->display);
 
+  if (window->icon)
+    g_object_unref (G_OBJECT (window->icon));
+  
   g_free (window->sm_client_id);
   g_free (window->role);
   g_free (window->res_class);
@@ -3500,14 +3503,20 @@ update_icon (MetaWindow *window)
 {  
   meta_error_trap_push (window->display);
 
+#if 0
   if (window->icon)
     {
       g_object_unref (G_OBJECT (window->icon));
       window->icon = NULL;
     }
+#endif
   
   /* FIXME */
 
+  /* Fallback */
+  if (window->icon == NULL)
+    window->icon = meta_ui_get_default_window_icon (window->screen->ui);
+  
   return meta_error_trap_pop (window->display);
 }