Hi Sergey,
Thanks for the review. Please see my comments inline. Following the new
webrev
webrev: http://cr.openjdk.java.net/~pbansal/8249251/webrev02/
<http://cr.openjdk.java.net/%7Epbansal/8249251/webrev02/>
On 14/07/20 12:43 PM, Sergey Bylokhov wrote:
Hi, Pankaj.
A few notes about the fix and test:
- The usage of textarea selection as a temporal solution is fine, but
did you notice that mouse over effect over JButtons
in the SwingSet use correct "light color"? Did you tried to check
can we use it for menu as well?
I tried this, but I think this is not very viable option.
1. The highlight is again not very much visible over the menu. It looks
good in SwingSet2 as there are other menus to compare, but if we create
one menu, the highlight is not visible properly. I saw that there is
very less difference in the color values.
2. Doing this will make the Menu more consistent with Popup menu
highlight in Ubuntu 20.04. But for Ubuntu 18.04 and OL 76, OL82, this
will make it less consistent. In OL 76, OL 82, Ubuntu 18.04 the
highlight in native popup is also of same color as selected Text color
background (Blue and Orange). so using button highlight color will make
the menu highlight less consistent with native terminal popup menu
highlight.
3. This is also a workaround in the end and we will need to fix this
also by reading the popup highlight color from Terminal.
4. If you would like to try out this out, here is a webrev
http://cr.openjdk.java.net/~pbansal/8249251/webrev02_Button/
<http://cr.openjdk.java.net/%7Epbansal/8249251/webrev02_Button/>
- The test should not check exact selection color and compare it to
the textarea selection, it should check
that the selection color is clearly visible on the background. I
guess it should use the logic similar to the
logic which was deleted by the fix.
Done.
- The next code should be inverted, or additional waitForIdle should
be added:
frame.setVisible(true);
........
SwingUtilities.invokeAndWait(() -> {
point = menu.getLocationOnScreen();
rect = menu.getBounds();
});
robot.waitForIdle();
robot.delay(500);
You need to wait and then read the bounds, otherwise the test may
fail(It sometimes fail on my local run for that reason)
Done.
Regards,
Pankaj
On 13.07.2020 11:30, Pankaj Bansal wrote:
Hi All,
Please review the following fix for jdk15.
Bug : https://bugs.openjdk.java.net/browse/JDK-8249251
webrev: http://cr.openjdk.java.net/~pbansal/8249251/webrev01/
<http://cr.openjdk.java.net/%7Epbansal/8249251/webrev01/>
Issue: In Ubuntu 20.04 in dark mode, the selected Menu is not being
highlighted properly. so, there is no difference between selected and
unselected Menu. The issue can be reproduced by running Swingset2 or
using the test added in fix.
Cause: In dark mode, the highlight color for Menu is not visible over
the dark background color for the Menubar. So, the highlight is not
visible properly and it looks like there is no highlight being drawn.
Fix: The fix is to use some color for highlighting, which will be
properly visible. We have taken the background color for selected
text. This color is is visible over the dark Background easily. The
fix is tested on Ubuntu 18.04, Ubuntu 20.04 and OL 8.2.
Added an automated test to verify that the highlight color is same as
background color for selected text. The test passes on mach5 with
multiple iterations. Link added in JBS.
Regards
Pankaj
