Merge lp:~3v1n0/bamf/better-wmclass-filter into lp:bamf/0.4

[Marco Trevisan (Treviño)]

BamfMatcher: use both instance and class names for matching WMClass

Also, filter out the .desktop files that have a defined StartupWMClass
that doesn't match with our .desktop file.

We use both WMClass instance and class name to match proper .desktop files, also we filter out from possible_desktop list the .desktop files that have defined a wmclass that does not match with the one of the target window.

This allow to make both the custom applications .desktop files to work correctly and the web applications to be matched as new ones.

Plus, favorites are used only if their class matches.

[Jason Smith (jassmith)]

Looks good, nothing too scary.

review: Approve

[Unity Merger (unity-merger)]

The prerequisite lp:~3v1n0/bamf/desktopless-webapps-fix has not yet been merged into lp:bamf.

[Unity Merger (unity-merger)]

Attempt to merge into lp:bamf failed due to conflicts:

text conflict in src/bamf-matcher.c

[lp:~3v1n0/bamf/better-wmclass-filter]

479. By Marco Trevisan (Treviño) on 2012-05-22

Merging with trunk

=== modified file 'src/bamf-application.c'
--- src/bamf-application.c	2012-04-26 06:16:01 +0000
+++ src/bamf-application.c	2012-05-22 16:24:22 +0000
@@ -120,17 +120,20 @@
   if (self->priv->desktop_file)
     {
       keyfile = g_key_file_new();
-      if (!g_key_file_load_from_file(keyfile, self->priv->desktop_file, G_KEY_FILE_NONE, NULL)) {
+
+      if (!g_key_file_load_from_file(keyfile, self->priv->desktop_file, G_KEY_FILE_NONE, NULL))
+        {
           g_key_file_free(keyfile);
-        return;
-      }
+          return;
+        }
 
       desktop = g_desktop_app_info_new_from_keyfile (keyfile);
 
-      if (!G_IS_APP_INFO (desktop)) {
-        g_key_file_free(keyfile);
-        return;
-      }
+      if (!G_IS_APP_INFO (desktop))
+        {
+          g_key_file_free(keyfile);
+          return;
+        }
 
       gicon = g_app_info_get_icon (G_APP_INFO (desktop));
 
@@ -139,14 +142,15 @@
       if (gicon)
         icon = g_icon_to_string (gicon);
 
-      if (g_key_file_has_key(keyfile, G_KEY_FILE_DESKTOP_GROUP, STUB_KEY, NULL)) {
-        /* This will error to return false, which is okay as it seems
-           unlikely anyone will want to set this flag except to turn
-           off the stub menus. */
-        self->priv->show_stubs = g_key_file_get_boolean (keyfile,
-                                                         G_KEY_FILE_DESKTOP_GROUP,
-                                                         STUB_KEY, NULL);
-      }
+      if (g_key_file_has_key(keyfile, G_KEY_FILE_DESKTOP_GROUP, STUB_KEY, NULL))
+        {
+          /* This will error to return false, which is okay as it seems
+             unlikely anyone will want to set this flag except to turn
+             off the stub menus. */
+          self->priv->show_stubs = g_key_file_get_boolean (keyfile,
+                                                           G_KEY_FILE_DESKTOP_GROUP,
+                                                           STUB_KEY, NULL);
+        }
       
       if (g_key_file_has_key (keyfile, G_KEY_FILE_DESKTOP_GROUP, "X-GNOME-FullName", NULL))
         {
@@ -489,11 +493,12 @@
 }
 
 static char *
-bamf_application_favorite_from_list (BamfApplication *self, GList *list)
+bamf_application_favorite_from_list (BamfApplication *self, GList *desktop_list)
 {
   BamfMatcher *matcher;
   GList *favs, *l;
   char *result = NULL;
+  const char *desktop_class;
 
   g_return_val_if_fail (BAMF_IS_APPLICATION (self), NULL);
 
@@ -504,10 +509,15 @@
     {
       for (l = favs; l; l = l->next)
         {
-          if (g_list_find_custom (list, l->data, (GCompareFunc) g_strcmp0))
+          if (g_list_find_custom (desktop_list, l->data, (GCompareFunc) g_strcmp0))
             {
-              result = l->data;
-              break;
+              desktop_class = bamf_matcher_get_desktop_file_class (matcher, l->data);
+
+              if (!desktop_class || g_strcmp0 (self->priv->wmclass, desktop_class) == 0)
+                {
+                  result = l->data;
+                  break;
+                }
             }
         }
     }
 
=== modified file 'src/bamf-matcher.c'
--- src/bamf-matcher.c	2012-05-22 15:10:26 +0000
+++ src/bamf-matcher.c	2012-05-22 16:24:22 +0000
@@ -659,9 +659,11 @@
 }
 
 static gboolean
-exec_string_should_be_processed (BamfMatcher *self,
-                                 char *exec)
+exec_string_should_be_processed (const char *exec)
 {
+  if (!exec)
+    return TRUE;
+
   return !g_str_has_prefix (exec, "ooffice") && !g_str_has_prefix (exec, "libreoffice");
 }
 
@@ -829,7 +831,7 @@
       return;
     }
 
