Merge lp:~unity-team/unity/unity.composition-char into lp:unity

[Jay Taoko]

UNBLOCK

* Use Gtk im for composition characters.

Fix bug #944674

[Mirco Müller (macslow)]

In IMTextEntry::CheckValidClientWindow() you do...

76 + if (1/*focused_*/)
77 + {
78 + gtk_im_context_focus_in(im_context_);
79 + }

... please don't. Either use a proper conditional statement that can change at runtime or just drop completely.

In IMTextEntry::TryHandleEvent() you don't initialize ev...

89 + CheckValidClientWindow(event.x11_window);
90 +
91 + GdkEventKey ev;
92 + KeyEventToGdkEventKey(event, ev);

... and that's asking for trouble. Maybe not in that particular case (about to be filled by the next call). Just bzero() it. Always be explicit!

review: Needs Fixing

[Jay Taoko (jaytaoko)]

* ev is filled by the call to KeyEventToGdkEventKey. I think there is no need to bzero it.

[Neil J. Patel (njpatel)]

Code looks good, passes tests, excellent work!

review: Approve

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/385/console reported an error when processing this lp:~unity-team/unity/unity.composition-char branch.
Not merging it.

[Brandon Schaefer (brandontschaefer)]

This breaks IBus support as well as any shortcut that uses the ctrl key, ie ctrl + a/x/c/v...

review: Needs Fixing

=== modified file 'plugins/unityshell/src/IMTextEntry.cpp'
--- plugins/unityshell/src/IMTextEntry.cpp	2012-02-23 23:35:35 +0000
+++ plugins/unityshell/src/IMTextEntry.cpp	2012-03-06 05:12:17 +0000
@@ -35,8 +35,13 @@
 NUX_IMPLEMENT_OBJECT_TYPE(IMTextEntry);
 
 IMTextEntry::IMTextEntry()
-  : TextEntry("", NUX_TRACKER_LOCATION)
+: TextEntry("", NUX_TRACKER_LOCATION)
+, im_context_(0)
+, client_window_(0)
+, focused_(false)
 {
+  SetupSimpleIM();
+
   mouse_up.connect(sigc::mem_fun(this, &IMTextEntry::OnMouseButtonUp));
 }
 
@@ -44,12 +49,20 @@
                                   unsigned int keysym,
                                   const char* character)
 {
-  bool need_to_filter_event = TryHandleSpecial(event_type, keysym, character);
-
-  if (need_to_filter_event)
-    need_to_filter_event = TextEntry::InspectKeyEvent(event_type, keysym, character);
-
-  return need_to_filter_event;
+
+  bool propagate_event = !(TryHandleEvent(event_type, keysym, character));
+
+  if (propagate_event)
+  {
+    propagate_event = TryHandleSpecial(event_type, keysym, character);
+  }
+
+  if (propagate_event)
+  {
+    propagate_event = TextEntry::InspectKeyEvent(event_type, keysym, character);
+  }
+
+  return propagate_event;
 }
 
 bool IMTextEntry::TryHandleSpecial(unsigned int eventType, unsigned int keysym, const char* character)
@@ -138,7 +151,6 @@
     SetText(new_text.c_str());
     SetCursor(cursor + text.length());
     QueueRefresh (true, true);
-    text_changed.emit(this);
   }
 }
 
@@ -152,4 +164,61 @@
     Paste(true);
   } 
 }
