RFR: 8140527: JInternalFrame has incorrect title button width [v4]

[Alexey Ivanov]

On Fri, 23 Jun 2023 08:41:25 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> Title buttons under Widows Classic L&F got their sizes from the XP desktop theme in which button width can be bigger than height. It is construed as XP bug where sizes aren't updated properly so it uses height units for width for XP and later windows. The proposed fix uses the [same technique](https://github.com/openjdk/jdk/blob/a0595761ef35c4eec8cb84326a869b9473cd5bba/src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsInternalFrameTitlePane.java#L78-L82) for Classic and forces title buttons to be square and to fit the frame title in height.
>> 
>> Before fix SwingSet2 demo (Windows Classic InternalFrame)
>> ![image](https://github.com/openjdk/jdk/assets/43534309/3d6ec539-5e17-46ce-aba5-e724af6085fe)
>> 
>> After fix
>> ![image](https://github.com/openjdk/jdk/assets/43534309/db0135e4-d7f3-41a4-bbfa-e8e95cbd071d)
>
> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix for JDK-8139392

Looks good to me.

The only concern is that UI Manager has a wrong width until `JInternalFrame` is shown or initialised. But it has been the case before the fix too.

> Having said that, I guess XP theme also padding is not correct as can be seen in this bug [JDK-8139392](https://bugs.openjdk.org/browse/JDK-8139392) which I have rectified
> 
> After fix ![image](https://user-images.githubusercontent.com/43534309/248217142-224f098c-5ff5-4713-8434-05d99e7d9a71.png)

Your latest changes also fix [JDK-8139392](https://bugs.openjdk.org/browse/JDK-8139392), do I get it right? If so, add it to the list of fixed issues.

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java line 292:

> 290:                 width = UIManager.getInt("InternalFrame.titleButtonHeight") + 2;
> 291:             } else {
> 292:                 width = UIManager.getInt("InternalFrame.titleButtonHeight") -2;

Perhaps, add a space between `-2` (to be consistent with `+ 2`). You've updated the entire method `getIconWidth`, so it will look better.

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsInternalFrameTitlePane.java line 80:

> 78:             // Fix for XP bug where sometimes these sizes aren't updated properly
> 79:             // Assume for now that height is correct and derive width from height
> 80:             buttonWidth = buttonHeight+6;

Perhaps?
Suggestion:

            buttonWidth = buttonHeight + 6;

For `height` above, it's `- 4`; no other instances of `-` or `+` inside the method, so spaces around `+` will be consistent.

test/jdk/javax/swing/JInternalFrame/InternalFrameTitleButtonTest.java line 55:

> 53: 
> 54:     public static void main(String[] args) throws Exception {
> 55:         if (OSInfo.getOSType() == OSInfo.OSType.WINDOWS) {

If you remove this line (OS check), the test would be easier to run as a stand-alone app. It's up to you though.

test/jdk/javax/swing/JInternalFrame/InternalFrameTitleButtonTest.java line 104:

> 102:                     if (icon.getIconHeight() > height - 4 ||
> 103:                         icon.getIconWidth() > height - 2) {
> 104:                         throw new RuntimeException("Wrong title icon size");

Suggestion:

                    if (icon.getIconHeight() > height - 4
                        || icon.getIconWidth() > height - 2) {
                        throw new RuntimeException("Wrong title icon size");

Wrapping the operator prevents ambiguity: continuation line or statement.

-------------

Marked as reviewed by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14555#pullrequestreview-1495668370
PR Comment: https://git.openjdk.org/jdk/pull/14555#issuecomment-1604699016
PR Review Comment: https://git.openjdk.org/jdk/pull/14555#discussion_r1240160993
PR Review Comment: https://git.openjdk.org/jdk/pull/14555#discussion_r1240162708
PR Review Comment: https://git.openjdk.org/jdk/pull/14555#discussion_r1240155464
PR Review Comment: https://git.openjdk.org/jdk/pull/14555#discussion_r1240156462