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

[Alexey Ivanov]

On Fri, 7 Jul 2023 04:01:56 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:
> 
>   Adjust width for Windows Vista theme to 32

**`WindowsIconFactory.java`**

Could you also remove `Dimension` import? It's unused.

Could you please add `final` modifier to the `part` field in `FrameButtonIcon` (line 178)?

**`WindowsInternalFrameTitlePane.java`**

Could you please organise imports? This will expand all the wildcard imports.

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

> 290:                 // ratio from the uxtheme part
> 291:                 width = UIManager.getInt("InternalFrame.titleButtonHeight") -2;
> 292:                 Dimension d = XPStyle.getPartSize(Part.WP_CLOSEBUTTON, State.NORMAL);

The part should've been `Part.WP_MDICLOSEBUTTON`.

Yet the returned size is wrong either way. On my Windows 11 system, the dimensions are 14×9 and 3×5 for `WP_CLOSEBUTTON` and `WP_MDICLOSEBUTTON`. Either is far from the real values.

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

> 81:         } else {
> 82:             buttonWidth = buttonHeight;
> 83:             buttonWidth += 2;

Suggestion:

            buttonWidth = buttonHeight + 2;

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

> 86:             setBorder(BorderFactory.createLineBorder(activeBorderColor, 1));
> 87:         }
> 88:         UIManager.put("InternalFrame.titleButtonWidth", buttonWidth);

> 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.

I was probably wrong in making you save the width into the `UIManager`. First of all, it creates an inconsistency between the width and the height. We don't use the value of height (`InternalFrame.titleButtonHeight`) either, we use the adjusted value only; neither do we store it back — it will break our calculations.

For the sake of backward compatibility, *we shouldn't update `InternalFrame.titleButtonWidth`.*

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

> 56:         }
> 57:         UIManager.setLookAndFeel(
> 58:                "com.sun.java.swing.plaf.windows.WindowsClassicLookAndFeel");

Shall `WindowsLookAndFeel` be tested as well?

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

> 101:                     Icon icon = ((JButton) c).getIcon();
> 102:                     if (icon.getIconHeight() > height - 4
> 103:                         || icon.getIconWidth() > height - 2) {

Should we make the test stricter? According to the code, `width = height + 2` for the classic theme and `width = height + 14` for visual styles. In both cases, `height = UIManager.getInt("InternalFrame.titleButtonHeight") - 4`.

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

Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14555#pullrequestreview-1519648245
PR Review Comment: https://git.openjdk.org/jdk/pull/14555#discussion_r1256352193
PR Review Comment: https://git.openjdk.org/jdk/pull/14555#discussion_r1256355797
PR Review Comment: https://git.openjdk.org/jdk/pull/14555#discussion_r1256375584
PR Review Comment: https://git.openjdk.org/jdk/pull/14555#discussion_r1256376314
PR Review Comment: https://git.openjdk.org/jdk/pull/14555#discussion_r1256381116