Stop working around a limitation in GTK IM-module.

There is a long standing limitation in GTK IM-module that IMEs cannot retrieve
the screen coordinates of each composing character, which is definitely needed
to align suggestion window to the left edge of composing text.

In ibus-mozc, we have worked around this limitation by recording the cursor
rectangle in the mozc server rather and simulating the character screen
coordinates from those cursor rectangles since OSS Mozc 1.3.911.102
(a1fae21).

This emulation has, however, never been perfect.  Following issues are
actually edge cases of the emulation.
- #243: ibus predict window is shown at the previous cursor position
- https://bugzilla.mozilla.org/show_bug.cgi?id=1120851

Therefore we decided to remove the above emulation from ibus-mozc and live
in more robust but unsophisticated world instead.  With this CL, the
suggestion window will show up just under the cursor location rather than
being aligned with composing text.

This clean-up also enables us to refactor mozc-server without bothering future
ibus-mozc maintainers because that emulation code that is implemented in
mozc-server.  In subsequent CLs we can remove the emulation code without
breaking existing ibus-mozc client.

Closes #243.

BUG=#243
TEST=manually done on Ubuntu 14.04.

Author: yukawa

src/mozc_version_template.txt

@@ -1,6 +1,6 @@
 MAJOR=2
 MINOR=17
-BUILD=2108
+BUILD=2109
 REVISION=102
 # NACL_DICTIONARY_VERSION is the target version of the system dictionary to be
 # downloaded by NaCl Mozc.

src/renderer/unix/window_manager.cc

