<Swing Dev> Unexpected NullPointerException by endComposition()

[Charles Lee]

Thank you :-P

On 11/10/2011 06:22 PM, Pavel Porvatov wrote:
> 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
>

-- 
Yours Charles

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