Merge lp:~3v1n0/unity/search-bar-modifiers-filter into lp:unity

[Marco Trevisan (Treviño)]

IMTextEntry: ignore invalid keypress when using modifiers keys.

All the key pressure when pressing the Alt or Super key should be ignored.
When pressing Ctrl key we should only allow: A key, arrow keys, Backspace or Delete keys and Home or Delete keys.

Adding proper filters to prevent invalid Ctrl+<key> pressure and all the Super+<key> and Alt+<key> presses, to be consistent with Gtk text entries and to avoid that using a compiz keybinding (such as Super+W) would cause the key to be written into the Dash or Hud search bars.

Unit tests included.

[Andrea Azzarone (azzar1)]

LGTM

review: Approve

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2012-06-15 02:03:31 +0000
+++ tests/CMakeLists.txt	2012-06-19 16:37:24 +0000
@@ -207,6 +207,7 @@
                  test_main.cpp
                  test_lensview_impl.cpp
                  test_icon_loader.cpp
+                 test_im_text_entry.cpp
                  test_hud_view.cpp
                  test_resultviewgrid.cpp
                  test_single_monitor_launcher_icon.cpp

=== added file 'tests/test_im_text_entry.cpp'
--- tests/test_im_text_entry.cpp	1970-01-01 00:00:00 +0000
+++ tests/test_im_text_entry.cpp	2012-06-19 16:37:24 +0000
@@ -0,0 +1,202 @@
+/*
+ * Copyright 2012 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3, as
+ * published by the  Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the applicable version of the GNU Lesser General Public
+ * License for more details.
+ *
+ * You should have received a copy of both the GNU Lesser General Public
+ * License version 3 along with this program.  If not, see
+ * <http://www.gnu.org/licenses/>
+ *
+ * Authored by: Marco Trevisan (Treviño) <[e-mail]>
+ *
+ */
+
+#include <gmock/gmock.h>
+#include "unity-shared/IMTextEntry.h"
+
+using namespace testing;
+using namespace unity;
+
+namespace
+{
+
+class TestEvent : public nux::Event
+{
+public:
+  TestEvent(nux::KeyModifier keymod, unsigned long keysym)
+  {
+    type = NUX_KEYDOWN;
+    key_modifiers = keymod;
+    x11_keysym = keysym;
+  }
+
+  TestEvent(unsigned long keysym)
+  {
+    type = NUX_KEYDOWN;
+    x11_keysym = keysym;
+  }
+};
+
+class MockTextEntry : public IMTextEntry
+{
+public:
+  MOCK_METHOD1(InsertText, void(std::string const&));
+  MOCK_METHOD0(Cut, void());
+  MOCK_METHOD0(Copy, void());
+  MOCK_METHOD1(Paste, void(bool));
+
+  bool TryHandleSpecial(nux::Event const& event)
+  {
+    return IMTextEntry::TryHandleSpecial(event);
+  }
+};
+
+
+TEST(TestIMTextEntry, CopyCtrlC)
+{
+  MockTextEntry text_entry;
+
+  TestEvent event(KEY_MODIFIER_CTRL, NUX_VK_c);
+
+  EXPECT_CALL(text_entry, Copy());
+  EXPECT_FALSE(text_entry.TryHandleSpecial(event));
+}
+
+TEST(TestIMTextEntry, CopyCtrlIns)
+{
+  MockTextEntry text_entry;
+
+  TestEvent event(KEY_MODIFIER_CTRL, NUX_VK_INSERT);
+
+  EXPECT_CALL(text_entry, Copy());
+  EXPECT_FALSE(text_entry.TryHandleSpecial(event));
+}
+
+TEST(TestIMTextEntry, PasteCtrlV)
+{
+  MockTextEntry text_entry;
+
+  TestEvent event(KEY_MODIFIER_CTRL, NUX_VK_v);
+
+  EXPECT_CALL(text_entry, Paste(false));
+  EXPECT_FALSE(text_entry.TryHandleSpecial(event));
+}
+
+TEST(TestIMTextEntry, PasteShiftIns)
+{
+  MockTextEntry text_entry;
+
+  TestEvent event(KEY_MODIFIER_SHIFT, NUX_VK_INSERT);
+
+  EXPECT_CALL(text_entry, Paste(false));
+  EXPECT_FALSE(text_entry.TryHandleSpecial(event));
+}
+
+TEST(TestIMTextEntry, CutCtrlX)
+{
+  MockTextEntry text_entry;
+
+  TestEvent event(KEY_MODIFIER_CTRL, NUX_VK_x);
+
+  EXPECT_CALL(text_entry, Cut());
+  EXPECT_FALSE(text_entry.TryHandleSpecial(event));
+}
+
+TEST(TestIMTextEntry, CutShiftDel)
+{
+  MockTextEntry text_entry;
+
+  TestEvent event(KEY_MODIFIER_SHIFT, NUX_VK_DELETE);
+
+  EXPECT_CALL(text_entry, Cut());
+  EXPECT_FALSE(text_entry.TryHandleSpecial(event));
+}
+
+TEST(TestIMTextEntry, CtrlMoveKeys)
+{
+  MockTextEntry text_entry;
+
+  TestEvent left(KEY_MODIFIER_CTRL, NUX_VK_LEFT);
+  EXPECT_TRUE(text_entry.TryHandleSpecial(left));
+
+  TestEvent right(KEY_MODIFIER_CTRL, NUX_VK_RIGHT);
+  EXPECT_TRUE(text_entry.TryHandleSpecial(right));
+
+  TestEvent home(KEY_MODIFIER_CTRL, NUX_VK_HOME);
+  EXPECT_TRUE(text_entry.TryHandleSpecial(home));
+
+  TestEvent end(KEY_MODIFIER_CTRL, NUX_VK_END);
+  EXPECT_TRUE(text_entry.TryHandleSpecial(end));
+}
+
+TEST(TestIMTextEntry, CtrlDeleteKeys)
+{
+  MockTextEntry text_entry;
+
+  TestEvent del(KEY_MODIFIER_CTRL, NUX_VK_DELETE);
+  EXPECT_TRUE(text_entry.TryHandleSpecial(del));
+
+  TestEvent backspace(KEY_MODIFIER_CTRL, NUX_VK_BACKSPACE);
+  EXPECT_TRUE(text_entry.TryHandleSpecial(backspace));
+}
+
+TEST(TestIMTextEntry, CtrlA)
+{
+  MockTextEntry text_entry;
+
+  TestEvent selectall(KEY_MODIFIER_CTRL, NUX_VK_a);
+  EXPECT_TRUE(text_entry.TryHandleSpecial(selectall));
+}
+
+TEST(TestIMTextEntry, CtrlKeybindings)
+{
+  MockTextEntry text_entry;
+
+  std::vector<unsigned long> allowed_keys { NUX_VK_a, NUX_VK_BACKSPACE,
+                                            NUX_VK_LEFT, NUX_VK_RIGHT,
+                                            NUX_VK_HOME, NUX_VK_END,
+                                            NUX_VK_BACKSPACE, NUX_VK_DELETE };
+
+  for (unsigned long keysym = 0; keysym < XK_VoidSymbol; ++keysym)
+  {
+    bool should_be_handled = false;
+
+    if (std::find(allowed_keys.begin(), allowed_keys.end(), keysym) != allowed_keys.end())
+      should_be_handled = true;
+
+    TestEvent event(KEY_MODIFIER_CTRL, keysym);
+    EXPECT_EQ(text_entry.TryHandleSpecial(event), should_be_handled);
+  }
+}
+
+TEST(TestIMTextEntry, AltKeybindings)
+{
+  MockTextEntry text_entry;
+
+  for (unsigned long keysym = 0; keysym < XK_VoidSymbol; ++keysym)
+  {
+    TestEvent event(KEY_MODIFIER_ALT, keysym);
+    EXPECT_FALSE(text_entry.TryHandleSpecial(event));
+  }
+}
+
+TEST(TestIMTextEntry, SuperKeybindings)
+{
+  MockTextEntry text_entry;
+
+  for (unsigned long keysym = 0; keysym < XK_VoidSymbol; ++keysym)
+  {
+    TestEvent event(KEY_MODIFIER_SUPER, keysym);
+    EXPECT_FALSE(text_entry.TryHandleSpecial(event));
+  }
+}
+
+}

