On Fri, 24 Feb 2023 21:54:30 GMT, Damon Nguyen <dngu...@openjdk.org> wrote:
> The issue is in Aqua L&F when an editable JComboBox with a border is used. In
> this case, when the comboBox is clicked for the drop-down menu to show, the
> drop-down menu appears at the wrong coordinates (blocking the text of the
> comboBox and making it unreadable).
>
> This seems to have been the case for a while and a similar issue appeared
> recently where an editable Aqua JComboBox also had wrong positioning due to
> having a border.
>
> This fix checks for a border and modifies the bounds to accommodate the
> border's size. Then the usual calculations for the comboBox popup works as
> expected.
>
> The new headful test creates an editable comboBox with a TitledBorder and
> with no border. Then, it automatically clicks the comboBox to open the popup,
> and clicks where the position of the first selectionItem should be. Finally,
> it checks if the selected item is correct. This is for all L&F's and the test
> passes on all OS's.
Changes requested by aivanov (Reviewer).
src/java.desktop/macosx/classes/com/apple/laf/AquaComboBoxPopup.java line 189:
> 187:
> 188: int popupBoundsY = comboBox.getBounds().height;
> 189: if (comboBox.isEditable() && comboBox.getBorder() != null) {
Does it not affect a non-editable combo box with with a border that has large
insets?
test/jdk/javax/swing/JComboBox/EditableComboBoxPopupPos.java line 62:
> 60:
> 61: for (UIManager.LookAndFeelInfo laf :
> UIManager.getInstalledLookAndFeels()) {
> 62: lafName = laf.getName().equals("CDE/Motif") ? "Motif" :
> laf.getName();
Is it really needed? Is `CDE/Motif` such a bad name that it needs adjusting?
test/jdk/javax/swing/JComboBox/EditableComboBoxPopupPos.java line 89:
> 87: // first selection item is in the correct position on
> screen.
> 88: cb1.setSelectedIndex(1);
> 89: cb2.setSelectedIndex(1);
These methods should be called on EDT.
test/jdk/javax/swing/JComboBox/EditableComboBoxPopupPos.java line 97:
> 95: }
> 96: } catch (Exception e) {
> 97: e.printStackTrace();
You should not catch the exception but propagate it for jtreg to fail the test
if it throws the exception. Otherwise, the test is considered passed even
though it may print `Test failed`.
test/jdk/javax/swing/JComboBox/EditableComboBoxPopupPos.java line 117:
> 115: robot.mouseMove(cb.getLocationOnScreen().x + cb.getWidth()
> 116: - BUTTON_OFFSET, cb.getLocationOnScreen().y
> 117: + POPUP_OFFSET);
`getLocationOnScreen()`, `getWidth()` should be called on EDT.
test/jdk/javax/swing/JComboBox/EditableComboBoxPopupPos.java line 131:
> 129: throws InterruptedException, InvocationTargetException {
> 130: if (c1.getSelectedItem().toString().equals("One")
> 131: && c2.getSelectedItem().toString().equals("One")) {
`getSelectedItem` should be called on EDT.
test/jdk/javax/swing/JComboBox/EditableComboBoxPopupPos.java line 135:
> 133: SwingUtilities.invokeAndWait(() -> frame.dispose());
> 134: } else {
> 135: SwingUtilities.invokeAndWait(() -> frame.dispose());
Your `main` method disposes of the frame. You should just move try-finally
block into the for-loop over available Look-and-Feels.
test/jdk/javax/swing/JComboBox/EditableComboBoxPopupPos.java line 139:
> 137: }
> 138: }
> 139: }
Please add a new line character to the end of the file.
-------------
PR: https://git.openjdk.org/jdk/pull/12750
