Merge lp:~unity-2d-team/unity-2d/unity-2d_enable-super-numkey-combo into lp:unity-2d

[Albert Astals Cid]

[launcher] Enabled Super+NumKey[0..9] shortcuts for launcher tiles.

We need separate hotkey's for them.

[Albert Astals Cid (aacid)]

This is the heir of https://code.launchpad.net/~unity-2d-team/unity-2d/enable-super-numkey-combo/+merge/89883 that was rejected because was against unity-2d-shell instead of unity-2d

Copying here Michał's comments of what needs to be fixed.

The test is broken:
* if the launcher item is already in "running" state, the test won't really test anything
* if the app in question doesn't start within TIMEOUT, the test will fail even though the feature actually worked
* LibreOffice Impress, for example, doesn't go into "running" state before you actually create a slidedeck, so the test will fail in a clean environment every time
* the result of this test is a workspace cluttered with up to 10 new windows that aren't closed

review: Needs Fixing

[Paweł Stołowski (stolowski)]

1. I think that the logic based on isX11Keysym in LauncherView::forwardNumericHotkey(), as well as Hotkey constructor are good candidates for introducing two simple classes inherited from HotKey: NumericHotkey and NumpadNumHotKey that hide x11keysm-based logic. They would inherit from Hotkey and from a common abstract class that provides index() interface, hinding index = key - .... calculation/comparison logic. I don't insist, but I prefer to avoid leaking of logic outside of classes and use polymorphic classes instead.

2. I think HotkeyMonitor::getHotkeyFor(Qt::Key ...) should check for equality of isX11keysym flag, and not only key and modifiers values.

review: Needs Fixing

[Gerry Boland (gerboland)]

This superceded by https://code.launchpad.net/~stolowski/unity-2d/numpad-modifier-support/+merge/97931, rejecting

review: Disapprove

Unmerged revisions

881. By Albert Astals Cid on 2012-02-03

forgot the test :D

880. By Albert Astals Cid on 2012-02-01

Backport r947 from unity-2d-shell

message:
  [shell][launcher] Enabled Super+NumKey[0..9] shortcuts for launcher tiles.
  We need separate hotkey's for them.

=== modified file 'launcher/app/launcherview.cpp'
--- launcher/app/launcherview.cpp	2011-11-25 11:13:44 +0000
+++ launcher/app/launcherview.cpp	2012-02-03 14:12:22 +0000
@@ -58,6 +58,10 @@
 static const char* SPREAD_DBUS_METHOD_IS_SHOWN = "IsShown";
 static const char* COMMANDS_LENS_ID = "commands.lens";
 
+static const int NumPad_Key_0 = 0xffb0;
+static const int NumPad_Key_1 = 0xffb1;
+static const int NumPad_Key_9 = 0xffb9;
+
 LauncherView::LauncherView(QWidget* parent) :
     Unity2DDeclarativeView(parent),
     m_superKeyPressed(false), m_superKeyHeld(false)
@@ -91,6 +95,17 @@
         hotkey = HotkeyMonitor::instance().getHotkeyFor(key, Qt::MetaModifier | Qt::ShiftModifier);
         connect(hotkey, SIGNAL(pressed()), SLOT(forwardNumericHotkey()));
     }
+
+    /* Super+NUMKEY{n} for 0 ≤ n ≤ 9 activates the item with index (n + 9) % 10.*/
+    /* Qt won't distinguish between keys[0-9] below the Functionkeys and keypad keys[0-9] on the Keypad.
+       Reference: <X11/keysymdef.h>
+    */
+    for (Qt::Key key = (Qt::Key)(NumPad_Key_0); key <= (Qt::Key)(NumPad_Key_9); key = (Qt::Key) (key + 1)) {
+        Hotkey* hotkey = HotkeyMonitor::instance().getHotkeyFor((Qt::Key)(key), Qt::MetaModifier, true);
+        connect(hotkey, SIGNAL(pressed()), SLOT(forwardNumericHotkey()));
+        hotkey = HotkeyMonitor::instance().getHotkeyFor((Qt::Key)(key), Qt::MetaModifier | Qt::ShiftModifier, true);
+        connect(hotkey, SIGNAL(pressed()), SLOT(forwardNumericHotkey()));
+    }
 }
 
 LauncherView::~LauncherView()
@@ -207,19 +222,25 @@
            In other words, the indexes are activated in the same order as
            the keys appear on a standard keyboard. */
         Qt::Key key = hotkey->key();
