195944 - AIX doesn't support Mozilla's default on-the-spot editing style

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Philip K. Warren]

While the patch for Bug 157394 prevented the Gdk-Warning message from being
printed, it is not the correct patch for AIX. For all input methods that support
it, we should default to over-the-spot editing style (0x0104 -
GDK_IM_STATUS_AREA | GDK_IM_PREEDIT_POSITION). With the current patch for Bug
157394, we seem to be choosing an input method at random - only selecting the
first one that matches supported styles:

http://makeashorterlink.com/?V50322FA3

Because of this, in several locales we end up choosing style (0x0101 -
GDK_IM_STATUS_AREA | GDK_IM_PREEDIT_AREA), which causes the entire Mozilla
window to turn white and be used to enter the preedit string.

I will attach a patch which fixes the problems found in Bug 157394 (Gdk-Warning
error messages), which also uses the proper over-the-spot editing style (0x0104)
on AIX if it is supported. In addition, I have added support for setting the
xim.preedit.input_style and xim.status.input_style to area from prefs.js.

Comment 1

[Philip K. Warren]

Attachment: Patch

Backs out changes from Bug 157394, sets preferred status input style to
GDK_IM_STATUS_AREA and sets preferred preedit input style to
GDK_IM_PREEDIT_POSITION. Also support the "area" value for
"xim.preedit.input_style" and "xim.status.input_style" preferences.

Comment 2

[Philip K. Warren]

Reassigning bug to me.

Comment 3

[Philip K. Warren]

Comment on attachment 116222 [details] [diff] [review]
Patch

Seeking r/sr.

Comment 4

[Philip K. Warren]

Comment on attachment 116222 [details] [diff] [review]
Patch

blizzard requested that [e-mail] review this patch.

Comment 5

[Masaki Katakai]

Hi Philip,

I don't have AIX platform here so I have a question for you:

AIX supports only GDK_IM_STATUS_AREA? I understand that
over-the-spot is GDK_IM_PREEDIT_POSITION & GDK_IM_STATUS_NOTHING
for Mozilla. My question is that why you need to use STATUS_AREA?

As you know, I understand that we need to prepare status_area
in mozilla side if we want to use STATUS_AREA e.g. CDE/Motif
toolkit has an additional line at bottom of each window.

I believe #ifdef HPUX codes below are the best for AIX also.
The codes will try to the preferred style first for the platform.
If fails, it will fallback. Are these codes working for AIX?

1215 #ifdef HPUX 
1216   preferred_preedit_style = (GdkIMStyle) GDK_IM_PREEDIT_POSITION;
1217   preferred_status_style = (GdkIMStyle) GDK_IM_STATUS_NOTHING;
1218   style = gdk_im_decide_style((GdkIMStyle)(preferred_preedit_style |
preferred_status_style));

I'd like to ask you to try the following and update the patch,
- try ifdef HPUX codes
- do not add "area" preferences, Mozilla does not have
  status area for STATUS_AREA style

Comment 6

[Philip K. Warren]

The majority of AIX languages which require a preedit area support the 
following input styles (I have included the supported input styles for ja_JP 
and zh_CN as examples):

ja_JP
0x0101: IM_STATUS_AREA | IM_PREEDIT_AREA
0x0104: IM_STATUS_AREA | IM_PREEDIT_POSITION
0x0202: IM_STATUS_CALLBACKS | IM_PREEDIT_CALLBACKS
0x0408: IM_STATUS_NOTHING | IM_PREEDIT_NOTHING

zh_CN
0x0101: IM_STATUS_AREA | IM_PREEDIT_AREA
0x0104: IM_STATUS_AREA | IM_PREEDIT_POSITION
0x0202: IM_STATUS_CALLBACKS | IM_PREEDIT_CALLBACKS
0x0408: IM_STATUS_NOTHING | IM_PREEDIT_NOTHING