=== modified file 'unity-shared/IMTextEntry.cpp'
--- unity-shared/IMTextEntry.cpp	2012-05-06 23:48:38 +0000
+++ unity-shared/IMTextEntry.cpp	2012-06-19 16:37:24 +0000
@@ -44,52 +44,60 @@
                                   unsigned int keysym,
                                   const char* character)
 {
-  bool need_to_filter_event = TryHandleSpecial(event_type, keysym, character);
- 
+  nux::Event const& event = nux::GetGraphicsDisplay()->GetCurrentEvent();
+  bool need_to_filter_event = TryHandleSpecial(event);
+
   if (need_to_filter_event)
     need_to_filter_event = TextEntry::InspectKeyEvent(event_type, keysym, character);
-  
+
   return need_to_filter_event;
 }
 
-bool IMTextEntry::TryHandleSpecial(unsigned int eventType, unsigned int keysym, const char* character)
+bool IMTextEntry::TryHandleSpecial(nux::Event const& event)
 {
-  nux::Event event = nux::GetWindowThread()->GetGraphicsDisplay().GetCurrentEvent();
-  unsigned int keyval = keysym;
-  bool shift = (event.GetKeyState() & NUX_STATE_SHIFT);
-  bool ctrl = (event.GetKeyState() & NUX_STATE_CTRL);
-
   /* If there is preedit, handle the event else where, but we
      want to be able to copy/paste while ibus is active */
   if (!preedit_.empty())
     return true;
 
-  if (eventType != NUX_KEYDOWN)
+  if (event.type != NUX_KEYDOWN)
     return false;
 
-  if (((keyval == NUX_VK_x) && ctrl && !shift) ||
-      ((keyval == NUX_VK_DELETE) && shift && !ctrl))
+  unsigned int keyval = event.GetKeySym();
+  bool shift = event.GetKeyModifierState(KEY_MODIFIER_SHIFT);
+  bool ctrl = event.GetKeyModifierState(KEY_MODIFIER_CTRL);
+  bool super = event.GetKeyModifierState(KEY_MODIFIER_SUPER);
+  bool alt = event.GetKeyModifierState(KEY_MODIFIER_ALT);
+
+  if ((ctrl && !shift && keyval == NUX_VK_x) ||    // Ctrl + X
+      (shift && !ctrl && keyval == NUX_VK_DELETE)) // Shift + Del
   {
     Cut();
   }
-  else if (((keyval == NUX_VK_c) && ctrl && (!shift)) ||
-           ((keyval == NUX_VK_INSERT) && ctrl && (!shift)))
+  else if (ctrl && !shift && (keyval == NUX_VK_c || keyval == NUX_VK_INSERT))  // Ctrl + C / Ins
   {
     Copy();
   }
-  else if (((keyval == NUX_VK_v) && ctrl && (!shift)) ||
-      ((keyval == NUX_VK_INSERT) && shift && (!ctrl)))
+  else if ((ctrl && !shift && keyval == NUX_VK_v) ||   // Ctrl + V
+           (shift && !ctrl && keyval == NUX_VK_INSERT)) // Shift + Ins
   {
     Paste();
   }
-  else if (keyval == NUX_VK_TAB || keyval == NUX_VK_LEFT_TAB)
-  {
-    return true;
-  }
-  else
-  {
-    return true;
-  }
+  else if (ctrl)
+  {
+    if (keyval == NUX_VK_LEFT || keyval == NUX_VK_RIGHT || // Ctrl + Move keys
+        keyval == NUX_VK_HOME || keyval == NUX_VK_END || // Ctrl + Home / End
+        keyval == NUX_VK_BACKSPACE || keyval == NUX_VK_DELETE || // Ctrl + Backspace / Delete
+        keyval == NUX_VK_a) // Ctrl + A
+    {
+      return true;
+    }
+  }
+  else if (!alt && !super)
+  {
+    return true;
+  }
+
   return false;
 }
 
@@ -146,10 +154,10 @@
   int button = nux::GetEventButton(bflags);
 
   if (button == 2)
-  { 
+  {
     SetCursor(XYToTextIndex(x,y));
     Paste(true);
-  } 
+  }
 }
 
 bool IMTextEntry::im_preedit()

=== modified file 'unity-shared/IMTextEntry.h'
--- unity-shared/IMTextEntry.h	2012-05-06 23:48:38 +0000
+++ unity-shared/IMTextEntry.h	2012-06-19 16:37:24 +0000
@@ -43,13 +43,14 @@
 
 private:
   bool InspectKeyEvent(unsigned int eventType, unsigned int keysym, const char* character);
-  bool TryHandleSpecial(unsigned int eventType, unsigned int keysym, const char* character);
-  void InsertText(std::string const& text);
-  void Cut();
-  void Copy();
-  void Paste(bool primary = false);
-
   void OnMouseButtonUp(int x, int y, unsigned long bflags, unsigned long kflags);
+
+protected:
+  bool TryHandleSpecial(nux::Event const& event);
+  virtual void InsertText(std::string const& text);
+  virtual void Cut();
+  virtual void Copy();
+  virtual void Paste(bool primary = false);
 };
 
 }