<AWT Dev> [8] [PATCH] Review request for 8001633: Wrong alt processing during switching between windows.

[Anton V. Tarasov]

On 31.10.2012 0:36, Mikhail Cherkasov wrote:
> Anton,
> Applet viewer processes all alt_events correct with/without this fix because applet viewer
> is from AWT world. So NPE problem appears due mixing  awt windows and swing windows.
> But this is a special case, and I hope no one will use both AWT and Swing windows in one application.

I'm afraid this architecture may still be valid for our Swing/AWT reg tests, at least. And so, we're 
better to handle it.
In some way like this:

if (tk instanceof SunToolkit) {
     Window ancestor = (winAncestor == null ?
         SunToolkit.getContainingWindow(ev.getComponent())  : winAncestor);

     // do the rest with ancestor
}

What do you think?

Thanks,
Anton.

>
> So I attached new version, with reversed the check order.
>
> On 10/30/2012 4:57 PM, Anton V. Tarasov wrote:
>> On 30.10.2012 16:45, Mikhail Cherkasov wrote:
>>> Anton, when we switch to Applet Viewer and press the 'alt' key
>>>  AltProcessor.postProcessKeyEvent executes the following lines(173-175):
>>>
>>> root = SwingUtilities.getRootPane(ev.getComponent());
>>> winAncestor = (root == null ? null :
>>>                         SwingUtilities.getWindowAncestor(root));
>>>
>>> SwingUtilities.getRootPane(ev.getComponent()); returns null for Applet Viewer.
>>> So when AltProcessor tries to process 'alt' release event the following code throws NPE:
>>>
>>> ev.getWhen() <= ((SunToolkit) tk).getWindowDeactivationTime(winAncestor); (~145 line)
>>>
>>> If new check goes first, the check with NPE will not be executed, but it's no explicit way to 
>>> fix this.
>>
>> Well, I think I've missed the latest findings related to the NPE (I thought it was caused by null 
>> AppContext).
>> In this case I'm ok with the simple rearrangement. However, another concern arises here. 
>> According to the suggested
>> code all alt_released events will be skipped inside an AppletViewer. Am I right?
>>
>> Thanks,
>> Anton.
>>
>>>
>>> May be better to add explicit check at the beginning of AltProcessor.altPressed method:
>>> if (winAncestor == null ) {
>>>   return; // nothing to do here because we have no window.
>>> } ?
>>>
>>> On 10/30/2012 3:35 PM, Anton V. Tarasov wrote:
>>>> Mikhail,
>>>>
>>>> If SunToolkit.getContainingWindow just forces AppContext initialization (?), than I'd prefer to 
>>>> fix it in some direct way,
>>>> or if that is impossible or hard to do, make appropriate comments.
>>>>
>>>> Thanks,
>>>> Anton.
>>>>
>>>> On 30.10.2012 15:28, Mikhail Cherkasov wrote:
>>>>> Leonid, you're right, reversing fix NPE.
>>>>> Please find new patch and webrev in attachments.
>>>>>
>>>>> On 10/29/2012 11:40 PM, Leonid Romanov wrote:
>>>>>> Hi,
>>>>>> I've found that reversing the check order in WindowsRootPaneUI.java like
>>>>>> this
>>>>>>
>>>>>> skip = SunToolkit.getContainingWindow(originalSource) != winAncestor
>>>>>>     || ev.getWhen() <= ((SunToolkit)
>>>>>> tk).getWindowDeactivationTime(winAncestor);
>>>>>>
>>>>>> also fixes the alt-tab exception regression. Could you check please that
>>>>>> this change indeed fixes the exception bug and if it does, incorporate it
>>>>>> into your patch.
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: awt-dev-bounces at openjdk.java.net [mailto:awt-dev-
>>>>>>> bounces at openjdk.java.net] On Behalf Of Mikhail Cherkasov
>>>>>>> Sent: Monday, October 29, 2012 8:18 PM
>>>>>>> To: Anton V. Tarasov; awt-dev at openjdk.java.net
>>>>>>> Subject: Re: <AWT Dev> [8] [PATCH] Review request for 8001633: Wrong alt
>>>>>>> processing during switching between windows.
>>>>>>>
>>>>>>> Please review the second version:
>>>>>>> http://cr.openjdk.java.net/~alexp/8001633/webrev.01/
>>>>>>> <http://cr.openjdk.java.net/%7Ealexp/8001633/webrev.01/>
>>>>>>> All remarks was corrected.
>>>>>>>
>>>>>>> On 10/29/2012 4:32 PM, Anton V. Tarasov wrote:
>>>>>>>> Hi Mikhail,
>>>>>>>>
>>>>>>>> * KeyEvent.java
>>>>>>>>
>>>>>>>> - No need to initialize 'originalSource' in constructors which call to
>>>>>>>> this(...) where you already initialized it.
>>>>>>>>
>>>>>>>> -950      * we need to able to obtain original source.
>>>>>>>>
>>>>>>>> "be" is omitted ("we need to be able")
>>>>>>>>
>>>>>>>> * WindowsRootPaneUI.java
>>>>>>>>
>>>>>>>> I think there's no need to put another 'skip' setting into a separate
>>>>>>>> if-block (skip will be equal 'false' in majority of cases).
>>>>>>>> Why don't you write it simply as follows?
>>>>>>>>
>>>>>>>> Component originalSource =
>>>>>>>> AWTAccessor.getKeyEventAccessor().getOriginalSource(ev);
>>>>>>>>
>>>>>>>> skip = (ev.getWhen()<=
>>>>>>>> ((SunToolkit)tk).getWindowDeactivationTime(winAncestor)) ||
>>>>>>>> SwingUtilities.getWindowAncestor(originalSource) != winAncestor);
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Anton.
>>>>>>>>
>>>>>>>> On 29.10.2012 14:23, Mikhail Cherkasov wrote:
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> Could you please review a fix for 8001633
>>>>>>>>> <http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8001633>:
>>>>>>> Wrong
>>>>>>>>> alt processing during switching between windows.
>>>>>>>>> Bug:http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8001633
>>>>>>>>> <http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8001633>
>>>>>>>>> Webrev:
>>>>>>>>> http://cr.openjdk.java.net/~bagiras/8/7082294.1/
>>>>>>>>>
>>>>>>>>> To prevent wrong focus traversing to menu was added additional check
>>>>>>>>> to AltProcessor.
>>>>>>>>> It checks that original source of 'alt' event belongs to
>>>>>>>>> AltProcessor.winAncestor
>>>>>>>>> or its component.
>>>>>>>>>
>>>>>>>>> Patch is attached.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Mikhail.
>>>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>