<Swing Dev> Unexpected NullPointerException by endComposition()

[Pavel Porvatov]

Hi Charles,

I've committed the fix, see here: 
http://hg.openjdk.java.net/jdk8/awt/jdk/rev/84e473cf4531

Thanks for your efforts.

Regards, Pavel
> 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/3fb2d23a/attachment.html>