-        if (key >= Qt::Key_1 && key <= Qt::Key_9) {
-            int index = key - Qt::Key_0;
-            if (hotkey->modifiers() & Qt::ShiftModifier) {
-                Q_EMIT newInstanceShortcutPressed(index);
-            } else {
-                Q_EMIT activateShortcutPressed(index);
-            }
-        } else if (key == Qt::Key_0) {
-            if (hotkey->modifiers() & Qt::ShiftModifier) {
-                Q_EMIT newInstanceShortcutPressed(10);
-            } else {
-                Q_EMIT activateShortcutPressed(10);
-            }
+        int index = 0;
+        if (hotkey->isX11Keysym()) {
+            if (key >= NumPad_Key_1 && key <= NumPad_Key_9) {
+                index = key - NumPad_Key_0;
+            } else if (key == NumPad_Key_0) {
+               index = 10;
+            }
+        } else {
+            if (key >= Qt::Key_1 && key <= Qt::Key_9) {
+                index = key - Qt::Key_0;
+            } else if (key == Qt::Key_0) {
+                index = 10;
+            }
+        }
+
+        if (hotkey->modifiers() & Qt::ShiftModifier) {
+            Q_EMIT newInstanceShortcutPressed(index);
+        } else {
+            Q_EMIT activateShortcutPressed(index);
         }
     }
 }

=== modified file 'libunity-2d-private/src/hotkey.cpp'
--- libunity-2d-private/src/hotkey.cpp	2011-05-03 15:28:49 +0000
+++ libunity-2d-private/src/hotkey.cpp	2012-02-03 14:12:22 +0000
@@ -45,9 +45,9 @@
     return 0;
 }
 
-Hotkey::Hotkey(Qt::Key key, Qt::KeyboardModifiers modifiers, QObject *parent) :
+Hotkey::Hotkey(Qt::Key key, Qt::KeyboardModifiers modifiers, QObject *parent, bool isX11keysym) :
     QObject(parent), m_connections(0),
-    m_key(key), m_modifiers(modifiers),
+    m_key(key), m_modifiers(modifiers), m_isX11keysym(isX11keysym),
     m_x11key(0), m_x11modifiers(0)
 {
     /* Translate the QT modifiers to X11 modifiers */
@@ -65,23 +65,29 @@
         m_x11modifiers |= Mod4Mask;
     }
 