-  if (exec_string_should_be_processed (self, exec))
+  if (exec_string_should_be_processed (exec))
     {
       /**
        * Set of nasty hacks which should be removed some day. We wish to keep the full exec
@@ -939,7 +941,7 @@
       gchar **parts = g_strsplit (line, "\t", 3);
       exec = parts[1];
 
-      if (exec_string_should_be_processed (self, exec))
+      if (exec_string_should_be_processed (exec))
         {
           char *tmp = trim_exec_string (self, exec);
           g_free (parts[1]);
@@ -1377,6 +1379,9 @@
 {
   const char *class_name = bamf_legacy_window_get_class_name (window);
 
+  if (!class_name)
+    return FALSE;
+
   return (g_str_has_prefix (class_name, "LibreOffice") ||
           g_str_has_prefix (class_name, "libreoffice") ||
           g_str_has_prefix (class_name, "OpenOffice") ||
@@ -1543,17 +1548,20 @@
 
   gboolean valid_app = FALSE;
 
-  if (g_strcmp0 (window_class, "Google-chrome") == 0 &&
-      g_strcmp0 (instance_name, "google-chrome") != 0 &&
-      !g_str_has_prefix (instance_name, "Google-chrome"))
-    {
-      valid_app = TRUE;
-    }
-  else if (g_strcmp0 (window_class, "Chromium-browser") == 0 &&
-           g_strcmp0 (instance_name, "chromium-browser") != 0 &&
-           !g_str_has_prefix (instance_name, "Chromium-browser"))
-    {
-      valid_app = TRUE;
+  if (instance_name && window_class)
+    {
+      if (g_strcmp0 (window_class, "Google-chrome") == 0 &&
+          g_strcmp0 (instance_name, "google-chrome") != 0 &&
+          !g_str_has_prefix (instance_name, "Google-chrome"))
+        {
+          valid_app = TRUE;
+        }
+      else if (g_strcmp0 (window_class, "Chromium-browser") == 0 &&
+               g_strcmp0 (instance_name, "chromium-browser") != 0 &&
+               !g_str_has_prefix (instance_name, "Chromium-browser"))
+        {
+          valid_app = TRUE;
+        }
     }
 
   return valid_app;
@@ -1620,36 +1628,50 @@
 
 static GList *
 bamf_matcher_possible_applications_for_window (BamfMatcher *self,
-                                               BamfWindow *bamf_window)
+                                               BamfWindow *bamf_window,
+                                               const char **target_class_out)
 {
   BamfMatcherPrivate *priv;
   BamfLegacyWindow *window;
   GList *desktop_files = NULL, *l;
   char *desktop_file = NULL;
-  char *desktop_class = NULL;
+  const char *desktop_class = NULL;
+  const char *class_name = NULL;
+  const char *instance_name = NULL;
+  const char *target_class = NULL;
+  gboolean filter_by_wmclass = FALSE;
 
   g_return_val_if_fail (BAMF_IS_WINDOW (bamf_window), NULL);
   g_return_val_if_fail (BAMF_IS_MATCHER (self), NULL);
 
   priv = self->priv;
   window = bamf_window_get_window (bamf_window);
-
   desktop_file = get_window_hint (window, _NET_WM_DESKTOP_FILE);
-
-  const char *class_name = bamf_legacy_window_get_class_name (window);
-  const char *instance_name = bamf_legacy_window_get_class_instance_name (window);
-  gboolean known_desktop_class = bamf_matcher_has_instance_class_desktop_file (self, instance_name);
-
-  if (!known_desktop_class)
+  class_name = bamf_legacy_window_get_class_name (window);
+  instance_name = bamf_legacy_window_get_class_instance_name (window);
+
+  target_class = instance_name;
+  filter_by_wmclass = bamf_matcher_has_instance_class_desktop_file (self, target_class);
+
+  if (!filter_by_wmclass)
   {
-    known_desktop_class = is_web_app_window (self, window);
+    if (is_web_app_window (self, window))
+      {
+        // This ensures that a new application is created even for unknown webapps
+        filter_by_wmclass = TRUE;
+      }
+    else
+      {
+        target_class = class_name;
+        filter_by_wmclass = bamf_matcher_has_instance_class_desktop_file (self, target_class);
+      }
   }
 
   if (desktop_file)
     {
-      desktop_class = g_hash_table_lookup (priv->desktop_class_table, desktop_file);
+      desktop_class = bamf_matcher_get_desktop_file_class (self, desktop_file);
 
-      if (!known_desktop_class || g_strcmp0 (desktop_class, instance_name) == 0)
+      if ((!filter_by_wmclass && !desktop_class) || g_strcmp0 (desktop_class, target_class) == 0)
         {
           desktop_files = g_list_prepend (desktop_files, desktop_file);
         }
@@ -1675,9 +1697,9 @@
 
               if (desktop_file)
                 {
-                  desktop_class = g_hash_table_lookup (priv->desktop_class_table, desktop_file);
+                  desktop_class = bamf_matcher_get_desktop_file_class (self, desktop_file);
 
-                  if (!known_desktop_class || g_strcmp0 (desktop_class, instance_name) == 0)
+                  if ((!filter_by_wmclass && !desktop_class) || g_strcmp0 (desktop_class, target_class) == 0)
                     {
                       if (!g_list_find_custom (desktop_files, desktop_file,
                                                (GCompareFunc) g_strcmp0))
@@ -1708,10 +1730,10 @@
             {
               gboolean append = FALSE;
 
-              if (instance_name)
+              if (target_class)
                 {
-                  desktop_class = g_hash_table_lookup (priv->desktop_class_table, desktop_file);
-                  if (!known_desktop_class || g_strcmp0 (desktop_class, instance_name) == 0)
+                  desktop_class = bamf_matcher_get_desktop_file_class (self, desktop_file);
+                  if ((!filter_by_wmclass && !desktop_class) || g_strcmp0 (desktop_class, target_class) == 0)
                     {
                       append = TRUE;
                     }
@@ -1753,34 +1775,40 @@
       g_list_free (pid_list);
     }
 
-  if (!desktop_files && known_desktop_class)
-    {
-      desktop_files = bamf_matcher_get_class_matching_desktop_files (self, instance_name);
+  if (!desktop_files && filter_by_wmclass)
+    {
+      desktop_files = bamf_matcher_get_class_matching_desktop_files (self, target_class);
+    }
+
+  if (target_class_out)
+    {
+      *target_class_out = target_class;
     }
 
   return desktop_files;
 }
 
-static void
-bamf_matcher_setup_window_state (BamfMatcher *self,
-                                 BamfWindow *bamf_window)
+static BamfApplication *
+bamf_matcher_get_application_for_window (BamfMatcher *self,
+                                         BamfWindow *bamf_window,
+                                         gboolean *new_application)
 {
   GList *possible_apps, *l;
   BamfLegacyWindow *window;
-  const gchar *app_class;
+  const gchar *win_class_name;
+  const gchar *target_class = NULL;
+  const gchar *app_class = NULL;
   const gchar *app_desktop = NULL;
   BamfApplication *app = NULL, *best = NULL;
 
-  g_return_if_fail (BAMF_IS_MATCHER (self));
-  g_return_if_fail (BAMF_IS_WINDOW (bamf_window));
+  g_return_val_if_fail (BAMF_IS_MATCHER (self), NULL);
+  g_return_val_if_fail (BAMF_IS_WINDOW (bamf_window), NULL);
 
   window = bamf_window_get_window (bamf_window);
-
-  possible_apps = bamf_matcher_possible_applications_for_window (self, bamf_window);
-  const char *win_instance = bamf_legacy_window_get_class_instance_name (window);
-  const char *win_class_name = bamf_legacy_window_get_class_name (window);
-
-  app_class = win_instance;
+  win_class_name = bamf_legacy_window_get_class_name (window);
+
+  possible_apps = bamf_matcher_possible_applications_for_window (self, bamf_window, &target_class);
+  app_class = target_class;
 
   /* Loop over every possible desktop file that could match the window, and try
    * to reuse an already-opened window that uses it.
@@ -1799,7 +1827,7 @@
               const gchar *app_desktop_class;
               app_desktop_class = bamf_application_get_wmclass (app);
 
-              if (win_instance && app_desktop_class && strcasecmp (win_instance, app_desktop_class) == 0)
+              if (target_class && app_desktop_class && strcasecmp (target_class, app_desktop_class) == 0)
                 {
                   best = app;
                   break;
@@ -1829,7 +1857,7 @@
               const gchar *best_desktop_class;
 
               best_app_class = bamf_application_get_wmclass (best);
-              best_desktop_class = g_hash_table_lookup (self->priv->desktop_class_table, best_desktop);
+              best_desktop_class = bamf_matcher_get_desktop_file_class (self, best_desktop);
 
               /* We compare the two classes using their "distance" from the
                * desidered class value */
@@ -1867,7 +1895,7 @@
 
           if (bamf_application_contains_similar_to_window (app, bamf_window))
             {
-              if (win_instance && g_strcmp0 (win_instance, app_desktop_class) == 0)
+              if (target_class && g_strcmp0 (target_class, app_desktop_class) == 0)
                 {
                   best = app;
                   break;
@@ -1896,12 +1924,19 @@
         }
 
       bamf_application_set_wmclass (best, app_class);
-      bamf_matcher_register_view_stealing_ref (self, BAMF_VIEW (best));
-    }
 
-  bamf_view_add_child (BAMF_VIEW (best), BAMF_VIEW (bamf_window));
+      if (new_application)
+        *new_application = TRUE;
+    }
+  else
+    {
+      if (new_application)
+        *new_application = FALSE;
+    }
 
   g_list_free_full (possible_apps, g_free);
