Looks fine.
On 18/04/2018 11:47, Anton Litvinov wrote:
Hello Sergey,
Thank you for review of this fix. No, this topic was not earlier
discussed in review process of fixes for other bugs on the touch
keyboard subject. We deliberately do not show the touch keyboard on
"FocusEvent.FOCUS_GAINED" event, because of the following reasons:
1. This is the behavior observable in other native applications like MS
Edge browser, "notepad.exe", "explorer.exe" (File Explorer), Firefox
browser. In this applications the touch keyboard is not shown when the
text component just gets focus, the touch keyboard is shown after an
explicit touch action.
2. Such behavior is implemented by some classes from .NET Framework and
is described on the following web page from Microsoft documentation.
Web page URL:
https://docs.microsoft.com/en-us/windows/uwp/design/input/keyboard-interactions#appendix
This passage from this web page describes such behavior:
"If your app sets focus programmatically to a text input control, the
touch keyboard is not invoked. This eliminates unexpected behaviors not
instigated directly by the user. However, the keyboard does
automatically hide when focus is moved programmatically to a non-text
input control."
3. Showing the touch keyboard on "FocusEvent.FOCUS_GAINED" event may
lead us to the error in the following scenario:
Step 1 - A user touches a text component.
Step 2 - The text component receives focus.
Step 3 - The touch keyboard is shown.
Step 4 - The user manually closes the touch keyboard.
Step 5 - The user touches the text component again.
Step 6 - The touch keyboard is not shown, because the focus did
not leave the text component and no FOCUS_GAINED was received.
Thank you for the suggestion to make "WTookit.compOnxxx" variables as
weak references, what should resolve the possible memory leak. I created
the new version of the fix for this bug, which includes the changes
listed below. Could you please look at the second version of the fix for
this bug.
Webrev (the 2nd version):
http://cr.openjdk.java.net/~alitvinov/8199748/jdk11/webrev.01
The 2nd version of the fix resolves the bug in absolutely different way
in comparison with the 1st version of the fix and has the following
changes:
1) The variables "WToolkit.compOnTouchDownEvent",
"WToolkit.compOnMousePressedEvent" were made instances of
"WeakReference<Component>".
2) The variable "NULL_COMPONENT_WR" was introduced to address issue
with thread safety with "compOnTouchDownEvent",
"compOnMousePressedEvent" variables.
3) The big "if" expression, which verifies if the component is valid
for showing the touch keyboard, was moved into a dedicated method
"boolean isComponentValidForTouchKeyboard(Component)".
4) Checks on equality to "null" were removed from the following "if"
expression, because further "instanceof" expressions for both "comp" and
"e" variables will also return "false", if these variables equal "null".
"1103 if ((comp == null) || (e == null)"
5) The bug itself is resolved by just not hiding the touch keyboard on
"FOCUS_LOST", if it is known that the component which gets focus is a
text component. This change of the fix addresses the issue described by
Alexey Ivanov in a parallel e-mail in the review of this fix, where with
the 1st version of the fix the touch keyboard would be hidden, if the
focus was shifted to the second text component by pressing "TAB" on the
touch keyboard.
Thank you,
Anton
On 17/04/2018 00:48, Sergey Bylokhov wrote:
Hi, Anton.
Maybe this was discussed before, but can you please clarify why we did
not show the keyborad on focus_gain?
BTW looks like the WTookit.compOnxxx should be a weak references.
On 16/04/2018 05:03, Anton Litvinov wrote:
Hello,
Could you please review the following fix for the bug.
Bug: https://bugs.openjdk.java.net/browse/JDK-8199748
Webrev: http://cr.openjdk.java.net/~alitvinov/8199748/jdk11/webrev.00
In the fix for JDK-8166772 it was deliberately implemented that the
touch keyboard is shown only on "MouseEvent.MOUSE_RELEASED" event and
is hidden on "FocusEvent.FOCUS_LOST" event.
The reason of the bug is the fact that, when the touch keyboard is
already shown for one text component and a user touches another text
component, then the following 2 events occur in the presented order:
1. "MouseEvent.MOUSE_RELEASED" event arrives. The touch keyboard is
shown for the new text component.
2. "FocusEvent.FOCUS_LOST" event arrives for the previous text
component. The touch keyboard shown for the new text component
becomes hidden.
The fix allows not to hide the touch keyboard during processing of
"FocusEvent.FOCUS_LOST" event, if the touch keyboard has just been
shown, as a result of processing of "MouseEvent.MOUSE_RELEASED"
event, for the component which gets focus
"FocusEvent.getOppositeComponent()".
Thank you,
Anton
--
Best regards, Sergey.
