Bug 1580

Created attachment 1063 [details] [review]
p_xim_spot.diff

Needs to be discussed outside of Bugzilla first.  
 

reopen for discussion. 

Needs to be discussed with Mike and James Su. 

I asked Sun's Xi18n experts about this patch, and got this answer:
   The patch is good. Solaris already has this in the xi18n layer.

More details by James 
 
--> http://bugzilla.suse.de/show_bug.cgi?id=38203 
 
This enables xim input methods can get the input spot location in gtk2 apps. 
 
 
Gtk2 only uses OffTheSpot mode if the XIM server does not support OnTheSpot 
mode. In this situation, the XIM server can not receive the spot location 
event, so it can not locate the candidate window to its correct position. So 
many XIM servers will stick the candidate window to the lower left corner of 
the client window. If the client window is very large, then it will be very 
inconvenience. 
 
In fact, it's also true for OnTheSpot mode. But most of XIM servers will be 
mad, if I turn on the SpotLocation event for OnTheSpot mode. I don't know 
what's the matter with it. Theoretically it should also be ok to turn on this 
event for OnTheSpot mode, but there must be some bugs in Xlib to prevent us 
from doing this. 
 
And further more, many applications do not support OnTheSpot mode very well, 
for example OpenOffice, Evolution etc. It's better to turn off the OnTheSpot 
support by default for these apps. 

I will take it and commit it.
Thanks Alan for verifying!
BTW: who is the original author?

I'll take it.

Author is James Su. 

Prepared for commit. Will credit James as author.