The default input style for all CDE/Motif applications (including Netscape 
Communicator) is 0x0104. Without the patch from Bug 157394, Mozilla defaults to 
the 0x0202 style which causes several GDK warnings to appear on the command 
line. With the patch from Bug 157394, the style is seemingly chosen at random 
based on the first style returned from the XGetIMValues function call which 
matches a valid style (this is predominantly the 0x0101 style, which causes the 
entire Mozilla window to be used as the preedit area).

If I use the HPUX settings (0x0404), it will not be detected as a supported 
style, so Mozilla will use the default 0x0202 input style which caused the 
problems described in Bug 157394.

The best solution for AIX at the current time is to use the 0x0104 input style, 
which at least gives us a working preedit window. This does have the drawback 
that we do not currently provide a working status area.

What is involved in implementing support for GDK_IM_STATUS_AREA in Mozilla? I 
would be fine with removing the portions of the patch which add support for 
the "area" type in the preferences until it is supported correctly in Mozilla.

Thanks for the quick review.

Comment 7

[Masaki Katakai]

Thanks, I understand that AIX supports only

  0x0104: IM_STATUS_AREA | IM_PREEDIT_POSITION

combination, so we do not set IM_STATUS_NOTHING | IM_PREEDIT_POSITION.

But the interesting is that XIM server said 

  0x0202: IM_STATUS_CALLBACKS | IM_PREEDIT_CALLBACKS

is supported, which is the default input style of Mozilla,
but the gdk library shows error.

Can you check the version of AIX's GTK?

When we started implementation of XIM codes for Mozilla,
we were using gtk+1.2.3 which did not accept XIM callbacks
setting at creation. So, in current implementation, we set
the the attributes after IC creation. But current gtk+1.2.10
seems to accept at creation. I'll try to evaluate.

Comment 8

[Masaki Katakai]

Sorry, my comment is not correct. Even gtk+1.2.10 does not
accept callbacks setting at creation.

Is it possible for you to debug your GTK?

I understand Mozilla will not support "area" status window
because major/popular desktop environments (GNOME/KDE) are
now using callback+callback (GNOME2) and position+nothing
(GNOME1, KDE).

Comment 9

[Philip K. Warren]

I have some additional information to share. It appears that AIX will not create
an X input context for a style of type GDK_IM_STATUS_CALLBACKS |
GDK_IM_PREEDIT_CALLBACKS without specifying the callback functions. This is
where the GDK warnings are coming from. This makes things especially difficult
because GTK 1.2 does not support specifying callbacks at creation time. It
looks like what we currently do for the Mozilla + GTK1 version is:

1) Create X input context with gdk_ic_new with GDK_IM_PREEDIT_CALLBACKS
and GDK_IM_STATUS_CALLBACKS set, but we do not specify callback
functions because it is impossible to do so in GTK 1.2.

2) After X input context has been created, we manually set up the
callback functions with a call to XSetICValues.

This doesn't work on AIX, because we are unable to get an input context
in step #1. This causes the following error messages to be printed on
the console every time we try and create an input context:

Argument is shortage 

Gdk-WARNING **: can not create input context with specified input style.

It seems that what we are currently doing in Mozilla violates the X standard,
but I am not certain. Here is the section of the standard dealing with the
XCreateIC function call:

"The XCreateIC function creates a context within the specified input method.
Some of the arguments are mandatory at creation time, and the input context will
not be created if those arguments are not provided. The mandatory arguments are
the input style and the set of text callbacks (if the input style selected
requires callbacks). All other input context values can be set later."

I have been experimenting with the following on AIX:

1) Create dummy GdkIC with style GDK_IM_STATUS_NOTHING | GDK_IM_PREEDIT_NOTHING.
2) Find GDK's XIM by calling XIMOfIC(((GdkICPrivate*)mIC)->xic).
3) Destroy dummy XIC by calling  XDestroyIC(((GdkICPrivate*)mIC)->xic).
4) Create new XIC supporting GDK_IM_STATUS_CALLBACKS | GDK_IM_PREEDIT_CALLBACKS
style with XCreateIC (specifying all callback functions) and assign it into
((GdkICPrivate*)mIC)->xic.