@@ -101,27 +101,19 @@ Rect WindowManager::UpdateCandidateWindow(
   const Size new_window_size = candidate_window_->Update(candidates);
 
   Point new_window_pos = candidate_window_->GetWindowPos();
-  if (candidates.has_window_location()) {
-    if (candidates.window_location() == commands::Candidates::CARET) {
-      DCHECK(candidates.has_caret_rectangle());
-      new_window_pos.x = candidates.caret_rectangle().x();
-      new_window_pos.y = candidates.caret_rectangle().y()
-          + candidates.caret_rectangle().height();
-    } else {
-      DCHECK(candidates.has_composition_rectangle());
-      new_window_pos.x = candidates.composition_rectangle().x();
-      new_window_pos.y = candidates.composition_rectangle().y()
-          + candidates.composition_rectangle().height();
-    }
+  if (command.has_preedit_rectangle()) {
+    new_window_pos.x = command.preedit_rectangle().left();
+    new_window_pos.y = command.preedit_rectangle().bottom();
   }
 
   const Rect working_area = GetMonitorRect(new_window_pos.x, new_window_pos.y);
   const Point alignment_base_point_in_local_window_coord(
       candidate_window_->GetCandidateColumnInClientCord().Left(), 0);
-  const Rect caret_rect(candidates.caret_rectangle().x(),
-                              candidates.caret_rectangle().y(),
-                              candidates.caret_rectangle().width(),
-                              candidates.caret_rectangle().height());
+  const auto &preedit_rect = command.preedit_rectangle();
+  const Rect caret_rect(preedit_rect.left(),
+                        preedit_rect.top(),
+                        preedit_rect.right() - preedit_rect.left(),
+                        preedit_rect.bottom() - preedit_rect.top());
   // |caret_rect| is not always equal to preedit rect but can be an alternative
   // in terms of positional calculation, especially for vertical adjustment in
   // horizontal writing.

src/renderer/unix/window_manager_test.cc

@@ -331,13 +331,12 @@ TEST(WindowManagerTest, UpdateCandidateWindowTest) {
     const Size window_size(35, 45);
     const gint monitor = 0x7777;
 
-    candidates->set_window_location(commands::Candidates::CARET);
     const Rect caret_rect(16, 26, 2, 13);
-    commands::Rectangle *rectangle = candidates->mutable_caret_rectangle();
-    rectangle->set_x(caret_rect.Left());
-    rectangle->set_y(caret_rect.Top());
-    rectangle->set_width(caret_rect.Width());
-    rectangle->set_height(caret_rect.Height());
+    auto *rectangle = command.mutable_preedit_rectangle();
+    rectangle->set_left(caret_rect.Left());
+    rectangle->set_top(caret_rect.Top());
+    rectangle->set_right(caret_rect.Right());
+    rectangle->set_bottom(caret_rect.Bottom());
     const Point expected_window_position(
         caret_rect.Left() - client_cord_rect.Left(),
         caret_rect.Top() + caret_rect.Height());
@@ -400,14 +399,12 @@ TEST(WindowManagerTest, UpdateCandidateWindowTest) {
     const Size window_size(35, 45);
     const gint monitor = 0x7777;
 
-    candidates->set_window_location(commands::Candidates::COMPOSITION);
     const Rect comp_rect(16, 26, 2, 13);
-    commands::Rectangle *rectangle
-        = candidates->mutable_composition_rectangle();
-    rectangle->set_x(comp_rect.Left());
-    rectangle->set_y(comp_rect.Top());
-    rectangle->set_width(comp_rect.Width());
-    rectangle->set_height(comp_rect.Height());
+    auto *rectangle = command.mutable_preedit_rectangle();
+    rectangle->set_left(comp_rect.Left());
+    rectangle->set_top(comp_rect.Top());
+    rectangle->set_right(comp_rect.Right());
+    rectangle->set_bottom(comp_rect.Bottom());
     const Point expected_window_position(
         comp_rect.Left() - client_cord_rect.Left(),
         comp_rect.Top() + comp_rect.Height());

src/unix/ibus/candidate_window_handler_interface.h

@@ -49,6 +49,10 @@ class CandidateWindowHandlerInterface {
   virtual void Update(IBusEngine *engine,
                       const commands::Output &output) = 0;
 
+  // Updates candidate state. This function also shows or hides candidate window
+  // based on the last |Update| call.
+  virtual void UpdateCursorRect(IBusEngine *engine) = 0;
+
   // Hides candidate window.
   virtual void Hide(IBusEngine *engine) = 0;
 

src/unix/ibus/gtk_candidate_window_handler.cc

@@ -54,16 +54,25 @@ GtkCandidateWindowHandler::~GtkCandidateWindowHandler() {
 }
 
 bool GtkCandidateWindowHandler::SendUpdateCommand(
-    const commands::Output &output, bool visibility) const {
+    IBusEngine *engine,
+    const commands::Output &output,
+    bool visibility) const {
   using commands::RendererCommand;
-
   RendererCommand command;
-  command.mutable_output()->CopyFrom(output);
-  command.set_type(commands::RendererCommand::UPDATE);
+
+  *command.mutable_output() = output;
+  command.set_type(RendererCommand::UPDATE);
   command.set_visible(visibility);
   RendererCommand::ApplicationInfo *appinfo
       = command.mutable_application_info();
 
+  auto *preedit_rectangle = command.mutable_preedit_rectangle();
+  const auto &cursor_area = engine->cursor_area;
+  preedit_rectangle->set_left(cursor_area.x);
+  preedit_rectangle->set_top(cursor_area.y);
+  preedit_rectangle->set_right(cursor_area.x + cursor_area.width);
+  preedit_rectangle->set_bottom(cursor_area.y + cursor_area.height);
+
   // Set pid
   static_assert(sizeof(::getpid()) <= sizeof(appinfo->process_id()),
                 "|appinfo->process_id()| must have sufficient room.");
@@ -83,17 +92,24 @@ bool GtkCandidateWindowHandler::SendUpdateCommand(
 
 void GtkCandidateWindowHandler::Update(IBusEngine *engine,
                                        const commands::Output &output) {
-  last_update_output_->CopyFrom(output);
+  *last_update_output_ = output;
+
+  UpdateCursorRect(engine);
+}
 
-  SendUpdateCommand(output, output.candidates().candidate_size() != 0);
+void GtkCandidateWindowHandler::UpdateCursorRect(IBusEngine *engine) {
+  const bool has_candidates =
+      last_update_output_->has_candidates() &&
+      last_update_output_->candidates().candidate_size() > 0;
+  SendUpdateCommand(engine, *last_update_output_, has_candidates);
 }
 
 void GtkCandidateWindowHandler::Hide(IBusEngine *engine) {
-  SendUpdateCommand(*(last_update_output_.get()), false);
+  SendUpdateCommand(engine, *last_update_output_, false);
 }
 
 void GtkCandidateWindowHandler::Show(IBusEngine *engine) {
-  SendUpdateCommand(*(last_update_output_.get()), true);
+  SendUpdateCommand(engine, *last_update_output_, true);
 }
 
 void GtkCandidateWindowHandler::OnIBusCustomFontDescriptionChanged(

src/unix/ibus/gtk_candidate_window_handler.h

@@ -36,6 +36,9 @@
 #include "unix/ibus/candidate_window_handler_interface.h"
 
 namespace mozc {
+namespace commands {
+class RendererCommand;
+}  // namespace commands
 namespace renderer {
 class RendererInterface;
 }  // namespace renderer
@@ -48,6 +51,7 @@ class GtkCandidateWindowHandler : public CandidateWindowHandlerInterface {
   virtual ~GtkCandidateWindowHandler();
 
   virtual void Update(IBusEngine *engine, const commands::Output &output);
+  virtual void UpdateCursorRect(IBusEngine *engine);
   virtual void Hide(IBusEngine *engine);
   virtual void Show(IBusEngine *engine);
 
@@ -58,7 +62,9 @@ class GtkCandidateWindowHandler : public CandidateWindowHandlerInterface {
       bool use_custom_font_description);
 
  protected:
-  bool SendUpdateCommand(const commands::Output &output, bool visibility) const;
+  bool SendUpdateCommand(IBusEngine *engine,
+                         const commands::Output &output,
+                         bool visibility) const;
 
   std::unique_ptr<renderer::RendererInterface> renderer_;
   std::unique_ptr<commands::Output> last_update_output_;

src/unix/ibus/gtk_candidate_window_handler_test.cc

@@ -31,6 +31,7 @@
 
 #include <unistd.h>  // for getpid()
 
+#include "base/coordinates.h"
 #include "protocol/renderer_command.pb.h"
 #include "renderer/renderer_mock.h"
 #include "testing/base/public/gmock.h"
@@ -110,6 +111,40 @@ MATCHER_P(VisibilityEq, visibility, "") {
   return true;
 }
 
+MATCHER_P(PreeditRectangleEq, rect, "") {
+  if (!arg.has_preedit_rectangle()) {
+    *result_listener
+        << "RendererCommand::preedit_rectangle does not exist";
+    return false;
+  }
+  const auto &actual_rect = arg.preedit_rectangle();
+  if (rect.Left() != actual_rect.left()) {
+    *result_listener << "left field does not match\n"
+                     << "  expected: " << rect.Left() << "\n"
+                     << "  actual:   " << actual_rect.left();
+    return false;
+  }
+  if (rect.Top() != actual_rect.top()) {
+    *result_listener << "top field does not match\n"
+                     << "  expected: " << rect.Top() << "\n"
+                     << "  actual:   " << actual_rect.top();
+    return false;
+  }
+  if (rect.Right() != actual_rect.right()) {
+    *result_listener << "right field does not match\n"
+                     << "  expected: " << rect.Right() << "\n"
+                     << "  actual:   " << actual_rect.right();
+    return false;
+  }
+  if (rect.Bottom() != actual_rect.bottom()) {
+    *result_listener << "bottom field does not match\n"
+                     << "  expected: " << rect.Bottom() << "\n"
+                     << "  actual:   " << actual_rect.bottom();
+    return false;
+  }
+  return true;
+}
+
 MATCHER_P(OutputEq, expected, "") {
   if (expected.Utf8DebugString() != arg.Utf8DebugString()) {
     *result_listener
@@ -129,47 +164,73 @@ MATCHER_P(OutputEq, expected, "") {
 }  // namespace
 
 TEST(GtkCandidateWindowHandlerTest, SendUpdateCommandTest) {
+  const Rect kExpectedCursorArea(10, 20, 200, 100);
+
+  IBusEngine engine = {};
+  engine.cursor_area.x = kExpectedCursorArea.Left();
+  engine.cursor_area.y = kExpectedCursorArea.Top();
+  engine.cursor_area.width = kExpectedCursorArea.Width();
+  engine.cursor_area.height = kExpectedCursorArea.Height();
+
   {
     SCOPED_TRACE("visibility check. false case");
     Output output;
     RendererMock *renderer_mock = new RendererMock();
     TestableGtkCandidateWindowHandler gtk_candidate_window_handler(
         renderer_mock);
-    EXPECT_CALL_EXEC_COMMAND(*renderer_mock, VisibilityEq(false));
-    gtk_candidate_window_handler.SendUpdateCommand(output, false);
+    EXPECT_CALL_EXEC_COMMAND(*renderer_mock,
+                             VisibilityEq(false),
+                             PreeditRectangleEq(kExpectedCursorArea));
+    gtk_candidate_window_handler.SendUpdateCommand(&engine, output, false);
   }
   {
     SCOPED_TRACE("visibility check. true case");
     Output output;
     RendererMock *renderer_mock = new RendererMock();
     TestableGtkCandidateWindowHandler gtk_candidate_window_handler(
         renderer_mock);
-    EXPECT_CALL_EXEC_COMMAND(*renderer_mock, VisibilityEq(true));
-    gtk_candidate_window_handler.SendUpdateCommand(output, true);
+    EXPECT_CALL_EXEC_COMMAND(*renderer_mock,
+                             VisibilityEq(true),
+                             PreeditRectangleEq(kExpectedCursorArea));
+    gtk_candidate_window_handler.SendUpdateCommand(&engine, output, true);
   }
   {
     SCOPED_TRACE("return value check. false case.");
     Output output;
     RendererMock *renderer_mock = new RendererMock();
     TestableGtkCandidateWindowHandler gtk_candidate_window_handler(
         renderer_mock);
-    EXPECT_CALL_EXEC_COMMAND(*renderer_mock, VisibilityEq(true))
+    EXPECT_CALL_EXEC_COMMAND(*renderer_mock,
+                             VisibilityEq(true),
+                             PreeditRectangleEq(kExpectedCursorArea))
         .WillOnce(Return(false));
-    EXPECT_FALSE(gtk_candidate_window_handler.SendUpdateCommand(output, true));
+    EXPECT_FALSE(gtk_candidate_window_handler.SendUpdateCommand(
+        &engine, output, true));
   }
   {
     SCOPED_TRACE("return value check. true case.");
     Output output;
     RendererMock *renderer_mock = new RendererMock();
     TestableGtkCandidateWindowHandler gtk_candidate_window_handler(
         renderer_mock);
-    EXPECT_CALL_EXEC_COMMAND(*renderer_mock, VisibilityEq(true))
+    EXPECT_CALL_EXEC_COMMAND(*renderer_mock,
+                             VisibilityEq(true),
+                             PreeditRectangleEq(kExpectedCursorArea))
         .WillOnce(Return(true));
-    EXPECT_TRUE(gtk_candidate_window_handler.SendUpdateCommand(output, true));
+    EXPECT_TRUE(gtk_candidate_window_handler.SendUpdateCommand(
+        &engine, output, true));
   }
 }
 
 TEST(GtkCandidateWindowHandlerTest, UpdateTest) {
+  const Rect kExpectedCursorArea(10, 20, 200, 100);
+
+  IBusEngine engine = {};
+  engine.cursor_area.x = kExpectedCursorArea.Left();
+  engine.cursor_area.y = kExpectedCursorArea.Top();
+  engine.cursor_area.width = kExpectedCursorArea.Width();
+  engine.cursor_area.height = kExpectedCursorArea.Height();
+
   const int sample_idx1 = 0;
   const int sample_idx2 = 1;
   const char *sample_candidate1 = "SAMPLE_CANDIDATE1";
@@ -180,8 +241,10 @@ TEST(GtkCandidateWindowHandlerTest, UpdateTest) {
     RendererMock *renderer_mock = new RendererMock();
     TestableGtkCandidateWindowHandler gtk_candidate_window_handler(
         renderer_mock);
-    EXPECT_CALL_EXEC_COMMAND(*renderer_mock, VisibilityEq(false));
-    gtk_candidate_window_handler.Update(NULL, output);
+    EXPECT_CALL_EXEC_COMMAND(*renderer_mock,
+                             VisibilityEq(false),
+                             PreeditRectangleEq(kExpectedCursorArea));
+    gtk_candidate_window_handler.Update(&engine, output);
   }
   {
     SCOPED_TRACE("If there is at least one candidate, "
@@ -194,8 +257,10 @@ TEST(GtkCandidateWindowHandlerTest, UpdateTest) {
     RendererMock *renderer_mock = new RendererMock();
     TestableGtkCandidateWindowHandler gtk_candidate_window_handler(
         renderer_mock);
-    EXPECT_CALL_EXEC_COMMAND(*renderer_mock, VisibilityEq(true));
-    gtk_candidate_window_handler.Update(NULL, output);
+    EXPECT_CALL_EXEC_COMMAND(*renderer_mock,
+                             VisibilityEq(true),
+                             PreeditRectangleEq(kExpectedCursorArea));
+    gtk_candidate_window_handler.Update(&engine, output);
   }
   {
     SCOPED_TRACE("Update last updated output protobuf object.");
@@ -221,29 +286,49 @@ TEST(GtkCandidateWindowHandlerTest, UpdateTest) {
                              Property(&RendererCommand::output,
                                       OutputEq(output2)))
         .WillOnce(Return(true));
-    gtk_candidate_window_handler.Update(NULL, output1);
+    gtk_candidate_window_handler.Update(&engine, output1);
     EXPECT_THAT(*(gtk_candidate_window_handler.last_update_output_.get()),
                 OutputEq(output1));
-    gtk_candidate_window_handler.Update(NULL, output2);
+    gtk_candidate_window_handler.Update(&engine, output2);
     EXPECT_THAT(*(gtk_candidate_window_handler.last_update_output_.get()),
                 OutputEq(output2));
   }
 }
 
 TEST(GtkCandidateWindowHandlerTest, HideTest) {
+  const Rect kExpectedCursorArea(10, 20, 200, 100);
+
+  IBusEngine engine = {};
+  engine.cursor_area.x = kExpectedCursorArea.Left();
+  engine.cursor_area.y = kExpectedCursorArea.Top();
+  engine.cursor_area.width = kExpectedCursorArea.Width();
+  engine.cursor_area.height = kExpectedCursorArea.Height();
+
   RendererMock *renderer_mock = new RendererMock();
   TestableGtkCandidateWindowHandler gtk_candidate_window_handler(
       renderer_mock);
-  EXPECT_CALL_EXEC_COMMAND(*renderer_mock, VisibilityEq(false));
-  gtk_candidate_window_handler.Hide(NULL);
+  EXPECT_CALL_EXEC_COMMAND(*renderer_mock,
+                           VisibilityEq(false),
+                           PreeditRectangleEq(kExpectedCursorArea));
+  gtk_candidate_window_handler.Hide(&engine);
 }
 
 TEST(GtkCandidateWindowHandlerTest, ShowTest) {
+  const Rect kExpectedCursorArea(10, 20, 200, 100);
+
+  IBusEngine engine = {};
+  engine.cursor_area.x = kExpectedCursorArea.Left();
+  engine.cursor_area.y = kExpectedCursorArea.Top();
+  engine.cursor_area.width = kExpectedCursorArea.Width();
+  engine.cursor_area.height = kExpectedCursorArea.Height();
+
   RendererMock *renderer_mock = new RendererMock();
   TestableGtkCandidateWindowHandler gtk_candidate_window_handler(
       renderer_mock);
-  EXPECT_CALL_EXEC_COMMAND(*renderer_mock, VisibilityEq(true));
-  gtk_candidate_window_handler.Show(NULL);
+  EXPECT_CALL_EXEC_COMMAND(*renderer_mock,
+                           VisibilityEq(true),
+                           PreeditRectangleEq(kExpectedCursorArea));
+  gtk_candidate_window_handler.Show(&engine);
 }
 
 }  // namespace ibus