Hi Anton,

Thank you for the clarification. The fix looks good to me.

Thanks,
Dmitry 

> On 2 Mar 2018, at 15:27, anton.litvi...@oracle.com wrote:
> 
> Hello Dmitry,
> 
> Thank you for review and partial approval of this fix. I think that there is 
> no need to add checks for "Component.isDisplayable()", 
> "Component.isVisible()" in this method, because:
> 
> 1.  Practical test shows, that for enabled, editable, focusable but not 
> visible text components the touch keyboard is not shown in JDK with the fix 
> for the bug JDK-8166772.
> 2.  Showing of the touch keyboard implemented by the fix for JDK-8166772 
> occurs as a result of handling "java.awt.event.MouseEvent" events on Java 
> code level and not of handling "java.awt.event.FocusEvent". The events of 
> type "MouseEvent" are not dispatched to not visible "java.awt.Component" 
> instances according to the following code from the method 
> "java.awt.Container.getMouseEventTargetImpl(int, int, boolean, 
> EventTargetFilter, boolean, boolean"
> 
>     Component comp = component.get(i);
>     if (comp != null && comp.visible &&
> 
> 3.  I do not see, how the end user may touch or click enabled, editable, 
> focusable, but not displayable text component. I do not think that there is 
> need to cover such a case, unless a test case demonstrating showing of the 
> touch keyboard for not displayable text component exists.
> 
> Will you approve this fix?
> 
> Thank you,
> Anton
> 
> On 01/03/2018 22:04, Dmitry Markov wrote:
>> Hi Anton,
>> 
>> The fix looks good, but I wonder if we also shall check results of 
>> isDisplayable() and isVisible() before showing touch keyboard. My assumption 
>> is the following: we cannot transfer focus to non-displayable or invisible 
>> components, so it is unnecessary to display touch keyboard for such 
>> components too.
>> 
>> Thanks,
>> Dmitry
>> 
>>> On 1 Mar 2018, at 16:30, anton.litvi...@oracle.com wrote:
>>> 
>>> Hello,
>>> 
>>> Could you please review the following fix for the bug.
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8198605
>>> Webrev: http://cr.openjdk.java.net/~alitvinov/8198605/jdk11/webrev.00
>>> 
>>> The fix extends a number of the required conditions in the method 
>>> "sun.awt.windows.WToolkit.showOrHideTouchKeyboard(Component, AWTEvent)", 
>>> which should be satisfied for showing of the touch keyboard, by checking if 
>>> the component is focusable or not.
>>> 
>>> Thank you,
>>> Anton
> 

Reply via email to