This seems to be our only solution at this point if we want to support
GDK_IM_STATUS_CALLBACKS | GDK_IM_PREEDIT_CALLBACKS with GTK 1.2, but I have not
yet gotten this method to work properly. Do you have any additional suggestions?

Comment 10

[Philip K. Warren]

Attachment: Patch v2

This is what is required to get the default GDK_IM_STATUS_CALLBACKS |
GDK_IM_PREEDIT_CALLBACKS working on AIX.

What I had to do:
1) Add additional callback functions - on AIX all callback functions must be
specified. We were missing callbacks for XNPreeditCaretCallback,
XNStatusStartCallback, and XNStatusDoneCallback.
2) Create a dummy XIC with gdk_ic_new since AIX requires all callback functions
be set in XCreateIC but GTK doesn't support it with GTK 1.2.
3) Swap out the XIC created with gdk_ic_new with a valid XIC supporting
GDK_IM_PREEDIT_CALLBACKS and GDK_IM_STATUS_CALLBACKS.

Comment 11

[Philip K. Warren]

Comment on attachment 116576 [details] [diff] [review]
Patch v2

Requesting r/sr.

Comment 12

[Masaki Katakai]

Comment on attachment 116576 [details] [diff] [review]
Patch v2

I've verified thes codes and actually it works on  Solaris platform with #ifdef
_AIX. I think it's the best workaround for now because there is no plan to
support status area in Mozilla.

There is one thing I want to ask you to change.

   GdkICPrivate *IC = (GdkICPrivate *)gdk_ic_new(attr, attrmask);

may return NULL, also	  ((GdkICPrivate*) IC)->xic may be NULL if creation of
xic fails.

So,

   if (!IC || !((GdkICPrivate *) IC)->xic) {
     return;
   }

should be before your XDestroyIC(xic) and XCreateIC().

Comment 13

[Philip K. Warren]

Attachment: Patch v3

1) Check that IC and IC->xic are not null before performing the XDestroyIC and
the XCreateIC.
2) Use temporary style GDK_IM_PREEDIT_POSITION | GDK_IM_STATUS_AREA instaed of
GDK_IM_PREEDIT_NOTHING | GDK_IM_STATUS_NOTHING, because the latter pops up a
root window temporarily (between the gdk_ic_new and XDestroyIC function calls -
you can see this by running Mozilla with the --sync command line option).
3) Some more code cleanup.

Comment 14

[Lina Kemmel]

FYI:
Bidirectional input method server, which formally supports the input styles mentioned in comment 6, 
actually doesn't seem to differentiate between them: in bidi locales they all behave the same way.

Comment 15

[Christopher Blizzard (:blizzard)]

Comment on attachment 116811 [details] [diff] [review]
Patch v3

sr=blizzard

Comment 16

[Philip K. Warren]

Comment on attachment 116811 [details] [diff] [review]
Patch v3

Requesting approval for 1.4. Majority of new code is #ifdef'd AIX. This code is
necessary for X Input Method to work properly on AIX. It has been tested
thoroughly on AIX, Linux, and Solaris.

Comment 17

[Asa Dotzler [:asa]]

Comment on attachment 116811 [details] [diff] [review]
Patch v3

a=asa (on behalf of drivers) for checkin to 1.4.

Comment 18

[Philip K. Warren]

Fixed.

Checking in nsGtkIMEHelper.cpp;
/cvsroot/mozilla/widget/src/gtk/nsGtkIMEHelper.cpp,v  <--  nsGtkIMEHelper.cpp
new revision: 1.49; previous revision: 1.48
done
Checking in nsGtkIMEHelper.h;
/cvsroot/mozilla/widget/src/gtk/nsGtkIMEHelper.h,v  <--  nsGtkIMEHelper.h
new revision: 1.15; previous revision: 1.14
done