bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#10867: 26.3; XIM preedit/status font handling


From: Yichao Yu
Subject: bug#10867: 26.3; XIM preedit/status font handling
Date: Fri, 21 Aug 2020 00:53:02 -0400

> Thanks.  Can someone with access to a system with this issue and
> sufficient knowledge of what's going on please review this patch?

And some new findings and a more aggressive and robust fix.

1. It's fine to set status area and preedit position even without the
corresponding xim style set. The input method will handle that just
fine. In emacs, doing that for status area cause error due to
uninitialized memory (`needed` is not initialized in
`xic_set_statusarea` under such condition) so that one still needs to
be done conditionally.
2. Without XIMPreeditPosition and XIMStatusArea, xlib will not require
the fonts to be set and there's no issue with not finding font or
finding too many fonts anymore.

All in all, the following patch fixes all the issues cleanly.
Note that this is exactly what gtk3 does
(https://github.com/GNOME/gtk/blob/de04aaf82db8d694af7d42ab6bb2e26d3ef0c947/modules/input/gtkimcontextxim.c#L183),
i.e. it does not accept either XIMPreeditPosition or XIMStatusArea.
(It does accept callback but that is for "on-the-spot" style I believe
and is not implemented in emacs anyway.) That's why I expect this to
work with all modern input methods.

The support for cursor following (xic_set_preeditarea) as well as the
problematic approach involving the font was introduced in
https://github.com/emacs-mirror/emacs/commit/5a7df7d75faa0f8921fd6a9591cdf6836d89941b
20.5 years ago.

```
diff --git a/src/xfns.c b/src/xfns.c
index 78f977bf0a..ef4e2abfa5 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -2322,23 +2322,6 @@ hack_wm_protocols (struct frame *f, Widget widget)
static XIMStyle best_xim_style (XIMStyles *);


-/* Supported XIM styles, ordered by preference.  */
-
-static const XIMStyle supported_xim_styles[] =
-{
-  XIMPreeditPosition | XIMStatusArea,
-  XIMPreeditPosition | XIMStatusNothing,
-  XIMPreeditPosition | XIMStatusNone,
-  XIMPreeditNothing | XIMStatusArea,
-  XIMPreeditNothing | XIMStatusNothing,
-  XIMPreeditNothing | XIMStatusNone,
-  XIMPreeditNone | XIMStatusArea,
-  XIMPreeditNone | XIMStatusNothing,
-  XIMPreeditNone | XIMStatusNone,
-  0,
-};
-
-
#if defined HAVE_X_WINDOWS && defined USE_X_TOOLKIT
/* Create an X fontset on frame F with base font name BASE_FONTNAME.  */

@@ -2622,15 +2605,8 @@ xic_free_xfontset (struct frame *f)
static XIMStyle
best_xim_style (XIMStyles *xim)
{
-  int i, j;
-  int nr_supported = ARRAYELTS (supported_xim_styles);
-
-  for (i = 0; i < nr_supported; ++i)
-    for (j = 0; j < xim->count_styles; ++j)
-      if (supported_xim_styles[i] == xim->supported_styles[j])
-       return supported_xim_styles[i];
-
-  /* Return the default style.  */
+  /* Return the default style. This is what GTK3 uses and
+     should work fine with all modern input methods.  */
  return XIMPreeditNothing | XIMStatusNothing;
}

diff --git a/src/xterm.c b/src/xterm.c
index 2e0407aff4..0a242ad214 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -9704,7 +9704,7 @@ x_draw_window_cursor (struct window *w, struct
glyph_row *glyph_row, int x,

#ifdef HAVE_X_I18N
      if (w == XWINDOW (f->selected_window))
-       if (FRAME_XIC (f) && (FRAME_XIC_STYLE (f) & XIMPreeditPosition))
+       if (FRAME_XIC (f))
         xic_set_preeditarea (w, x, y);
#endif
    }
@@ -10387,11 +10387,8 @@ xim_instantiate_callback (Display *display,
XPointer client_data, XPointer call_
               create_frame_xic (f);
               if (FRAME_XIC_STYLE (f) & XIMStatusArea)
                 xic_set_statusarea (f);
-               if (FRAME_XIC_STYLE (f) & XIMPreeditPosition)
-                 {
-                   struct window *w = XWINDOW (f->selected_window);
-                   xic_set_preeditarea (w, w->cursor.x, w->cursor.y);
-                 }
+               struct window *w = XWINDOW (f->selected_window);
+               xic_set_preeditarea (w, w->cursor.x, w->cursor.y);
             }
       }

```





reply via email to

[Prev in Thread] Current Thread [Next in Thread]