Done.
     * lib/X11/imRm.c:
        Turn on forwarding XNSpotLocation event to XIM server in
        OffTheSpot and Root mode (bugzilla #1580, James Su).
 

Reading this and the patch again, I wonder why not using XNSpotLocation for
OnTheSpot as well. Reopeing the bug for this.

I assume XNSpotLocation is not open for OnTheSpot at current
for the two reasons, but I'm rather against both of the reasons.

  - If some XIM servers are known to be mad by getting XNSpotLocation in 
    OnTheSpot mode, it sounds like they are bugs of these XIM servers and 
    should be fixed in their end.
  - OnTheSpot is used in many modern apps, such as Gtk+2.0 apps, 
    OpenOffice.org, Java, hence I consider XNSpotLocation support is
    more important than any other styles.
   
 

And change Summary according to my request for OnTheSpot mode.

Hi,
  Most client won't process the SpotLocation event when using OnTheSpot mode,
and some clients will got crasy if they receive such event.
  And there might be some bugs in Xlib or X Server which will cause the events
are sent to client with wrong order.

I don't think any XIM clients are supposed to receive XNSpotLocation as 
events from XIM server. It's client's call to set or get XNSpotLocation, by calling
XSetICValues() and XGetICValues().
  What exactly do you see those clients misbehave by saying "they will get
crazy" with XNSpotLocation in OnTheSpot mode? Do they crash or hang? Wwill you name
what these clients are? I'd want to give a try.

Hi,
  Sorry, my mistake. I mean, many clients are not capable to send SpotLocation
event in OnTheSpot mode. At least I remembered that mozilla does not send it.
  And the clients will have strange behaviour when enable sending SpotLocation
event to XIM server because of wrong events order returned from XIM Server (it's
the bug of Xlib or X Server indeed. The order will get wrong even if XIM Server
send the events with correct order.)

Regards
James Su

Thanks, we're getting closer. As for mozilla, the late version uses Gtk+-2.0's
GtkIMContext, and XIM backend(im-xim.so) sets XNSpotLocation in the OnTheSpot mode.
I confirmed this with the Mozilla version 1.7. With the modified ximcp.so.2
which enables XNSpotLocation in OnTheSpot, XIM lookup choice window displays
near the spot location, while it doesn't with the original xim.so.2.

About events disordering, do you means XIM protocols from the XIM server are
received by XIM clients in wrong order, and this happens by enabling
XNSpotLocation in the OnTheSpot? If yes, can you help me reproduce this? I'd
want to know the version of libX11, and XIMserver and type of linux distros, and
any relavent information to reproduce. And, most important thing, what type of
events are actually disordered, and what failure actually occurs by the disorder.

Yes, that's what I mean. I encountered such issue on XFree86 4.3.0, kernel
2.6.0, SCIM and kinput2. But I forgot what kind of events were actually
disordered. I just remembered that kinput2 stopped to work after XNSpotLocation
was enabled for OnTheSpot mode.

I don't know if it has been fixed in recent libX11 version.

You may try as many XIM servers and clients as possible to see if they all works
correctly.

James. Finally, I could find an issue when XNSpotLocation is enabled
for XIMPreditCallbacks. I believe this is what you're seeing as well.

The issue is delay of preedit draw callback(s), isn't it? Sometimes preedit
draw callback are not called timlely at the present event, but are called
at next events. E.g, suppose 'abc' should make "ABC" in preedit, but
actually it only makes "AB". But, a next operation, say, next key event or so,
will make it "ABC". Kind of?

I've made up a fix so that preedit draw callbacks, which are pended in
a pending callback queue *before* XSetICValues(XNSpotLocation) is called, 
should be processed timely. I'm attaching the patch. Can you please read and
review, and see if this could resolve this issue? (If your issue is different
from this, please kindly teach me again...)

Created attachment 1833 [details] [review]
process pended callback events before processing seticvalues

Hi,
  It should be the issue that I encountered. I'll try your patch as soon as
possible. But I'll on my vacation during Feb 6-20. I'll try to give you result
before Feb 25.
  Thank you very much.

Regards
James Su

Hi,
  Sorry for late. I tested the patch in comment #20. The events disorder issue
was disappeared after applying this patch.
  Thank you very much.

Hidetoshi, I don't fully understand your patch in attachment #1833 [details] [review].
I understand the need to call _XimProcessPendingCallbacks() in
XimProtoSetICValues(). However I don't quite understand the 
if (ic->private.proto.process_pend_cb_que)
      return;
shield. It seems serve as a semaphore to protect the function from getting
called twice. I would assume that this can only happen in a multi threaded
environment.
Disregarding the fact that XIM is not fully thread save this semaphore is not
atomic and therefore racey. Although this may never become a real issue and may
well be on the same level as all the other issues of XIM.
I don't fully understand why we need to do 
ic->private.proto.process_pend_cb_que = False;
in _XimPutCbIntoQueue(). The only place where this semaphore is set to TRUE is
in _XimProcessPendingCallbacks(). It is set to TRUE when the function is entered
and reset to FALSE when exited. Since the patch adds this in the case of 
(pcbq == (XimPendingCallback)NULL) this can only make a difference between
ic->private.proto.pend_cb_que = pcbq->next; and the next iteration of while()
in _XimProcessPendingCallbacks(). 
It should not be required for initialization as the entire structure gets
bezeroed() at _XimProtoCreateIC() or _XimLocalCreateIC(). Maybe I'm missing
something here. Did you actually encounter a situation where this is needed?
Furthermore I'm concerned that adding another field to XicProtoPrivateRec will
change XIcRec which seems to be part of a public API. The change will destroy
the ABI compatibility of this interface. 
I don't think this fact should be overlooked.

Egbert, I'd need to debug this again - two months since then, I forgot
the reason why I needed the process_pend_cb_que. I'll get back to you soon.
A quick question for now, I don't see what ABI compatibility will be break by
adding a memeber in XicProtoPrivateRec, which is private to a perticular XIM
implementation (XIMCP). Can you explain? 

You may be right. The public part exposed thru a public ABI seems to be the XICRec.
The XicRec contains an additional private part which changes in size due to your
change - however this should not matter for functions that only access the
public part. I got confused from XICReg and XicRec.
Maybe we are save here after all.

Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.

Egbert?

I don't see any of the proposed patches/hunks applied. Also SuSE/Novell never did. Let's close this one as WONTFIX.

This problem still presents in libX11 1.6.3
When using rxvt-unicode and fcitx with the OnTheSpot input style (enable OnTheSpot support in fcitx-xim, run urxvt with "urxvt -pt OnTheSpot"), the candidate list is displayed at the lower left corner of the window, instead of the current cursor position.

On the other hand, if I enable passing XNSpotLocation to the IM server (change line 1950 from
    0, /*(XIM_MODE_PRE_SET | XIM_MODE_PRE_GET),*/
to
    (XIM_MODE_PRE_SET | XIM_MODE_PRE_GET),
), then the position of the candidate list is correct.
I've tried it with recent fcitx and ibus and it works well.

Please reopen the bug and fix the problem.