XIM reorganization and cleanup; see [Patch 1919791] for details.

7 files changed, 124 insertions, 226 deletions

diff --git a/ChangeLog b/ChangeLog
index 4e64393..c4ec21f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2008-03-26  Joe English  <jenglish@users.sourceforge.net>
+
+	* generic/tkInt.h, generic/tkEvent.c, unix/tkUnixEvent.c, 
+	unix/tkUnixKey.c: XIM reorganization and cleanup; see
+	[Patch 1919791] for details.
+
 2008-03-21  Joe English  <jenglish@users.sourceforge.net>
 
 	* generic/tk.decls, generic/ttk/ttkStubLib.c, unix/Makefile.in:
diff --git a/generic/tkEvent.c b/generic/tkEvent.c
index fda8ac9..ffc8484 100644
--- a/generic/tkEvent.c
+++ b/generic/tkEvent.c
@@ -12,7 +12,7 @@
  * See the file "license.terms" for information on usage and redistribution of
  * this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: @(#) $Id: tkEvent.c,v 1.34 2006/11/24 18:04:14 jenglish Exp $
+ * RCS: @(#) $Id: tkEvent.c,v 1.35 2008/03/26 19:04:09 jenglish Exp $
  */
 
 #include "tkInt.h"
@@ -210,9 +210,7 @@ static void		UpdateButtonEventState(XEvent *eventPtr);
 static int		WindowEventProc(Tcl_Event *evPtr, int flags);
 #ifdef TK_USE_INPUT_METHODS
 static int		InvokeInputMethods(TkWindow *winPtr, XEvent *eventPtr);
-#if TK_XIM_SPOT
-static void		CreateXIMSpotMethods(TkWindow *winPtr);
-#endif /* TK_XIM_SPOT */
+static void		CreateXIC(TkWindow *winPtr);
 #endif /* TK_USE_INPUT_METHODS */
 
 /*
@@ -319,69 +317,52 @@ InvokeMouseHandlers(
 /*
  *----------------------------------------------------------------------
  *
- * CreateXIMSpotMethods --
+ * CreateXIC --
  *
- *	Create the X input methods for our winPtr. XIM is only ever enabled on
- *	Unix.
- *
- * Results:
- *	None.
- *
- * Side effects:
- *	An input context is created or we Tcl_Panic.
+ *	Create the X input context for our winPtr.
+ *	XIM is only ever enabled on Unix.
  *
  *----------------------------------------------------------------------
  */
 
-#if defined(TK_USE_INPUT_METHODS) && TK_XIM_SPOT
+#if defined(TK_USE_INPUT_METHODS)
 static void
-CreateXIMSpotMethods(
+CreateXIC(
     TkWindow *winPtr)
 {
     TkDisplay *dispPtr = winPtr->dispPtr;
+    long im_event_mask = 0L;
+    const char *preedit_attname = NULL;
+    XVaNestedList preedit_attlist = NULL;
 
-    if (dispPtr->flags & TK_DISPLAY_XIM_SPOT) {
-	XVaNestedList preedit_attr;
+    if (dispPtr->inputStyle & XIMPreeditPosition) {
 	XPoint spot = {0, 0};
 
-	if (dispPtr->inputXfs == NULL) {
-	    /*
-	     * We only need to create one XFontSet
-	     */
+	preedit_attname = XNPreeditAttributes;
+	preedit_attlist = XVaCreateNestedList(0,
+		XNSpotLocation, &spot,
+		XNFontSet, dispPtr->inputXfs,
+		NULL);
+    }
 
-	    char **missing_list;
-	    int missing_count;
-	    char *def_string;
+    winPtr->inputContext = XCreateIC(dispPtr->inputMethod,
+	    XNInputStyle, dispPtr->inputStyle,
+	    XNClientWindow, winPtr->window,
+	    XNFocusWindow, winPtr->window,
+	    preedit_attname, preedit_attlist,
+	    NULL);
 
-	    dispPtr->inputXfs = XCreateFontSet(dispPtr->display,
-		    "-*-*-*-R-Normal--14-130-75-75-*-*",
-		    &missing_list, &missing_count, &def_string);
-	    if (missing_count > 0) {
-		XFreeStringList(missing_list);
-	    }
-	}
+    if (preedit_attlist) {
+	XFree(preedit_attlist);
+    }
 
-	preedit_attr = XVaCreateNestedList(0, XNSpotLocation,
-		&spot, XNFontSet, dispPtr->inputXfs, NULL);
-	if (winPtr->inputContext != NULL) {
-	    Tcl_Panic("inputContext not NULL");
-	}
-	winPtr->inputContext = XCreateIC(dispPtr->inputMethod,
-		XNInputStyle, XIMPreeditPosition|XIMStatusNothing,
-		XNClientWindow, winPtr->window,
-		XNFocusWindow, winPtr->window,
-		XNPreeditAttributes, preedit_attr,
-		NULL);
-	XFree(preedit_attr);
-    } else {
-	if (winPtr->inputContext != NULL) {
-	    Tcl_Panic("inputContext not NULL");
-	}
-	winPtr->inputContext = XCreateIC(dispPtr->inputMethod,
-		XNInputStyle, XIMPreeditNothing|XIMStatusNothing,
-		XNClientWindow, winPtr->window,
-		XNFocusWindow, winPtr->window,
-		NULL);
+    /*
+     * Adjust the window's event mask if the IM requires it.
+     */
+    XGetICValues(winPtr->inputContext, XNFilterEvents, &im_event_mask, NULL);
+    if ((winPtr->atts.event_mask & im_event_mask) != im_event_mask) {
+	winPtr->atts.event_mask |= im_event_mask;
+	XSelectInput(winPtr->display, winPtr->window, winPtr->atts.event_mask);
     }
 }
 #endif
@@ -397,8 +378,7 @@ CreateXIMSpotMethods(
  *	context).
  *
  *	When the event is a FocusIn event, set the input context focus to the
- *	receiving window. This is needed for certain versions of Solaris, but
- *	we are still not sure whether it is being done in the right way.
+ *	receiving window.
  *
  * Results:
  *	1 when we are done with the event.
@@ -419,38 +399,24 @@ InvokeInputMethods(
     TkDisplay *dispPtr = winPtr->dispPtr;
 
     if ((dispPtr->flags & TK_DISPLAY_USE_IM)) {
-	long im_event_mask = 0L;
 	if (!(winPtr->flags & (TK_CHECKED_IC|TK_ALREADY_DEAD))) {
 	    winPtr->flags |= TK_CHECKED_IC;
 	    if (dispPtr->inputMethod != NULL) {
-#if TK_XIM_SPOT
-		CreateXIMSpotMethods(winPtr);
-#else
-		if (winPtr->inputContext != NULL) {
-		    Tcl_Panic("inputContext not NULL");
-		}
-		winPtr->inputContext = XCreateIC(dispPtr->inputMethod,
-			XNInputStyle, XIMPreeditNothing|XIMStatusNothing,
-			XNClientWindow, winPtr->window,
-			XNFocusWindow, winPtr->window,
-			NULL);
-#endif
-	    }
-	}
-	if (winPtr->inputContext != NULL &&
-	    (eventPtr->xany.type == FocusIn)) {
-	    XGetICValues(winPtr->inputContext,
-		    XNFilterEvents, &im_event_mask, NULL);
-	    if (im_event_mask != 0L) {
-		XSelectInput(winPtr->display, winPtr->window,
-			winPtr->atts.event_mask | im_event_mask);
-		XSetICFocus(winPtr->inputContext);
+		CreateXIC(winPtr);
 	    }
 	}
-	if (eventPtr->type == KeyPress || eventPtr->type == KeyRelease) {
-	    if (XFilterEvent(eventPtr, None)) {
-		return 1;
-	    }
+	switch (eventPtr->type) {
+	    case FocusIn:
+		if (winPtr->inputContext != NULL) {
+		    XSetICFocus(winPtr->inputContext);
+		}
+		break;
+	    case KeyPress:
+	    case KeyRelease:
+		if (XFilterEvent(eventPtr, None)) {
+		    return 1;
+		}
+		break;
 	}
     }
     return 0;
diff --git a/generic/tkInt.h b/generic/tkInt.h
index 2186089..7ae4bee 100644
--- a/generic/tkInt.h
+++ b/generic/tkInt.h
@@ -11,7 +11,7 @@
  * See the file "license.terms" for information on usage and redistribution of
  * this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: $Id: tkInt.h,v 1.81 2007/12/13 15:24:14 dgp Exp $
+ * RCS: $Id: tkInt.h,v 1.82 2008/03/26 19:04:09 jenglish Exp $
  */
 
 #ifndef _TKINT
@@ -145,16 +145,6 @@ typedef struct TkCursor {
 } TkCursor;
 
 /*
- * This defines whether we should try to use XIM over-the-spot style input.
- * Allow users to override it. It is a much more elegant use of XIM, but uses
- * a bit more memory.
- */
-
-#ifndef TK_XIM_SPOT
-#define TK_XIM_SPOT	1
-#endif
-
-/*
  * The following structure is kept one-per-TkDisplay to maintain information
  * about the caret (cursor location) on this display. This is used to dictate
  * global focus location (Windows Accessibility guidelines) and to position
@@ -530,9 +520,8 @@ typedef struct TkDisplay {
 
 #ifdef TK_USE_INPUT_METHODS
     XIM inputMethod;		/* Input method for this display. */
-#if TK_XIM_SPOT
+    XIMStyle inputStyle;	/* Input style selected for this display. */
     XFontSet inputXfs;		/* XFontSet cached for over-the-spot XIM. */
-#endif
 #endif /* TK_USE_INPUT_METHODS */
     Tcl_HashTable winTable;	/* Maps from X window ids to TkWindow ptrs. */
 
@@ -572,8 +561,6 @@ typedef struct TkDisplay {
  *	Indicates that we should collapse motion events on this display
  *  TK_DISPLAY_USE_IM:			(default on, set via tk.tcl)
  *	Whether to use input methods for this display
- *  TK_DISPLAY_XIM_SPOT:		(default off)
- *	Indicates that we should use over-the-spot XIM on this display
  *  TK_DISPLAY_WM_TRACING:		(default off)
  *	Whether we should do wm tracing on this display.
  *  TK_DISPLAY_IN_WARP:			(default off)
@@ -582,7 +569,6 @@ typedef struct TkDisplay {
 
 #define TK_DISPLAY_COLLAPSE_MOTION_EVENTS	(1 << 0)
 #define TK_DISPLAY_USE_IM			(1 << 1)
-#define TK_DISPLAY_XIM_SPOT			(1 << 2)
 #define TK_DISPLAY_WM_TRACING			(1 << 3)
 #define TK_DISPLAY_IN_WARP			(1 << 4)
 
diff --git a/unix/tcl.m4 b/unix/tcl.m4
index 3a13ea3..9476cae 100644
--- a/unix/tcl.m4
+++ b/unix/tcl.m4
@@ -1454,10 +1454,6 @@ dnl AC_CHECK_TOOL(AR, ar)
 	    # files in compat/*.c is being linked in.
 
 	    AS_IF([test x"${USE_COMPAT}" != x],[CFLAGS="$CFLAGS -fno-inline"])
-
-	    # XIM peeking works under XFree86.
-	    AC_DEFINE(PEEK_XCLOSEIM, 1, [May we use XIM peeking safely?])
-
 	    ;;
 	GNU*)
 	    SHLIB_CFLAGS="-fPIC"
diff --git a/unix/tkConfig.h.in b/unix/tkConfig.h.in
index 819efff..285bb38 100644
--- a/unix/tkConfig.h.in
+++ b/unix/tkConfig.h.in
@@ -130,9 +130,6 @@
 /* Define to the version of this package. */
 #undef PACKAGE_VERSION
 
-/* May we use XIM peeking safely? */
-#undef PEEK_XCLOSEIM
-
 /* Is this a static build? */
 #undef STATIC_BUILD
 
diff --git a/unix/tkUnixEvent.c b/unix/tkUnixEvent.c
index ab95290..bb66552 100644
--- a/unix/tkUnixEvent.c
+++ b/unix/tkUnixEvent.c
@@ -9,7 +9,7 @@
  * See the file "license.terms" for information on usage and redistribution of
  * this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: @(#) $Id: tkUnixEvent.c,v 1.26 2007/12/13 15:28:50 dgp Exp $
+ * RCS: @(#) $Id: tkUnixEvent.c,v 1.27 2008/03/26 19:04:10 jenglish Exp $
  */
 
 #include "tkUnixInt.h"
@@ -25,18 +25,6 @@ typedef struct ThreadSpecificData {
 } ThreadSpecificData;
 static Tcl_ThreadDataKey dataKey;
 
-#if defined(TK_USE_INPUT_METHODS) && defined(PEEK_XCLOSEIM)
-/*
- * Structure used to peek into internal XIM data structure. This is only known
- * to work with XFree86.
- */
-
-struct XIMPeek {
-    void *junk1, *junk2;
-    XIC ic_chain;
-};
-#endif
-
 /*
  * Prototypes for functions that are referenced only in this file:
  */
@@ -173,54 +161,10 @@ TkpCloseDisplay(
     TkWmCleanup(dispPtr);
 
 #ifdef TK_USE_INPUT_METHODS
-#if TK_XIM_SPOT
     if (dispPtr->inputXfs) {
 	XFreeFontSet(dispPtr->display, dispPtr->inputXfs);
     }
-#endif
     if (dispPtr->inputMethod) {
-	/*
-	 * Calling XCloseIM with an input context that has not been freed can
-	 * cause a crash. This crash has been reproduced under Linux systems
-	 * with XFree86 3.3 and may have also been seen under Solaris 2.3. The
-	 * crash is caused by a double free of memory inside the X library.
-	 * Memory that was already deallocated may be accessed again inside
-	 * XCloseIM. This bug can be avoided by making sure that a call to
-	 * XDestroyIC is made for each XCreateIC call. This bug has been fixed
-	 * in XFree86 4.2.99.2. The internal layout of the XIM structure
-	 * changed in the XFree86 4.2 release so the test should not be run
-	 * for with these new releases.
-	 */
-
-#if defined(TK_USE_INPUT_METHODS) && defined(PEEK_XCLOSEIM)
-	int do_peek = 0;
-	struct XIMPeek *peek;
-
-	if (strstr(ServerVendor(dispPtr->display), "XFree86")) {
-	    int vendrel = VendorRelease(dispPtr->display);
-
-	    if (vendrel < 336) {
-		/* 3.3.4 and 3.3.5 */
-		do_peek = 1;
-	    } else if (vendrel < 3900) {
-		/* Other 3.3.x versions */
-		do_peek = 1;
-	    } else if (vendrel < 40000000) {
-		/* 4.0.x versions */
-		do_peek = 1;
-	    } else {
-		/* Newer than 4.0 */
-		do_peek = 0;
-	    }
-	}
-
-	if (do_peek) {
-	    peek = (struct XIMPeek *) dispPtr->inputMethod;
-	    if (peek->ic_chain != NULL) {
-		Tcl_Panic("input contexts not freed before XCloseIM");
-	    }
-	}
-#endif
 	XCloseIM(dispPtr->inputMethod);
     }
 #endif
@@ -621,9 +565,7 @@ TkpSync(
  *
  * OpenIM --
  *
- *	Tries to open an X input method, associated with the given display.
- *	Right now we can only deal with a bare-bones input style: no preedit,
- *	and no status.
+ *	Tries to open an X input method associated with the given display.
  *
  * Results:
  *	Stores the input method in dispPtr->inputMethod; if there isn't a
@@ -639,11 +581,12 @@ static void
 OpenIM(
     TkDisplay *dispPtr)		/* Tk's structure for the display. */
 {
-    unsigned short i;
+    int i;
     XIMStyles *stylePtr;
+    XIMStyle bestStyle = 0;
 
     if (XSetLocaleModifiers("") == NULL) {
-	goto error;
+	return;
     }
 
     dispPtr->inputMethod = XOpenIM(dispPtr->display, NULL, NULL, NULL);
@@ -656,38 +599,45 @@ OpenIM(
 	goto error;
     }
 
-#if TK_XIM_SPOT
     /*
-     * If we want to do over-the-spot XIM, we have to check that this mode is
-     * supported. If not we will fall-through to the check below.
+     * Select the best input style supported by both the IM and Tk.
      */
-
     for (i = 0; i < stylePtr->count_styles; i++) {
-	if (stylePtr->supported_styles[i]
-		== (XIMPreeditPosition | XIMStatusNothing)) {
-	    dispPtr->flags |= TK_DISPLAY_XIM_SPOT;
-	    XFree(stylePtr);
-	    return;
+	XIMStyle thisStyle = stylePtr->supported_styles[i];
+	if (thisStyle == (XIMPreeditPosition | XIMStatusNothing)) {
+	    bestStyle = thisStyle;
+	    break;
+	} else if (thisStyle == (XIMPreeditNothing | XIMStatusNothing)) {
+	    bestStyle = thisStyle;
 	}
     }
-#endif /* TK_XIM_SPOT */
+    XFree(stylePtr);
+    if (bestStyle == 0) {
+	goto error;
+    }
 
-    for (i = 0; i < stylePtr->count_styles; i++) {
-	if (stylePtr->supported_styles[i]
-		== (XIMPreeditNothing | XIMStatusNothing)) {
-	    XFree(stylePtr);
-	    return;
+    dispPtr->inputStyle = bestStyle;
+
+    /*
+     * Create an XFontSet for preedit area.
+     */
+    if (dispPtr->inputStyle & XIMPreeditPosition) {
+	char **missing_list;
+	int missing_count;
+	char *def_string;
+
+	dispPtr->inputXfs = XCreateFontSet(dispPtr->display,
+		"-*-*-*-R-Normal--14-130-75-75-*-*",
+		&missing_list, &missing_count, &def_string);
+	if (missing_count > 0) {
+	    XFreeStringList(missing_list);
 	}
     }
-    XFree(stylePtr);
 
-  error:
-    if (dispPtr->inputMethod) {
-	/*
-	 * This call should not suffer from any core dumping problems since we
-	 * have not allocated any input contexts.
-	 */
+    return;
 
+error:
+    if (dispPtr->inputMethod) {
 	XCloseIM(dispPtr->inputMethod);
 	dispPtr->inputMethod = NULL;
     }
diff --git a/unix/tkUnixKey.c b/unix/tkUnixKey.c
index 7dc91da..4d422cf 100644
--- a/unix/tkUnixKey.c
+++ b/unix/tkUnixKey.c
@@ -9,7 +9,7 @@
  * See the file "license.terms" for information on usage and redistribution of
  * this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: @(#) $Id: tkUnixKey.c,v 1.12 2007/02/27 14:52:57 dkf Exp $
+ * RCS: @(#) $Id: tkUnixKey.c,v 1.13 2008/03/26 19:04:10 jenglish Exp $
  */
 
 #include "tkInt.h"
@@ -24,16 +24,9 @@
  * Tk_SetCaretPos --
  *
  *	This enables correct placement of the XIM caret. This is called by
- *	widgets to indicate their cursor placement, and the caret location is
- *	used by TkpGetString to place the XIM caret. This is currently only
+ *	widgets to indicate their cursor placement.  This is currently only
  *	used for over-the-spot XIM.
  *
- * Results:
- *	None
- *
- * Side effects:
- *	None
- *
  *----------------------------------------------------------------------
  */
 
@@ -44,16 +37,42 @@ Tk_SetCaretPos(
     int y,
     int height)
 {
-    TkCaret *caretPtr = &(((TkWindow *) tkwin)->dispPtr->caret);
+    TkWindow *winPtr = (TkWindow *) tkwin;
+    TkDisplay *dispPtr = winPtr->dispPtr;
+
+    if (   dispPtr->caret.winPtr == winPtr
+	&& dispPtr->caret.x == x
+	&& dispPtr->caret.y == y
+	&& dispPtr->caret.height == height) 
+    {
+	return;
+    }
+
+    dispPtr->caret.winPtr = winPtr;
+    dispPtr->caret.x = x;
+    dispPtr->caret.y = y;
+    dispPtr->caret.height = height;
 
+#ifdef TK_USE_INPUT_METHODS
     /*
-     * Use height for best placement of the XIM over-the-spot box.
+     * Adjust the XIM caret position.
      */
+    if (   (dispPtr->flags & TK_DISPLAY_USE_IM)
+	&& (dispPtr->inputStyle & XIMPreeditPosition)
+	&& (winPtr->inputContext != NULL) )
+    {
+	XVaNestedList preedit_attr;
+	XPoint spot;
 
-    caretPtr->winPtr = ((TkWindow *) tkwin);
-    caretPtr->x = x;
-    caretPtr->y = y;
-    caretPtr->height = height;
+	spot.x = dispPtr->caret.x;
+	spot.y = dispPtr->caret.y + dispPtr->caret.height;
+	preedit_attr = XVaCreateNestedList(0, XNSpotLocation, &spot, NULL);
+	XSetICValues(winPtr->inputContext,
+		XNPreeditAttributes, preedit_attr,
+		NULL);
+	XFree(preedit_attr);
+    }
+#endif
 }
 
 /*
@@ -85,9 +104,6 @@ TkpGetString(
     int len;
     Tcl_DString buf;
     Status status;
-#ifdef TK_USE_INPUT_METHODS
-    TkDisplay *dispPtr = winPtr->dispPtr;
-#endif
 
     /*
      * Overallocate the dstring to the maximum stack amount.
@@ -97,13 +113,9 @@ TkpGetString(
     Tcl_DStringSetLength(&buf, TCL_DSTRING_STATIC_SIZE-1);
 
 #ifdef TK_USE_INPUT_METHODS
-    if ((dispPtr->flags & TK_DISPLAY_USE_IM)
+    if ((winPtr->dispPtr->flags & TK_DISPLAY_USE_IM)
 	    && (winPtr->inputContext != NULL)
 	    && (eventPtr->type == KeyPress)) {
-#if TK_XIM_SPOT
-	XVaNestedList preedit_attr;
-	XPoint spot;
-#endif
 
 	len = XmbLookupString(winPtr->inputContext, &eventPtr->xkey,
 		Tcl_DStringValue(&buf), Tcl_DStringLength(&buf), NULL,
@@ -121,21 +133,6 @@ TkpGetString(
 	    len = 0;
 	}
 
-#if TK_XIM_SPOT
-	/*
-	 * Adjust the XIM caret position. We might want to check that this is
-	 * the right caret.winPtr as well.
-	 */
-
-	if (dispPtr->flags & TK_DISPLAY_XIM_SPOT) {
-	    spot.x = dispPtr->caret.x;
-	    spot.y = dispPtr->caret.y + dispPtr->caret.height;
-	    preedit_attr = XVaCreateNestedList(0, XNSpotLocation, &spot, NULL);
-	    XSetICValues(winPtr->inputContext,
-		    XNPreeditAttributes, preedit_attr, NULL);
-	    XFree(preedit_attr);
-	}
-#endif
     } else {
 	len = XLookupString(&eventPtr->xkey, Tcl_DStringValue(&buf),
 		Tcl_DStringLength(&buf), NULL, NULL);