On Fri, 11 Jul 2025 21:10:14 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java
>> line 932:
>>
>>> 930: skin.paintSkin(g, x + OFFSET, y +
>>> OFFSET, state);
>>> 931: } else {
>>> 932: if (icon.getIconHeight() > 16) {
>>
>> Why does height > 16 matter ? How does it make icons align ?
>>
>> Also - if that is satisfactorily answered then wouldn't the conditions here
>> be more succinctly coded as
>>
>> if (icon == null || icon.getIconHeight() > 16)
>> {
>> skin.paintSkin(g, x + OFFSET, y +
>> icon.getIconHeight() / 2, state);
>> } else {
>> skin.paintSkin(g, x + OFFSET, y +
>> OFFSET, state);
>> }
>
>> Why does height > 16 matter ? How does it make icons align ?
>
> Perhaps, it's because standard Windows menu icons are 16×16; if the icon is
> larger something may need adjusting.
>
> Yet I don't know the answer…
I am checking for 16 as Windows menu icon is of standard 16x16 size normally
(and anything bigger is considered large referring to JDK-8182043) so didn't
want to change the icon rendering height for normal,standard icons.
> Shouldn't Windows 10 do it the new way too ?
I didn't intentionally change the design for Windows 10 as that was the
behaviour which was in existence (even though by "accident" as we learnt) for
so long our JDK code for Windows L&F is in existence, and some user might see
the change (if we make Windows 11 and 10 same) and find it abrupt and not in
sync to whatever they were seeing and "used to" till now
Also, there was no issue raised for Windows 10 about this behaviour so I
believe it was same with native app.
Also, Windows 10 I believe is in its last leg and probable EOL in few months so
thought of not changing the behaviour at this last moment.
Finally, any change to Windows 10 design will require extensive testing and I
didn't have any Windows10 system so would have to rely on others and it would
have been hard to replicate and fix if reported by others.
>> test/jdk/javax/swing/JMenuItem/TestRadioAndCheckMenuItemWithIcon.java line
>> 31:
>>
>>> 29: * JRadioButtonMenuItem and JCheckboxMenuItem
>>> 30: * is rendered with ImageIcon in WindowsLookAndFeel
>>> 31: * @requires (os.family == "windows")
>>
>> Can't we run this with all L&Fs on all platforms ?
>> What about it would be so different in some other case ?
>>
>> Also that last update for different sized icons isn't being tested - so can
>> we vary the icon size in this test ?
>
> Should we run this test for all L&Fs?
>
> So far, Windows L&F was unique in the way that it combined both check marks /
> bullets and icons. If we implement it the new way so that all the check marks
> / bullets and menu icons align, the menu behaviour in Windows L&F will be the
> same as that in Metal and Nimbus at least.
>
> `javax/swing/JMenuItem/RightLeftOrientation.java` tests a similar scenario.
> If the behaviour of Windows L&F is the same as in other L&F, we may just add
> this bug id to an existing test.
We can but since the fix is in Windows I thought to make it Windows
centric..Haven't quite tested in GTK or Aqua to see how it looks there..
I thought if we finalize the PR and settle on the design and it works fine,
then we can look to extend the test later on to other platforms..
As of now, there is also a known issue in PassFailJFrame if multiple L&F are to
be tested in iteration, it doesn't recreate its UI after testing for a L&F is
complete so causing some exception.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r2203865329
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r2203864736
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r2203865428