+
+void IMTextEntry::KeyEventToGdkEventKey(Event& event, GdkEventKey& gdk_event)
+{
+  gdk_event.type = event.type == nux::NUX_KEYDOWN ? GDK_KEY_PRESS : GDK_KEY_RELEASE;
+  gdk_event.window = client_window_;
+  gdk_event.send_event = FALSE;
+  gdk_event.time = event.x11_timestamp;
+  gdk_event.state = event.x11_key_state;
+  gdk_event.keyval = event.x11_keysym;
+
+  gchar* txt = const_cast<gchar*>(event.GetText());
+  gdk_event.length = strlen(txt);
+  gdk_event.string = txt;
+
+  gdk_event.hardware_keycode = event.x11_keycode;
+  gdk_event.group = 0;
+  gdk_event.is_modifier = 0;
+}
+
+bool IMTextEntry::TryHandleEvent(unsigned int eventType,
+                                 unsigned int keysym,
+                                 const char* character)
+{
+  nux::Event event = nux::GetWindowThread()->GetGraphicsDisplay().GetCurrentEvent();
+  
+  GdkEventKey ev;
+  KeyEventToGdkEventKey(event, ev);
+
+  return gtk_im_context_filter_keypress(im_context_, &ev);
+}
+
+void IMTextEntry::OnCommit(GtkIMContext* context, char* str)
+{
+  LOG_DEBUG(logger) << "Commit: " << str;
+  DeleteSelection();
+
+  if (str)
+  {
+    std::string new_text = GetText();
+    new_text.insert(cursor_, str);
+    
+    int cursor = cursor_;
+    SetText(new_text.c_str());
+    SetCursor(cursor + strlen(str));
+    QueueRefresh (true, true);
+    text_changed.emit(this);
+  }
+}
+
+void IMTextEntry::SetupSimpleIM()
+{
+  LOG_DEBUG(logger) << "Using Simple IM.";
+  im_context_ = static_cast<GtkIMContext*>(gtk_im_context_simple_new());
+  
+  sig_manager_.Add(new Signal<void, GtkIMContext*, char*>(im_context_, "commit", sigc::mem_fun(this, &IMTextEntry::OnCommit)));
+}
+
 }

=== modified file 'plugins/unityshell/src/IMTextEntry.h'
--- plugins/unityshell/src/IMTextEntry.h	2012-02-23 19:52:40 +0000
+++ plugins/unityshell/src/IMTextEntry.h	2012-03-06 05:12:17 +0000
@@ -49,6 +49,18 @@
   void Paste(bool primary = false);
 
   void OnMouseButtonUp(int x, int y, unsigned long bflags, unsigned long kflags);
+
+  void KeyEventToGdkEventKey(Event& event, GdkEventKey& gdk_event);
+  bool TryHandleEvent(unsigned int eventType,
+                      unsigned int keysym,
+                      const char* character);
+  void SetupSimpleIM();
+  void OnCommit(GtkIMContext* context, char* str);
+
+  glib::Object<GtkIMContext> im_context_;
+  GdkWindow* client_window_;
+  glib::SignalManager sig_manager_;
+  bool focused_;
 };
 
 }

=== modified file 'tests/autopilot/autopilot/emulators/X11.py'
--- tests/autopilot/autopilot/emulators/X11.py	2012-03-01 20:16:50 +0000
+++ tests/autopilot/autopilot/emulators/X11.py	2012-03-06 05:12:17 +0000
@@ -78,6 +78,7 @@
         'Control' : 'Control_L',
         'Ctrl' : 'Control_L',
         'Alt' : 'Alt_L',
+        'AltR' : 'Alt_R',
         'Super' : 'Super_L',
         'Shift' : 'Shift_L',
         'Enter' : 'Return',

=== modified file 'tests/autopilot/autopilot/tests/test_dash.py'
--- tests/autopilot/autopilot/tests/test_dash.py	2012-03-05 01:30:34 +0000
+++ tests/autopilot/autopilot/tests/test_dash.py	2012-03-06 05:12:17 +0000
@@ -389,3 +389,33 @@
         searchbar = self.dash.get_searchbar()
         self.assertEqual("hello world", searchbar.search_string)
 
+class DashCompositionCharactersTests(AutopilotTestCase):
+    """Tests that composition characters works."""
+
+    def setUp(self):
+        super(DashCompositionCharactersTests, self).setUp()
+        self.dash = Dash()
+        self.addCleanup(self.dash.ensure_hidden)
+
+    def tearDown(self):
+        super(DashCompositionCharactersTests, self).tearDown()
+        self.dash.ensure_hidden()
+
+    def test_composition_characters(self):
+        """Expanding or collapsing the filterbar must keave keyboard focus in the
+        search bar.
+        """
+        self.dash.reveal_application_lens()
+        filter_bar = self.dash.get_current_lens().get_filterbar()
+        filter_bar.ensure_collapsed()
+
+        sleep(1)
+        self.keyboard.press('Shift')
+        self.keyboard.press_and_release('Multi_key')
+        self.keyboard.press_and_release('6')
+        self.keyboard.release('Shift')
+        self.keyboard.type('o')
+
+        searchbar = self.dash.get_searchbar()
+        self.assertEqual("ô", searchbar.search_string)        
+