<Swing Dev> Unexpected NullPointerException by endComposition()

[Charles Lee]

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20111104/bdc35665/attachment.html>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patch.contr.v5
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20111104/bdc35665/patch.contr.v5>