Re: Another problem with gtk+/gtk/gtkimmulticontext.c

[Owen Taylor <otaylor redhat com>]

<yaoz vidar niaaa nih gov> writes:

> I have encountered another problem with gtk+/gtk/gtkimmulticontext.c:
> 
> 
> Symptom
> -------
> 
> I am trying to watch the life cycle of an input method, for example,
> 'xim' module comes with GTK+ 2.0.  Switching to 'xim' in gtk+/tests/texttext,
> I see the following calling sequence:
> 
> gtk_im_context_xim_register_type
> gtk_im_context_xim_new
> gtk_im_context_xim_class_init
> gtk_im_context_xim_init
> gtk_im_context_xim_new
> gtk_im_context_xim_init
> ...
> 
> Notice that TWO new GtkIMContextXIMs are created here.

[...]

> Possible Fix
> ------------
> 
> According to log, line 182 was added to 'emit "preedit_changed"
> in case there was still a preedit string'.  Which IM module will handle
> this "preedit_changed" signal? 

Actually, the ::preedit_changed signal is handled by the application/widget, 
not by any input method. It's notification to the application that
the preedit text has changed and it needs to display the new text.

But yes emitting this signal while the slave is unset isn't the
right thing to do.

>  My understanding is it will be the new one we
> are switching to.  Because the new IM module is to be created, it is equivalent
> to clearing the preedit area.  If my understanding is right, then the fix
> could be quite simple:
> 
> index: gtkimmulticontext.c
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gtk/gtkimmulticontext.c,v
> retrieving revision 1.20
> diff -u -r1.20 gtkimmulticontext.c
> --- gtkimmulticontext.c 2002/03/20 21:47:00     1.20
> +++ gtkimmulticontext.c 2002/03/21 19:21:42
> @@ -177,9 +177,6 @@
> 
>        g_object_unref (multicontext->slave);
>        multicontext->slave = NULL;
> -
> -      if (!finalizing)
> -       g_signal_emit_by_name (multicontext, "preedit_changed");
>      }
> 
>    multicontext->slave = slave;
> @@ -209,6 +206,9 @@
> 
>        if (multicontext->client_window)
>         gtk_im_context_set_client_window (slave, multicontext->client_window);
> +
> +      if (!finalizing)
> +       g_signal_emit_by_name (slave, "preedit_changed");
>      }
>  }
> 
> That is, moving emit "preedit_changed" signal to the slave section after
> the slave has been created.

This isn't quite right, because:

 a) It doesn't emit the ::preedit_changed signal under the same conditions
    as the original code.

 b) The signal should be emitted on the multicontext, not the slave,
    since its notification for the application.

But it's close. The patch I'm applying is below.

Thanks,
                                        Owen

Index: gtkimmulticontext.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkimmulticontext.c,v
retrieving revision 1.20
diff -u -p -r1.20 gtkimmulticontext.c
--- gtkimmulticontext.c	20 Mar 2002 21:47:00 -0000	1.20
+++ gtkimmulticontext.c	17 May 2002 19:22:41 -0000
@@ -157,6 +157,8 @@ gtk_im_multicontext_set_slave (GtkIMMult
 			       GtkIMContext      *slave,
 			       gboolean           finalizing)
 {
+  gboolean need_preedit_changed = FALSE;
+  
   if (multicontext->slave)
     {
       if (!finalizing)
@@ -179,7 +181,7 @@ gtk_im_multicontext_set_slave (GtkIMMult
       multicontext->slave = NULL;
 
       if (!finalizing)
-	g_signal_emit_by_name (multicontext, "preedit_changed");
+	need_preedit_changed = TRUE;
     }
   
   multicontext->slave = slave;
@@ -210,6 +212,9 @@ gtk_im_multicontext_set_slave (GtkIMMult
       if (multicontext->client_window)
 	gtk_im_context_set_client_window (slave, multicontext->client_window);
     }
+
+  if (need_preedit_changed)
+    g_signal_emit_by_name (multicontext, "preedit_changed");
 }
 
 static GtkIMContext *