-    /* Translate the QT key to X11 keycode */
-
-    /* QKeySequence can be used to translate a Qt::Key in a format that is
-       understood by XStringToKeysym if the sequence is composed only by the key */
-    QString keyString = QKeySequence(key).toString();
-    KeySym keysym = XStringToKeysym(keyString.toLatin1().data());
-    if (keysym == NoSymbol) {
-        /* XStringToKeysym doesn’t work well with exotic characters (such as
-          'É'). Note that this fallback code path looks much simpler but doesn’t
-          work for special keys such as the function keys (e.g. F1), which is
-          why the translation with XStringToKeysym is attempted first. */
-        keysym = (ushort) key;
+    if (isX11keysym) {
+        m_x11key = XKeysymToKeycode(QX11Info::display(), key);
     }
-    m_x11key = XKeysymToKeycode(QX11Info::display(), keysym);
-    if (m_x11key == 0) {
-        UQ_WARNING << "Could not get keycode for keysym" << keysym
-                   << "(" << keyString << ")";
+    else  {
+        /* Translate the QT key to X11 keycode */
+
+        /* QKeySequence can be used to translate a Qt::Key in a format that is
+           understood by XStringToKeysym if the sequence is composed only by the key */
+        QString keyString = QKeySequence(key).toString();
+        KeySym keysym = XStringToKeysym(keyString.toLatin1().data());
+
+        if (keysym == NoSymbol) {
+            /* XStringToKeysym doesn’t work well with exotic characters (such as
+              'É'). Note that this fallback code path looks much simpler but doesn’t
+              work for special keys such as the function keys (e.g. F1), which is
+              why the translation with XStringToKeysym is attempted first. */
+            keysym = (ushort) key;
+        }
+        m_x11key = XKeysymToKeycode(QX11Info::display(), keysym);
+        if (m_x11key == 0) {
+            UQ_WARNING << "Could not get keycode for keysym" << keysym
+                       << "(" << keyString << ")";
+        }
     }
 }
 

=== modified file 'libunity-2d-private/src/hotkey.h'
--- libunity-2d-private/src/hotkey.h	2011-02-23 17:03:50 +0000
+++ libunity-2d-private/src/hotkey.h	2012-02-03 14:12:22 +0000
@@ -22,6 +22,8 @@
 
 #include <QObject>
 
+typedef unsigned long KeySym;
+
 class Hotkey : public QObject
 {
     friend class HotkeyMonitor;
@@ -33,6 +35,7 @@
 public:
     Qt::Key key() const { return m_key; }
     Qt::KeyboardModifiers modifiers() const { return m_modifiers; }
+    bool isX11Keysym() const { return m_isX11keysym; }
 
 Q_SIGNALS:
     void keyChanged(Qt::Key key);
@@ -45,13 +48,14 @@
     virtual void disconnectNotify(const char * signal);
 
 private:
-    Hotkey(Qt::Key key, Qt::KeyboardModifiers modifiers, QObject *parent);
+    Hotkey(Qt::Key key, Qt::KeyboardModifiers modifiers, QObject *parent, bool isX11keysym = false);
     bool processNativeEvent(uint x11Keycode, uint x11Modifiers, bool isPressEvent);
 
 private:
     uint m_connections;
     Qt::Key m_key;
     Qt::KeyboardModifiers m_modifiers;
+    bool m_isX11keysym;
     uint m_x11key;
     uint m_x11modifiers;
 };

=== modified file 'libunity-2d-private/src/hotkeymonitor.cpp'
--- libunity-2d-private/src/hotkeymonitor.cpp	2011-10-14 07:07:09 +0000
+++ libunity-2d-private/src/hotkeymonitor.cpp	2012-02-03 14:12:22 +0000
@@ -64,7 +64,7 @@
 
 
 Hotkey*
-HotkeyMonitor::getHotkeyFor(Qt::Key key, Qt::KeyboardModifiers modifiers)
+HotkeyMonitor::getHotkeyFor(Qt::Key key, Qt::KeyboardModifiers modifiers, bool isX11keysym)
 {
     Q_FOREACH(Hotkey* currentHotkey, m_hotkeys) {
         if (currentHotkey->key() == key &&
@@ -73,7 +73,7 @@
         }
     }
 
-    Hotkey *hotkey = new Hotkey(key, modifiers, this);
+    Hotkey *hotkey = new Hotkey(key, modifiers, this, isX11keysym);
     m_hotkeys.append(hotkey);
     return hotkey;
 }

=== modified file 'libunity-2d-private/src/hotkeymonitor.h'
--- libunity-2d-private/src/hotkeymonitor.h	2011-10-14 07:07:09 +0000
+++ libunity-2d-private/src/hotkeymonitor.h	2012-02-03 14:12:22 +0000
@@ -33,7 +33,7 @@
     static HotkeyMonitor& instance();
     ~HotkeyMonitor();
 
-    Hotkey* getHotkeyFor(Qt::Key key, Qt::KeyboardModifiers modifiers);
+    Hotkey* getHotkeyFor(Qt::Key key, Qt::KeyboardModifiers modifiers, bool isX11keysym = false);
 
     void disableModifiers(Qt::KeyboardModifiers modifiers);
     void enableModifiers(Qt::KeyboardModifiers modifiers);

=== added file 'tests/launcher/enable-super-numkey.rb'
--- tests/launcher/enable-super-numkey.rb	1970-01-01 00:00:00 +0000
+++ tests/launcher/enable-super-numkey.rb	2012-02-03 14:12:22 +0000
@@ -0,0 +1,82 @@
+#!/usr/bin/env ruby1.8
+=begin
+/*
+ * This file is part of unity-2d
+ *
+ * Copyright 2011 Canonical Ltd.
+ *
+ * Authors:
+ * - Gerry Boland <[e-mail]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+=end
+
+require '../run-tests.rb' unless $INIT_COMPLETED
+require 'xdo/keyboard'
+
+############################# Test Suite #############################
+context "Launcher Super+NUM key support Tests" do
+  # Run once at the beginning of this test suite
+  startup do
+    $SUT.execute_shell_command 'killall unity-2d-launcher'
+    $SUT.execute_shell_command 'killall unity-2d-launcher'
+  end
+
+  # Run once at the end of this test suite
+  shutdown do
+  end
+
+  # Run before each test case begins
+  setup do
+    # Execute the application
+    @app = $SUT.run( :name => UNITY_2D_LAUNCHER,
+                     :arguments => "-testability",
+                     :sleeptime => 2 )
+  end
+
+  # Run after each test case completes
+  teardown do
+    $SUT.execute_shell_command "pkill -nf unity-2d-launcher"
+  end
+
+  #####################################################################################
+    # Test cases
+    # Test case objectives:
+    # * To verify Launcher tile shortcuts Super+NumKey[0,9] are enabled
+    # Pre-conditions
+    # * None
+    # Test steps
+    # * List the numeric shortcuts from LauncherList
+    # * Simluate the Super+Numkey[0,9].
+    # * Verify that corresponding application is running.
+    # Post-conditions
+    # * None
+    # References
+    # * lp: 750514
+    test "Launcher enable Super+Numkey for tile shortcuts" do
+        tile_list = @app.Unity2dPanel().LauncherList(:name =>'main').children ({ :shortcutText => /\d/ })
+        tile_list.delete_if {|x| tile_list.index(x) >= 10 } #Remove duplicates if any.
+        tile_list.each do |tile|
+            shortcut = tile["shortcutText"]
+            numkey="{NUM"+shortcut+"}"
+            XDo::Keyboard.key_down('SUPER')
+            XDo::Keyboard.simulate(numkey)
+            XDo::Keyboard.key_up('SUPER')
+
+            verify_true(TIMEOUT, numkey+"-Failed") {
+                tile['running'] == 'true'
+            }
+        end
+    end
+end