+
+  return best;
 }
 
 /* Ensures that the window hint is set if a registered pid matches, and that set window hints
@@ -1972,6 +2007,7 @@
 handle_raw_window (BamfMatcher *self, BamfLegacyWindow *window)
 {
   BamfWindow *bamfwindow;
+  BamfApplication *bamfapplication;
 
   g_return_if_fail (BAMF_IS_MATCHER (self));
   g_return_if_fail (BAMF_IS_LEGACY_WINDOW (window));
@@ -1989,7 +2025,15 @@
   bamfwindow = bamf_window_new (window);
   bamf_matcher_register_view_stealing_ref (self, BAMF_VIEW (bamfwindow));
 
-  bamf_matcher_setup_window_state (self, bamfwindow);
+  gboolean new_app = FALSE;
+  bamfapplication = bamf_matcher_get_application_for_window (self, bamfwindow, &new_app);
+
+  if (new_app)
+    {
+      bamf_matcher_register_view_stealing_ref (self, BAMF_VIEW (bamfapplication));
+    }
+
+  bamf_view_add_child (BAMF_VIEW (bamfapplication), BAMF_VIEW (bamfwindow));
 }
 
 static void
@@ -2243,6 +2287,15 @@
     }
 }
 
+const char *
+bamf_matcher_get_desktop_file_class (BamfMatcher * self, const char * desktop_file)
+{
+  g_return_val_if_fail (BAMF_IS_MATCHER (self), NULL);
+  g_return_val_if_fail (desktop_file, NULL);
+
+  return g_hash_table_lookup (self->priv->desktop_class_table, desktop_file);
+}
+
 static int
 x_error_handler (Display *display, XErrorEvent *event)
 {
@@ -2347,7 +2400,6 @@
   return (idx_a < idx_b) ? -1 : 1;
 }
 
-
 GVariant *
 bamf_matcher_get_window_stack_for_monitor (BamfMatcher *matcher, gint monitor)
 {
@@ -2515,8 +2567,8 @@
       if (g_list_find_custom (priv->favorites, fav, (GCompareFunc) g_strcmp0))
         continue;
 
+      bamf_matcher_load_desktop_file (matcher, fav);
       priv->favorites = g_list_prepend (priv->favorites, g_strdup (fav));
-      bamf_matcher_load_desktop_file (matcher, fav);
     }
 
   g_signal_emit (matcher, matcher_signals[FAVORITES_CHANGED], 0);
 
=== modified file 'src/bamf-matcher.h'
--- src/bamf-matcher.h	2012-01-21 00:29:55 +0000
+++ src/bamf-matcher.h	2012-05-22 16:24:22 +0000
@@ -74,6 +74,9 @@
                                                           const char *application,
                                                           gint pid);
 
+const char  * bamf_matcher_get_desktop_file_class        (BamfMatcher * self,
+                                                          const char * desktop_file);
+
 const char  * bamf_matcher_get_active_application        (BamfMatcher *matcher);
 
 const char  * bamf_matcher_get_active_window             (BamfMatcher *matcher);