Merge lp:~attente/unity-settings-daemon/gnome-key-grabber into lp:unity-settings-daemon

[William Hua]

Revert the legacy key grabber.

(This branch depends on https://code.launchpad.net/~attente/unity/gnome-key-grabber/+merge/202755 being released)

Revert the legacy key grabber.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:4015
http://jenkins.qa.ubuntu.com/job/unity-settings-daemon-ci/6/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-settings-daemon-trusty-amd64-ci/6
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-settings-daemon-trusty-armhf-ci/6
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-settings-daemon-trusty-i386-ci/6

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-settings-daemon-ci/6/rebuild

review: Approve (continuous-integration)

[Dimitri John Ledkov (xnox)]

Looks good, once a sensible version of unity with matching change is known, unity-settings-daemon needs "Breaks: unity (<< FIRST_VERSION_WITH_KEY_GRABS)"

[Sebastien Bacher (seb128)]

Thanks for the work!

There were some discussions last week, on #ubutu-desktop about that (see the irclog if you are interested into the details), turns out that some of the flavors/sessions would like to keep using the legacy grabber.

It looks like keeping the codepath for the LTS should be reasonable low cost (feel free to comment on that if you disagree, we might have overlooked details/issues), could you change the merge to keep the code but make it used only out of GNOME and Unity?

review: Needs Fixing

[lp:~attente/unity-settings-daemon/gnome-key-grabber]

4016. By William Hua on 2014-02-14

Update debian/changelog.

4017. By William Hua on 2014-02-17

Try again.

4018. By William Hua on 2014-02-17

Ubuntu has key grabber.

4019. By William Hua on 2014-02-17

Breaks pre-key-grabber Unity.

[William Hua (attente)]

Ok, thanks for clarifying, I guess I misunderstood before because I thought u-s-d would only be available under Unity, and other DEs would be using the less patched g-s-d.

[Sebastien Bacher (seb128)]

You didn't misunderstood, we though it would be the case, but it turned out that some flavors/session feel like they are closer from unity (they use indicators, they don't want the new GtkHeaderBars, they want menus) ... so we are revisiting the approch and going to make those sessions be variant of Unity rather than GNOME.

Thanks for updating it

One small comment, we usually do check for XDG_CURRENT_DESKTOP=GNOME|Unity, is there any reason to look at DESKTOP_SESSION there?

review: Needs Information

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:4019
http://jenkins.qa.ubuntu.com/job/unity-settings-daemon-ci/9/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-settings-daemon-trusty-amd64-ci/9
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-settings-daemon-trusty-armhf-ci/9
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-settings-daemon-trusty-i386-ci/9

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-settings-daemon-ci/9/rebuild

review: Approve (continuous-integration)

[William Hua (attente)]

So, the code in that file originally used DESKTOP_SESSION. But if we use XDG_CURRENT_DESKTOP, then there is no way to differentiate between GNOME Shell, GNOME Classic, and the two GNOME Fallback sessions. These are the values for each DE:

[Desktop: DESKTOP_SESSION, XDG_CURRENT_DESKTOP]
GNOME Shell: gnome, GNOME
GNOME Classic: gnome-classic, GNOME
GNOME Flashback (Compiz): gnome-fallback-compiz, GNOME
GNOME Flashback (Metacity): gnome-fallback, GNOME
Unity: ubuntu, Unity

So the question is if GNOME Classic and GNOME Fallback should use the legacy key grabber. If yes, we should use DESKTOP_SESSION, if not, we can use XDG_CURRENT_DESKTOP.

[Sebastien Bacher (seb128)]

that makes sense, thanks for the explanation!

review: Approve

[Sebastien Bacher (seb128)]

Works fine here, thanks

review: Approve

=== modified file 'debian/control'
--- debian/control	2014-02-07 14:57:09 +0000
+++ debian/control	2014-02-17 16:37:48 +0000
@@ -61,6 +61,7 @@
         gnome-session (<< 3.9.90-0ubuntu6),
         gnome-screensaver (<< 2.28.0),
         gnome-color-manager (<< 3.0),
