[gimp] app: fix various undo- and preedit-related text tool issues

[Michael Natterer <mitch src gnome org>]

commit cd147a4a4898eab6df09466d3791813147c60da2
Author: Michael Natterer <mitch gimp org>
Date:   Sun Jun 5 16:23:50 2016 +0200

    app: fix various undo- and preedit-related text tool issues
    
    In the text tool editor code, connect to GtkIMContext::preedit-start
    and introduce a boolean text_tool->preedit_active which indicates that
    a preedit is going on.
    
    Remove the new preedit-removal code from gimp_text_tool_reset_im_context()
    because it was not reflecting the IM's internal state and made things
    worse. Instead, added gimp_text_tool_abort_im_context() which really
    gets rid of any ongoing preedit by force.
    
    In the main text tool code, check for preedit_active and if TRUE,
    apply any edits directly widhout pushing undo steps. Factored out
    gimp_text_tool_apply_list() for that purpose in order not do
    duplicate a lot of code.
    
    On undo and on button_press, force-abort any ongoing preedit. This is
    the right thing to do on undo, but not really on button_press, but I
    don't see another way to keep states consistent.

 app/tools/gimptexttool-editor.c |   49 ++++++++++++++-
 app/tools/gimptexttool-editor.h |    1 +
 app/tools/gimptexttool.c        |  130 ++++++++++++++++++++++++++++----------
 app/tools/gimptexttool.h        |    1 +
 4 files changed, 144 insertions(+), 37 deletions(-)
---
diff --git a/app/tools/gimptexttool-editor.c b/app/tools/gimptexttool-editor.c
index ef4bae9..acae5be 100644
--- a/app/tools/gimptexttool-editor.c
+++ b/app/tools/gimptexttool-editor.c
@@ -95,6 +95,8 @@ static void     gimp_text_tool_xy_to_iter         (GimpTextTool    *text_tool,
                                                    gdouble          y,
                                                    GtkTextIter     *iter);
 
