(original) (raw)

Hi Charles,

The fix looks good, but the test should use EDT in the main method because Swing components are used. Don't worry about that, I'll fix it and commit the fix soon

Thanks, Pavel

On 11/03/2011 10:41 PM, Pavel Porvatov wrote:

Hi Charles,

On 10/27/2011 09:12 PM, Pavel Porvatov wrote:

Hi Charles,

On 10/14/2011 04:06 PM, Pavel Porvatov wrote:

Hi Charles,

On 10/11/2011 05:50 PM, Pavel Porvatov wrote:

Hi Charles,

On 10/08/2011 05:41 PM, Pavel Porvatov wrote:
I got your point. What about this solution:
If in the compose mode, endCompositoin just sendComposedText instead of sendCommittedText.

The patch is attached

Could you please explain the fix? May be it removes NPE but it puzzles me. So if buffer.length() == 0 you invoke sendCommittedText, right? But sendCommittedText commits buffer, but buffer is empty. Looks strange...

BTW: the code like "if (!notInCompositionMode) {" a little bit difficult to understand =) I'd preffer to avoid two negations and use "if (notInCompositionMode)" and swap if/else blocks...

Regards, Pavel

Hi Pavel,

Sorry for the confusion. Here is some explanation, please correct me if I am wrong:
1\. There two modes which is judge from the buffer size: composed mode when the buffer size is not zero and normal mode when the buffer size is zero.

Right

2\. The original code make no difference whether it is in the composed mode or normal mode. In the normal mode, which buffer size is zero, it sends the committed text. In the composed mode, which buffer size is not zero, it also sends the committed code. And NPE occurred here.
3\. In the patch, I do not change the logic when in the normal mode. (notInCompositionMode branch) Why? I guess it is the logic of "Ends any input composition that may currently be going on in this context. Depending on the platform and possibly user preferences, this may commit or delete uncommitted text. " from

the api spec....

Yes. But after your change the following code looks
strange for me:

if (!notInCompositionMode) {

....

} else {

>>>> sendCommittedText();

}

So if we are not in composition mode we send something
(empty string actually). Logically we shouldn't send
anything (IMO), because buffer is empty. Why should we
do something at all if endComposition is invoked and
we are not in composition mode?

4. In the patch, the logic in the
composed mode is that: if it is in the composed
mode, keep every thing as just composed :-)

I found a new bug (???) in the fix. If you apply the
patch, run the MouseEventTest2 test and follow the
instructions from the bug description NPE will not be
thrown, but the JTextArea remains in composition mode
even after endComposition completion.

Right. It seems that we have to do some thing in the jdk
:-). Here it is:



The patch attached is just adding a null check at the
beginning of the mouseClicked method in DefaultCaret. So
why the component is null in the DefaultCaret? That
because the caret has already been deinstalled. It seems
to be an order problem of mouse event and the event
which endCompositon sent. The endComposition will
exchange the caret and deinstall the old one. On the
other hand, mouse click event was happening on the old
caret. So the component of the old caret is null now.
NPE happens.

It looks that you are trying to fix the consequence, but
not the root of the problem. The endComposition method
shouldn't send anything to deinstalled DefaultCaret. I
think the previous version of the fix was much closer than
this one.



Regards, Pavel

Hi Pavel,



The problem is how should we deal with the uncommitted
compose character when endComposition.

1. Remain the character. Not good, will remain in compose
mode after endComposition.

2. Delete the character. I think it just like the
cancelComposition. We have to send some thing to delete the
characters which are already shown on the text area.



Here is a new patch which add a little bit logic in the
endComposition method:

1. It still remain the null check in the mouseClick

2. It use cancelCompostion in the endComposition when in the
compose mode.



Any idea?

I deeply analyzed the problem and found out that I agree with
your last fix *without* changing in the
CodePointInputMethod.java class (you sent such version on
10/19/2011). I answered on that mail "It looks that you are
trying to fix the consequence, but not the root of the
problem. The endComposition method shouldn't send anything to
deinstalled DefaultCaret." Actually we shouldn't send anything
to deinstalled DefaultCaret and I found code that removes
listener of deinstalled DefaultCaret. But at the same time
deinstalled DefaultCaret gets mouseClick notification because
AWT makes copy of all listeners before notifications.
Unfortunately we can't change such functionality and the best
and simplest way to fix the problem is to skip mouseClicked
notification for deinstalled carets.



Could you please write an automatic test, please?



BTW: I didn't catch "problem is how should we deal with the
uncommitted compose character when endComposition". Current
implementation works fine, IMO: it commits entered characters
and ends composition.



Regards, Pavel

Hi Pavel,



>>> "problem is how should we deal with the
uncommitted compose character when endComposition"

I am talking about what user will when the endComposition calls.
To me, it is more reasonable when the uncommitted character go
away if the composition is not complete :-). That's the reason I
am using cancelComposition. Thanks for pointing out the AWT copy
things. Would you like to point me where it is? It is kind of
hard to debug in the awt/swing code :-P



Below is the patch and simple test case (attached):

--
Yours Charles

--
Yours Charles