+        unity (<< 7.1.2+14.04.20140214.1-0ubuntu1),
         unity-greeter (<< 0.2.1-0ubuntu1),
         indicator-datetime (<< 12.10.3daily13.03.26)
 Suggests: x11-xserver-utils,

=== modified file 'plugins/media-keys/gsd-media-keys-manager.c'
--- plugins/media-keys/gsd-media-keys-manager.c	2013-11-13 02:03:10 +0000
+++ plugins/media-keys/gsd-media-keys-manager.c	2014-02-17 16:37:48 +0000
@@ -741,8 +741,8 @@
 }
 
 static gboolean
-grab_media_key_unity (MediaKey            *key,
-                      GsdMediaKeysManager *manager)
+grab_media_key_legacy (MediaKey            *key,
+                       GsdMediaKeysManager *manager)
 {
     char *tmp;
     gboolean need_flush;
@@ -832,7 +832,7 @@
                         if (!manager->priv->have_legacy_keygrabber)
                             grab_media_key (key, manager);
                         else {
-                            if (grab_media_key_unity (key, manager))
+                            if (grab_media_key_legacy (key, manager))
                                 need_flush = TRUE;
                         }
                         break;
@@ -997,7 +997,7 @@
 	g_ptr_array_add (manager->priv->keys, key);
 
     if (manager->priv->have_legacy_keygrabber)
-        grab_media_key_unity (key, manager);
+        grab_media_key_legacy (key, manager);
 }
 
 static void
@@ -1038,7 +1038,7 @@
                 g_ptr_array_add (manager->priv->keys, key);
 
                 if (manager->priv->have_legacy_keygrabber)
-                        grab_media_key_unity (key, manager);
+                        grab_media_key_legacy (key, manager);
         }
         g_strfreev (custom_paths);
 
@@ -2123,8 +2123,9 @@
         GVariant *sources;
         gint i, n;
 
-        if (!manager->priv->have_legacy_keygrabber)
-                return;
+        if (g_strcmp0 (g_getenv ("DESKTOP_SESSION"), "ubuntu") != 0)
+                if (!manager->priv->have_legacy_keygrabber)
+                        return;
 
         settings = g_settings_new (GNOME_DESKTOP_INPUT_SOURCES_DIR);
         sources = g_settings_get_value (settings, KEY_INPUT_SOURCES);
@@ -2755,13 +2756,20 @@
         init_kbd (manager);
 }
 
+static gboolean
+session_has_key_grabber (void)
+{
+        const gchar *session = g_getenv ("DESKTOP_SESSION");
+        return g_strcmp0 (session, "gnome") == 0 || g_strcmp0 (session, "ubuntu") == 0;
+}
+
 static void
 on_shell_appeared (GDBusConnection   *connection,
                    const char        *name,
                    const char        *name_owner,
                    gpointer           user_data)
 {
-        if (g_strcmp0 (g_getenv ("DESKTOP_SESSION"), "gnome") != 0)
+        if (!session_has_key_grabber ())
                 return;
 
         GsdMediaKeysManager *manager = user_data;
@@ -2787,7 +2795,7 @@
                    const char       *name,
                    gpointer          user_data)
 {
-        if (g_strcmp0 (g_getenv ("DESKTOP_SESSION"), "gnome") != 0)
+        if (!session_has_key_grabber ())
                 return;
 
         GsdMediaKeysManager *manager = user_data;
@@ -2807,7 +2815,7 @@
         GsdMediaKeysManager *manager = user_data;
         GSList *l;
 
-        if (g_strcmp0 (g_getenv ("DESKTOP_SESSION"), "gnome") == 0)
+        if (session_has_key_grabber ())
                 return;
 
         manager->priv->have_legacy_keygrabber = TRUE;
@@ -2844,7 +2852,7 @@
 {
         GsdMediaKeysManager *manager = user_data;
 
-        if (g_strcmp0 (g_getenv ("DESKTOP_SESSION"), "gnome") == 0)
+        if (session_has_key_grabber ())
                 return;
 
         manager->priv->have_legacy_keygrabber = FALSE;