GNOME Bugzilla – Bug 787142
Avoid assertion failed warnings from pango_layout_get_cursor_pos() upon completing a preedit using Windows IME (CJK-related)
Last modified: 2017-10-30 06:37:54 UTC
Hi, Since commit f11f989 (GtkEntry: Remove recompute idle) from 3.19.x assertion failed warnings ("Pango-WARNING: Assertion failed: (index >= 0 && index <= layout->length)) from Pango's pango_layout_get_cursur_pos() is displayed whenever I confirm input (i.e. the "commit" signal is emitted after the preedit is done/committed) of Chinese text on Windows, which is done via modules/input/gtkimcontextime.c. This means that this affects the gtk-3-20, gtk-3-22 and master branches. Upon some more examination, it seems that since we do the recomputation directly, the priv->preedit_length and priv->preedit_cursor (which are to compute the index that is to be passed to pango_layout_get_cursur_pos(), did not get reset to 0 (since we are no longer preediting as we are committing the preedit) in time, at least for Windows, so that the value of the index gets beyond layout->length, which would trigger the assertion failed warning, 2 times per preedit commit. As a result, I think, since priv->preedit_length and priv->preedit_cursor becomes no longer relevant during a preedit commit, we should reset those values to 0 upon receiving the "commit" signal in gtkentry.c before doing gtk_entry_enter_text(), as the committed CJK (or so) text is already passed as a parameter when the "commit" signal is emitted by the IM module. I will attach a patch for this shortly. With blessings, thank you!
Created attachment 358953 [details] [review] gtkentry.c: Reset priv->preedit_length and priv->preedit_cursor upon preedit commit Hi, This is the patch to gtkentry.c to fix this issue, at least on Windows--I tried this on Fedora as well with no ill effects. With blessings, thank you!
Review of attachment 358953 [details] [review]: there is a preedit-changed (and preedit-end) signal. input methods are required to emit it when the preedit text changes. this should already take care of resetting those variables. Does the ime input method context not emit those signals ?
Hi Matthias, Yes it does, and I understand that, but it seems that after Timm's change (please see the initial report of this bug) gtk_entry_recompute() (and so gtk_entry_update_handles(), gtk_entry_get_cursor_locations(), which in turn calls pango_layout_get_cursor_pos()) gets called before gets called before the reset, hence the problems here. On the IME input method, the commit signal is emitted before the pair of preedit-changed and preedit-end signals. With blessings, thank you!
Created attachment 359061 [details] [review] input/IME: Defer emitting the "commit" event Hi, After looking through the other input modules that involve using preedit buffers, I think the way to fix this is to defer emitting the commit signal after we emit the pair of preedit-changed and preedit-end signals, so that we are sure that the priv->preedit_length and priv->preedit_cursor gets reset to 0 before we commit the entered string done in preediting (which is from Windows IME). So, this is my stab at fixing this issue. With blessings, thank you!
I'll let somebody with more experience on this to decide whether to take it in.
Matthias, ping
Review of attachment 359061 [details] [review]: double checked this, let's get this in
Review of attachment 359061 [details] [review]: Hi Nacho, Thanks, I pushed the patch as: gtk-3-22: c255ba6 master: 64a489a With blessings, thank you!