On Fri, 7 Jul 2023 04:01:56 GMT, Prasanta Sadhukhan <[email protected]> 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) >>  >> >> After fix >>  > > 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
