Re: [11]JDK-8160767 [TEST_BUG] java/awt/Frame/MaximizedToIconified/MaximizedToIconified.java
Hi Prem, Did you run it on all platforms? There may be an animation when window is iconified or maximized which takes more than 100ms. --Semyon On 03/01/2018 11:08 PM, Prem Balakrishnan wrote: Hi All, Please Review fix for JDK 11: Bug: https://bugs.openjdk.java.net/browse/JDK-8160767 Webrev: http://cr.openjdk.java.net/~pkbalakr/8160767/webrev.00/ Test fails due to synchronization issue, state changes are validated immediately after calling setExtendedState(). Adding delay after setExtendedState() call and validating the state changes solves this issue. Regards, Prem
Re: [11] Review request for 8198605: Touch keyboard is shown for a non-focusable text component
Hello Sergey, Thank you for review and approval of this fix. Do you mean such an RFE may be used for simplification of testing of this bug or all possible bugs connected with this new functionality, which allows to automatically show and hide the touch keyboard, on a PC without a touch screen? The fix for the bug JDK-8166772 allows to display the touch keyboard on clicks from a regular physical mouse, but only for the case, when no physical keyboards are attached to the PC and the system setting "Automatically show the touch keyboard in windowed apps when there's no keyboard attached to your device" is turned on in MS Windows OS. Therefore technically it is possible to change the code of "Java_sun_awt_windows_WToolkit_showTouchKeyboard" function from the file "src/java.desktop/windows/native/libawt/windows/awt_Toolkit.cpp" to check or not to check for presence of any attached keyboards depending on some new boolean flag, what will let to test this functionality without a touch screen just having a mouse and a physical keyboard. But implementation of such a flag, perhaps as a new Java system property, will require filing CSR request and describing this flag in the release notes. Also testing the functionality related to the touch keyboard without a touch screen is testing of not the main use case. I am ready to create such an RFE and to work on it, if it will be beneficial for testing and development purposes, but if a person, who tests the functionality and the developer working on the issue has a device with a touch screen, then such possible property will simplify testing of the functionality only for the narrow case in which a user has MS Window 8 OS or later, does not have a touch screen, cannot detach the keyboard, since it is part of a laptop, and still wants to use the touch keyboard using the regular mouse. Thank you, Anton On 01/03/2018 21:53, Sergey Bylokhov wrote: Hi, Anton. Looks fine. ps: maybe we can create some rfe to enable Touch keyboard even if the system has some keyboards for the test purpose? On 01/03/2018 08: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
Re: [11] Review request for 8198605: Touch keyboard is shown for a non-focusable text component
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 >
Re: [11] Review request for 8198605: Touch keyboard is shown for a non-focusable text component
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
Re: RFR: JDK-8198844 Clean up GensrcX11Wrappers
Adding 2d-dev in the hopes of getting some input from component team.
Seems like they should be aware of us removing the support for multiple
data models.
Looks like you left a debug message at line 40 in GensrcX11Wrappers.gmk.
/Erik
On 2018-03-02 03:00, Magnus Ihse Bursie wrote:
On 2018-03-02 00:02, Erik Joelsson wrote:
Hello,
In xlibtypes.txt comment, should it be sizes-64.txt?
Yes, good catch.
Generating both 32 and 64 seems a bit outdated at this point. Surely
this is a remnant of bundling 64 and 32 bit together on Solaris in
the past? Perhaps someone in 2d can answer this? Would be nice to be
able to clean up that part as well if possible.
Yes, you are right. We should clean up that as well. I was just too
conservative. I've now actually checked what happens when you generate
both 32 and 64 bit versions, and the result is that instead of code like:
public static int getSize() { return 96; }
we get code like this:
public static int getSize() { return ((XlibWrapper.dataModel ==
32)?(80):(96)); }
Since we do no longer support multiple data models for the same build,
this is just unnecessary. In fact, that leads to an even better
cleanup, since we will always need just a single input file.
I also wrapped the tool calls in ExecuteWithLog.
Updated webrev:
http://cr.openjdk.java.net/~ihse/JDK-8198844-clean-up-GensrcX11Wrappers/webrev.02
/Magnus
Re: RFR: JDK-8198844 Clean up GensrcX11Wrappers
On 2018-03-02 00:02, Erik Joelsson wrote:
Hello,
In xlibtypes.txt comment, should it be sizes-64.txt?
Yes, good catch.
Generating both 32 and 64 seems a bit outdated at this point. Surely
this is a remnant of bundling 64 and 32 bit together on Solaris in the
past? Perhaps someone in 2d can answer this? Would be nice to be able
to clean up that part as well if possible.
Yes, you are right. We should clean up that as well. I was just too
conservative. I've now actually checked what happens when you generate
both 32 and 64 bit versions, and the result is that instead of code like:
public static int getSize() { return 96; }
we get code like this:
public static int getSize() { return ((XlibWrapper.dataModel ==
32)?(80):(96)); }
Since we do no longer support multiple data models for the same build,
this is just unnecessary. In fact, that leads to an even better cleanup,
since we will always need just a single input file.
I also wrapped the tool calls in ExecuteWithLog.
Updated webrev:
http://cr.openjdk.java.net/~ihse/JDK-8198844-clean-up-GensrcX11Wrappers/webrev.02
/Magnus
