<Swing Dev> Unexpected NullPointerException by endComposition()

[Pavel Porvatov]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20111103/ddff143f/attachment.html>