Created attachment 13573 [details] [review]
initilize info->style to zero

Tue Jan 14 21:19:13 2003    <hidetoshi.tajima@sun.com>

	* modules/input/gtkimcontextxim.c (setup_styles):
	initialize info->style to 0 (#103549)

Please review and approve a commit to the HEAD.

Wouldn't 

 XIMPreeditNothing | XIMStatusNothing

be a better initialization value than 0?

Also, shouldn't we print out a warning, since if the
input method can't either work with Callbacks or Nothing,
then we are't going to have correct operation.

What is the list of supported styles for these input methods?

They support 2 x 3 styles - combinations of XIMPreeditNothing or
XIMPreeditNone, and XIMStatusArea, XIMStatusNothing or XIMStatusNone.

I was not sure if using XIMPreeditNothing|XIMStatusNothing would work
or not if it is not in xim's supported input styles, so used 0 to make
the 1st choose_better_style()
call return the style2 value.

As to the warning message, print it when neither style
can be supported, or when XCreateIC() fails? XCreateIC()
calls anyway with XIMPreeditNothing|XIMStatusNothing.

Ah, I misunderstood what you were saying, I thought you
were talking about the case when none of the styles
passed:

 	if ((xim_styles->supported_styles[i] & ALLOWED_MASK) ==
xim_styles->supported_styles[i])

This is the case where I was talking about printing a warning.

My idea to be using Nothing|Nothing is that it should be
worse than any real value, by choose_better_style(), but
still a legitimate value for a return.

Note 1: It looks like to me that choose_better_style() doesn't
   handle XIMPreeditNone correctly.

Note 2: The distinction between Nothing and None is not entirely
   clear to me.

 

Yep, I should have said "gtk's setting style does not match
any of the styles XIM supports" :-) 

For the case none of the style was passed, setup_styles()
does nothing now, so info_im->style is left uninialized.
better to initialize info_im->style o Nothing|Nothing for 
this case as well.

To my understanding, Nothing|Nothing is to let XIM do
whatever way(mostly window location) it wants to show 
preedit/status GUI, so there are still feedbacks to users,
while None|None is for no preedit/status feeback,
but only commit string is sent. [so None|None is not use case after all.]

A new patch is followed.

Created attachment 13763 [details] [review]
2nd patch to fix a uninitialized inputstyle value

Hmmm, if a input method only listed:
 
 PreeditNone|StatusNone

in its list of input methods, wouldn't this patch select
PreeditNothing|StatusNothing, since Nothing is better
than None?

Perhaps we should set it to 0 as your first patch did,
and then if it's still 0 at the end, change it to
Nothing|Nothing?

I think we also need to fix select_better_style() to
handle PreeditNone.

I've made the 3rd patch. I did:
  - handle PreeditNone case in choose_better_style()
  - set info->style to 0 first, and set it to Nothing|Nothing
     if it is still 0 at the end.
  - use XIMPreeditNone or XIMStatusNone if the input method 
    supports them but don't support XIMPreeditNothing or 
    XIMStatusNothing.

I intended we should still prefer to use Nothing rather than None
if the input method support both, samely as we prefer Callback to 
Nothing [unless use_preedit is set to false].



 

Created attachment 13765 [details] [review]
3rd patch handle input method's supported styles properly

Looks fine to commit to me (HEAD and gtk-2-2)

Thanks, will commit the patch to both branches from now.

	* modules/input/gtkimcontextxim.c (setup_styles): fix uninitialization
	error when selecting input style (#103549).