Merge lp:~azzar1/unity/fix-934062-5.0 into lp:unity/5.0

[Andrea Azzarone]

Backport lp:~andyrock/unity/fix-934062

== Problem ==
"Keyboard Shortcuts" overlay can cause annoyance

== Fix ==
Close the overlay if a modifier key is pressed. Super + TAB must not be treated as a special case! [1]

== Test ==
AP test added.

[1] https://bugs.launchpad.net/unity/+bug/934062/comments/27

[Marco Trevisan (Treviño) (3v1n0)]

+1

review: Approve

=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2012-06-27 01:19:32 +0000
+++ plugins/unityshell/src/unityshell.cpp	2012-07-03 18:08:21 +0000
@@ -1396,6 +1396,21 @@
       break;
     case KeyPress:
     {
+      if (super_keypressed_)
+      {
+        /* We need an idle to postpone this action, after the current event
+         * has been processed */
+        g_idle_add([] (gpointer data) -> gboolean {
+          auto self = static_cast<UnityScreen*>(data);
+
+          self->shortcut_controller_->SetEnabled(false);
+          self->shortcut_controller_->Hide();
+          self->EnableCancelAction(CancelActionTarget::SHORTCUT_HINT, false);
+
+          return FALSE;
+        }, this);
+      }
+
       KeySym key_sym;
       char key_string[2];
       int result = XLookupString(&(event->xkey), key_string, 2, &key_sym, 0);
@@ -1423,22 +1438,6 @@
 
         if (super_keypressed_)
         {
-          if (key_sym != XK_Escape || (key_sym == XK_Escape && !launcher_controller_->KeyNavIsActive()))
-          {
-            /* We need an idle to postpone this action, after the current event
-             * has been processed */
-            g_idle_add([] (gpointer data) -> gboolean {
-              auto self = static_cast<UnityScreen*>(data);
-              if (!self->launcher_controller_->KeyNavIsActive())
-              {
-                self->shortcut_controller_->SetEnabled(false);
-                self->shortcut_controller_->Hide();
-                self->EnableCancelAction(CancelActionTarget::SHORTCUT_HINT, false);
-              }
-              return FALSE;
-            }, this);
-          }
-
           skip_other_plugins = launcher_controller_->HandleLauncherKeyEvent(screen->dpy(), key_sym, event->xkey.keycode, event->xkey.state, key_string);
           if (!skip_other_plugins)
             skip_other_plugins = dash_controller_->CheckShortcutActivation(key_string);

=== modified file 'tests/autopilot/autopilot/tests/test_shortcut_hint.py'
--- tests/autopilot/autopilot/tests/test_shortcut_hint.py	2012-05-23 11:37:41 +0000
+++ tests/autopilot/autopilot/tests/test_shortcut_hint.py	2012-07-03 18:08:21 +0000
@@ -103,6 +103,13 @@
         self.keybinding_tap("expo/start")
         self.addCleanup(self.keybinding, "expo/cancel")
 
+    def test_shortcut_hint_hide_pressing_modifiers(self):
+        """Pressing a modifer key must hide the shortcut hint."""
+        self.shortcut_hint.ensure_visible()
+        self.addCleanup(self.shortcut_hint.ensure_hidden)
+
+        self.keyboard.press('Control')
+
         self.assertThat(self.shortcut_hint.visible, Eventually(Equals(False)))
 
     def test_launcher_switcher_next_doesnt_show_shortcut_hint(self):
@@ -132,84 +139,6 @@
 
         self.assertThat(self.shortcut_hint.visible, Equals(False))
 
-    def test_launcher_switcher_next_keeps_shortcut_hint(self):
-        """Super+Tab switcher cycling forwards must not dispel an already-showing
-        shortcut hint.
-
-        """
-        show_timeout = self.shortcut_hint.get_show_timeout()
-        self.shortcut_hint.ensure_visible()
-        self.addCleanup(self.shortcut_hint.ensure_hidden)
-
-        self.keybinding("launcher/switcher/next")
-        self.addCleanup(self.keyboard.press_and_release, "Escape")
-
-        self.assertThat(self.launcher.key_nav_is_active, Equals(True))
-
-        self.keybinding("launcher/switcher/next")
-        sleep(show_timeout * 2)
-        self.assertThat(self.shortcut_hint.visible, Equals(True))
-
-    def test_launcher_switcher_prev_keeps_shortcut_hint(self):
-        """Super+Tab switcher cycling backwards must not dispel an already-showing
-        shortcut hint.
-
-        """
-        show_timeout = self.shortcut_hint.get_show_timeout()
-        self.shortcut_hint.ensure_visible()
-        self.addCleanup(self.shortcut_hint.ensure_visible)
-
-        self.keybinding("launcher/switcher/next")
-        self.addCleanup(self.keyboard.press_and_release, "Escape")
-        self.assertThat(self.launcher.key_nav_is_active, Equals(True))
-
-        self.keybinding("launcher/switcher/prev")
-        self.keybinding("launcher/switcher/prev")
-        sleep(show_timeout * 2)
-        self.assertThat(self.shortcut_hint.visible, Equals(True))
-
-    def test_launcher_switcher_cancel_doesnt_hide_shortcut_hint(self):
-        """Cancelling the launcher switcher (by Escape) must not hide the
-        shortcut hint view.
-
-        """
-        self.shortcut_hint.ensure_visible()
-        self.addCleanup(self.shortcut_hint.ensure_hidden)
-
-        self.keybinding("launcher/switcher/next")
-        # NOTE: This will generate an extra Escape keypress if the test passes,
-        # but that's better than the alternative...
-        self.addCleanup(self.keybinding, "launcher/switcher/exit")
-
-        self.assertThat(self.launcher.key_nav_is_active, Eventually(Equals(True)))
-        self.assertThat(self.shortcut_hint.visible, Equals(True))
-
-        self.keyboard.press_and_release("Escape")
-
-        self.assertThat(self.launcher.key_nav_is_active, Eventually(Equals(False)))
-        self.assertThat(self.shortcut_hint.visible, Equals(True))
-
-    def test_launcher_switcher_and_shortcut_hint_escaping(self):
-        """Cancelling the launcher switcher (by Escape) should not hide the
-        shortcut hint view, an extra keypress is needed.
-
-        """
-        self.shortcut_hint.ensure_visible()
-        self.addCleanup(self.shortcut_hint.ensure_hidden)
-
-        self.keybinding("launcher/switcher/next")
-
-        self.assertThat(self.launcher.key_nav_is_active, Eventually(Equals(True)))
-        self.assertThat(self.shortcut_hint.visible, Equals(True))
-
-        self.keyboard.press_and_release("Escape")
-
-        self.assertThat(self.launcher.key_nav_is_active, Eventually(Equals(False)))
-        self.assertThat(self.shortcut_hint.visible, Equals(True))
-
-        self.keyboard.press_and_release("Escape")
-        self.assertThat(self.shortcut_hint.visible, Eventually(Equals(False)))
-
     def test_launcher_icons_hints_show_with_shortcut_hint(self):
         """When the shortcut hint is shown also the launcer's icons hints should
         be shown.