+static void     gimp_text_tool_im_preedit_start   (GtkIMContext    *context,
+                                                   GimpTextTool    *text_tool);
 static void     gimp_text_tool_im_preedit_end     (GtkIMContext    *context,
                                                    GimpTextTool    *text_tool);
 static void     gimp_text_tool_im_preedit_changed (GtkIMContext    *context,
@@ -127,6 +129,9 @@ gimp_text_tool_editor_init (GimpTextTool *text_tool)
   text_tool->overwrite_mode = FALSE;
   text_tool->x_pos          = -1;
 
+  g_signal_connect (text_tool->im_context, "preedit-start",
+                    G_CALLBACK (gimp_text_tool_im_preedit_start),
+                    text_tool);
   g_signal_connect (text_tool->im_context, "preedit-end",
                     G_CALLBACK (gimp_text_tool_im_preedit_end),
                     text_tool);
@@ -535,9 +540,6 @@ gimp_text_tool_editor_key_release (GimpTextTool *text_tool,
 void
 gimp_text_tool_reset_im_context (GimpTextTool *text_tool)
 {
-  /* Cancel any ungoing preedit on reset. */
-  gimp_text_tool_im_delete_preedit (text_tool);
-
   if (text_tool->needs_im_reset)
     {
       text_tool->needs_im_reset = FALSE;
@@ -546,6 +548,28 @@ gimp_text_tool_reset_im_context (GimpTextTool *text_tool)
 }
 
 void
+gimp_text_tool_abort_im_context (GimpTextTool *text_tool)
+{
+  GimpTool         *tool  = GIMP_TOOL (text_tool);
+  GimpDisplayShell *shell = gimp_display_get_shell (tool->display);
+
+  text_tool->needs_im_reset = TRUE;
+  gimp_text_tool_reset_im_context (text_tool);
+
+  /* the following lines seem to be the only way of really getting
+   * rid of any ongoing preedit state, please somebody tell me
+   * a clean way... mitch
+   */
+
+  gtk_im_context_focus_out (text_tool->im_context);
+  gtk_im_context_set_client_window (text_tool->im_context, NULL);
+
+  gtk_im_context_set_client_window (text_tool->im_context,
+                                    gtk_widget_get_window (shell->canvas));
+  gtk_im_context_focus_in (text_tool->im_context);
+}
+
+void
 gimp_text_tool_editor_get_cursor_rect (GimpTextTool   *text_tool,
                                        gboolean        overwrite,
                                        PangoRectangle *cursor_rect)
@@ -1336,10 +1360,23 @@ gimp_text_tool_xy_to_iter (GimpTextTool *text_tool,
 }
 
 static void
+gimp_text_tool_im_preedit_start (GtkIMContext *context,
+                                 GimpTextTool *text_tool)
+{
+  GIMP_LOG (TEXT_EDITING, "preedit start");
+
+  text_tool->preedit_active = TRUE;
+}
+
+static void
 gimp_text_tool_im_preedit_end (GtkIMContext *context,
                                GimpTextTool *text_tool)
 {
   gimp_text_tool_delete_selection (text_tool);
+
+  text_tool->preedit_active = FALSE;
+
+  GIMP_LOG (TEXT_EDITING, "preedit end");
 }
 
 static void
@@ -1349,6 +1386,10 @@ gimp_text_tool_im_preedit_changed (GtkIMContext *context,
   GtkTextBuffer *buffer = GTK_TEXT_BUFFER (text_tool->buffer);
   PangoAttrList *attrs;
 
+  GIMP_LOG (TEXT_EDITING, "preedit changed");
+
+  gtk_text_buffer_begin_user_action (buffer);
+
   gimp_text_tool_im_delete_preedit (text_tool);
 
   gimp_text_tool_delete_selection (text_tool);
@@ -1473,6 +1514,8 @@ gimp_text_tool_im_preedit_changed (GtkIMContext *context,
     }
 
   pango_attr_list_unref (attrs);
+
+  gtk_text_buffer_end_user_action (buffer);
 }
 
 static void
diff --git a/app/tools/gimptexttool-editor.h b/app/tools/gimptexttool-editor.h
index 40bae85..5cdf9db 100644
--- a/app/tools/gimptexttool-editor.h
+++ b/app/tools/gimptexttool-editor.h
@@ -45,6 +45,7 @@ gboolean   gimp_text_tool_editor_key_release      (GimpTextTool        *text_too
                                                    GdkEventKey         *kevent);
 
 void       gimp_text_tool_reset_im_context        (GimpTextTool        *text_tool);
+void       gimp_text_tool_abort_im_context        (GimpTextTool        *text_tool);
 
 void       gimp_text_tool_editor_get_cursor_rect  (GimpTextTool        *text_tool,
                                                    gboolean             overwrite,
diff --git a/app/tools/gimptexttool.c b/app/tools/gimptexttool.c
index 01cc9ba..aadab32 100644
--- a/app/tools/gimptexttool.c
+++ b/app/tools/gimptexttool.c
@@ -148,6 +148,8 @@ static void      gimp_text_tool_text_changed    (GimpText          *text,
 
 static gboolean  gimp_text_tool_apply           (GimpTextTool      *text_tool,
                                                  gboolean           push_undo);
+static void      gimp_text_tool_apply_list      (GimpTextTool      *text_tool,
+                                                 GList             *pspecs);
 
 static void      gimp_text_tool_create_layer    (GimpTextTool      *text_tool,
                                                  GimpText          *text);
@@ -366,7 +368,14 @@ gimp_text_tool_button_press (GimpTool            *tool,
     {
       gimp_tool_control_activate (tool->control);
 
-      gimp_text_tool_reset_im_context (text_tool);
+      /* clicking anywhere while a preedit is going on aborts the
+       * preedit, this is ugly but at least leaves everything in
+       * a consistent state
+       */
+      if (text_tool->preedit_active)
+        gimp_text_tool_abort_im_context (text_tool);
+      else
+        gimp_text_tool_reset_im_context (text_tool);
 
       text_tool->selecting = FALSE;
 
@@ -1130,17 +1139,59 @@ gimp_text_tool_proxy_notify (GimpText         *text,
   if ((pspec->flags & G_PARAM_READWRITE) == G_PARAM_READWRITE &&
       pspec->owner_type == GIMP_TYPE_TEXT)
     {
-      gimp_text_tool_block_drawing (text_tool);
+      if (text_tool->preedit_active)
+        {
+          /* if there is a preedit going on, don't queue pending
+           * changes to be idle-applied with undo; instead, flush the
+           * pending queue (happens only when preedit starts), and
+           * apply the changes to text_tool->text directly. Preedit
+           * will *always* end by removing the preedit string, and if
+           * the preedit was committed, it will insert the resulting
+           * text, which will not trigger this if() any more.
+           */
+
+          GList *list = NULL;
 
-      text_tool->pending = g_list_append (text_tool->pending, (gpointer) pspec);
+         /* if there are pending changes, apply them before applying
+           * preedit stuff directly (bypassing undo)
+           */
+          if (text_tool->pending)
+            {
+              gimp_text_tool_block_drawing (text_tool);
+              gimp_text_tool_apply (text_tool, TRUE);
+            }
 
-      if (text_tool->idle_id)
-        g_source_remove (text_tool->idle_id);
+          gimp_text_tool_block_drawing (text_tool);
 
-      text_tool->idle_id =
-        g_idle_add_full (G_PRIORITY_LOW,
-                         gimp_text_tool_apply_idle, text_tool,
-                         NULL);
+          list = g_list_append (list, (gpointer) pspec);
+          gimp_text_tool_apply_list (text_tool, list);
+          g_list_free (list);
+
+          gimp_text_tool_frame_item (text_tool);
+
+          gimp_image_flush (gimp_item_get_image (GIMP_ITEM (text_tool->layer)));
+
+          gimp_text_tool_unblock_drawing (text_tool);
+        }
+      else
+        {
+          /* else queue the property change for normal processing,
+           * including undo
+           */
+
+          gimp_text_tool_block_drawing (text_tool);
+
+          text_tool->pending = g_list_append (text_tool->pending,
+                                              (gpointer) pspec);
+
+          if (text_tool->idle_id)
+            g_source_remove (text_tool->idle_id);
+
+          text_tool->idle_id =
+            g_idle_add_full (G_PRIORITY_LOW,
+                             gimp_text_tool_apply_idle, text_tool,
+                             NULL);
+        }
     }
 }
 
@@ -1151,6 +1202,10 @@ gimp_text_tool_text_notify (GimpText         *text,
 {
   g_return_if_fail (text == text_tool->text);
 
+  /* an undo cancels all preedit operations */
+  if (text_tool->preedit_active)
+    gimp_text_tool_abort_im_context (text_tool);
+
   gimp_text_tool_block_drawing (text_tool);
 
   if ((pspec->flags & G_PARAM_READWRITE) == G_PARAM_READWRITE)
@@ -1219,8 +1274,6 @@ gimp_text_tool_apply (GimpTextTool *text_tool,
   const GParamSpec *pspec = NULL;
   GimpImage        *image;
   GimpTextLayer    *layer;
-  GObject          *src;
-  GObject          *dest;
   GList            *list;
   gboolean          undo_group = FALSE;
 
@@ -1299,8 +1352,35 @@ gimp_text_tool_apply (GimpTextTool *text_tool,
       gimp_image_undo_push_text_layer (image, NULL, layer, pspec);
     }
 
-  src  = G_OBJECT (text_tool->proxy);
-  dest = G_OBJECT (text_tool->text);
+  gimp_text_tool_apply_list (text_tool, list);
+
+  g_list_free (text_tool->pending);
+  text_tool->pending = NULL;
+
+  if (push_undo)
+    {
+      g_object_set (layer, "modified", FALSE, NULL);
+
+      if (undo_group)
+        gimp_image_undo_group_end (image);
+    }
+
+  gimp_text_tool_frame_item (text_tool);
+
+  gimp_image_flush (image);
+
+  gimp_text_tool_unblock_drawing (text_tool);
+
+  return FALSE;
+}
+
+static void
+gimp_text_tool_apply_list (GimpTextTool *text_tool,
+                           GList        *pspecs)
+{
+  GObject *src  = G_OBJECT (text_tool->proxy);
+  GObject *dest = G_OBJECT (text_tool->text);
+  GList   *list;
 
   g_signal_handlers_block_by_func (dest,
                                    gimp_text_tool_text_notify,
@@ -1311,9 +1391,10 @@ gimp_text_tool_apply (GimpTextTool *text_tool,
 
   g_object_freeze_notify (dest);
 
-  for (; list; list = g_list_next (list))
+  for (list = pspecs; list; list = g_list_next (list))
     {
-      GValue value = G_VALUE_INIT;
+      const GParamSpec *pspec;
+      GValue            value = G_VALUE_INIT;
 
       /*  look ahead and compress changes  */
       if (list->next && list->next->data == list->data)
@@ -1329,9 +1410,6 @@ gimp_text_tool_apply (GimpTextTool *text_tool,
       g_value_unset (&value);
     }
 
-  g_list_free (text_tool->pending);
-  text_tool->pending = NULL;
-
   g_object_thaw_notify (dest);
 
   g_signal_handlers_unblock_by_func (dest,
@@ -1340,22 +1418,6 @@ gimp_text_tool_apply (GimpTextTool *text_tool,
   g_signal_handlers_unblock_by_func (dest,
                                      gimp_text_tool_text_changed,
                                      text_tool);
-
-  if (push_undo)
-    {
-      g_object_set (layer, "modified", FALSE, NULL);
-
-      if (undo_group)
-        gimp_image_undo_group_end (image);
-    }
-
-  gimp_text_tool_frame_item (text_tool);
-
-  gimp_image_flush (image);
-
-  gimp_text_tool_unblock_drawing (text_tool);
-
-  return FALSE;
 }
 
 static void
diff --git a/app/tools/gimptexttool.h b/app/tools/gimptexttool.h
index 0c8a8a1..054a353 100644
--- a/app/tools/gimptexttool.h
+++ b/app/tools/gimptexttool.h
@@ -77,6 +77,7 @@ struct _GimpTextTool
   GtkIMContext   *im_context;
   gboolean        needs_im_reset;
 
+  gboolean        preedit_active;
   gchar          *preedit_string;
   gint            preedit_cursor;
   GtkTextMark    *preedit_start;