<AWT Dev> [8] [PATCH] Review request for 8001633: Wrong alt processing during switching between windows.
Mikhail Cherkasov
mikhail.cherkasov at oracle.com
Tue Oct 30 13:36:01 PDT 2012
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.
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.
>>>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
diff --git a/src/share/classes/com/sun/java/swing/plaf/windows/WindowsRootPaneUI.java b/src/share/classes/com/sun/java/swing/plaf/windows/WindowsRootPaneUI.java
--- a/src/share/classes/com/sun/java/swing/plaf/windows/WindowsRootPaneUI.java
+++ b/src/share/classes/com/sun/java/swing/plaf/windows/WindowsRootPaneUI.java
@@ -31,6 +31,8 @@
import java.awt.KeyEventPostProcessor;
import java.awt.Window;
import java.awt.Toolkit;
+
+import sun.awt.AWTAccessor;
import sun.awt.SunToolkit;
import java.awt.event.ActionEvent;
@@ -133,10 +135,15 @@
// window. If this time appears to be greater than the altRelease
// event time the event is skipped to avoid unexpected menu
// activation. See 7121442.
+ // Also we must ensure that original source of key event belongs
+ // to the same window object as winAncestor. See 8001633.
boolean skip = false;
Toolkit tk = Toolkit.getDefaultToolkit();
if (tk instanceof SunToolkit) {
- skip = ev.getWhen() <= ((SunToolkit)tk).getWindowDeactivationTime(winAncestor);
+ Component originalSource = AWTAccessor.getKeyEventAccessor()
+ .getOriginalSource(ev);
+ skip = SunToolkit.getContainingWindow(originalSource) != winAncestor ||
+ ev.getWhen() <= ((SunToolkit) tk).getWindowDeactivationTime(winAncestor);
}
if (menu != null && !skip) {
diff --git a/src/share/classes/java/awt/event/KeyEvent.java b/src/share/classes/java/awt/event/KeyEvent.java
--- a/src/share/classes/java/awt/event/KeyEvent.java
+++ b/src/share/classes/java/awt/event/KeyEvent.java
@@ -930,6 +930,10 @@
long extendedKeyCode) {
ev.extendedKeyCode = extendedKeyCode;
}
+
+ public Component getOriginalSource( KeyEvent ev ) {
+ return ev.originalSource;
+ }
});
}
@@ -939,6 +943,14 @@
*/
private static native void initIDs();
+ /**
+ * The original event source.
+ *
+ * Event source can be changed during processing, but in some cases
+ * we need to be able to obtain original source.
+ */
+ private Component originalSource;
+
private KeyEvent(Component source, int id, long when, int modifiers,
int keyCode, char keyChar, int keyLocation, boolean isProxyActive) {
this(source, id, when, modifiers, keyCode, keyChar, keyLocation);
@@ -1023,6 +1035,7 @@
} else if ((getModifiers() == 0) && (getModifiersEx() != 0)) {
setOldModifiers();
}
+ originalSource = source;
}
/**
diff --git a/src/share/classes/sun/awt/AWTAccessor.java b/src/share/classes/sun/awt/AWTAccessor.java
--- a/src/share/classes/sun/awt/AWTAccessor.java
+++ b/src/share/classes/sun/awt/AWTAccessor.java
@@ -629,6 +629,11 @@
* Sets extendedKeyCode field for KeyEvent
*/
void setExtendedKeyCode(KeyEvent ev, long extendedKeyCode);
+
+ /**
+ * Gets original source for KeyEvent
+ */
+ Component getOriginalSource(KeyEvent ev);
}
/**
-------------- next part --------------
A non-text attachment was scrubbed...
Name: webrev.7z
Type: application/octet-stream
Size: 41921 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20121031/1acd4010/webrev.7z
More information about the awt-dev
mailing list