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

[Andrea Azzarone]

Close the overlay is a modifier key is pressed. Don't treat super+tab as a special case.

== 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

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:2457
http://s-jenkins:8080/job/unity-ci/53/

review: Approve (continuous-integration)

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

It's sad to remove so much tests, but... I have to approve this... :/

review: Approve

=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2012-06-29 20:35:23 +0000
+++ plugins/unityshell/src/unityshell.cpp	2012-07-02 15:44:22 +0000
@@ -1407,6 +1407,19 @@
       break;
     case KeyPress:
     {
+      if (super_keypressed_)
+      {
+        /* We need an idle to postpone this action, after the current event
+         * has been processed */
+        sources_.Add(std::make_shared<glib::Idle>([&]() {
+          shortcut_controller_->SetEnabled(false);
+          shortcut_controller_->Hide();
+          EnableCancelAction(CancelActionTarget::SHORTCUT_HINT, false);
+
+          return false; 
+        }));
+      }
+
       KeySym key_sym;
       char key_string[2];
       int result = XLookupString(&(event->xkey), key_string, 2, &key_sym, 0);
@@ -1434,21 +1447,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 */
-            sources_.Add(std::make_shared<glib::Idle>([&]() {
-              if (!launcher_controller_->KeyNavIsActive())
-              {
-                shortcut_controller_->SetEnabled(false);
-                shortcut_controller_->Hide();
-                EnableCancelAction(CancelActionTarget::SHORTCUT_HINT, false);
-              }
-              return false;
-            }));
-          }
-
           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/unity/tests/test_shortcut_hint.py'
--- tests/autopilot/unity/tests/test_shortcut_hint.py	2012-05-17 11:52:32 +0000
+++ tests/autopilot/unity/tests/test_shortcut_hint.py	2012-07-02 15:44:22 +0000
@@ -104,6 +104,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):
@@ -133,84 +140,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.