<Swing Dev> Unexpected NullPointerException by endComposition()

[Charles Lee]

On 11/10/2011 02:37 AM, Pavel Porvatov wrote:
> 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
That's great. Thanks a lot, Pavel.

-- 
Yours Charles

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20111110/2d6c8210/attachment.html>