RFR: 6672644: JComboBox still scrolling if switch to another window and return back [v4]

[Prasanta Sadhukhan]

On Thu, 7 Nov 2024 09:05:02 GMT, Damon Nguyen <dnguyen at openjdk.org> wrote:

>> In a JComboBox, if the user opens the dropdown list and clicks and holds the down-button, then ALT-TABs to switch focus, when the user re-focuses the frame with the JComboBox and opens the dropdown list again, the list will still be scrolling even though the down-button isn't pressed.
>> 
>> This isn't OS or L&F specific, although Aqua L&F does not have any directional arrows in the dropdown list (and is thus exempt). This led me to believe it could be handled in BasicComboBoxUI where focusLost and focusGain are used or isPopupVisible but the scroll behavior cannot be altered here. Likewise for BasicComboPopup where `autoscroll` is used. However, this behavior isn't related to autoscroll and is actually found in the JScrollbar of the JScrollpane inside of the JComboBox. The timer for the scroll action starts but is never stopped if focus is lost, so a new listener is created and used. The proposed solution uses `KeyboardFocusManager` to track the focus owner. The listener stops the `scrollTimer` when the `focusOwner` property is changed. With this change, the list no longer automatically scrolls when re-focused and instead opens normally.
>> 
>> The included test is manual due to the need to confirm that the list still scrolls after ALT-TABing. The L&F is set to Metal since it is the cross-platform lookandfeel and has directional buttons for the JScrollPane list.
>
> Damon Nguyen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add space

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicScrollBarUI.java line 1244:

> 1242:                     } else if (decrButton.isFocusPainted()) {
> 1243:                         decrButton.doClick();
> 1244:                     }

Why this doClick block is required?

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicScrollBarUI.java line 1592:

> 1590: 
> 1591:         public void mousePressed(MouseEvent e)          {
> 1592:             System.out.println("MOUSEPRESSED: " + e);

Remove this println

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicScrollBarUI.java line 1685:

> 1683:                                 >= trackListener.currentMouseY)
> 1684:                                     ((Timer)e.getSource()).stop();
> 1685:                     } else if (getThumbBounds().y <= trackListener.currentMouseY)        {

GUess you are not changing this block so no need for formatting cleanup in this PR.. infact the "{" are also way off if you had to change...there are other non-touched area like l1593...better to cleanup in separate cleanup..

test/jdk/javax/swing/JComboBox/JComboBoxScrollFocusTest.java line 43:

> 41:              popup list. While holding left click, the list should be scrolling
> 42:              down. Press ALT + TAB while holding down left click to switch
> 43:              focus to a different window. Focus the test frame again and click

left click => left click button
focus to a different window => and release the left click mouse button

test/jdk/javax/swing/JComboBox/JComboBoxScrollFocusTest.java line 72:

> 70:         frame.setSize(400, 200);
> 71:         frame.setLocationRelativeTo(null);
> 72:         frame.setVisible(true);

no need of setVisible, will be taken care by PFJ

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20845#discussion_r1836638995
PR Review Comment: https://git.openjdk.org/jdk/pull/20845#discussion_r1836639441
PR Review Comment: https://git.openjdk.org/jdk/pull/20845#discussion_r1836643345
PR Review Comment: https://git.openjdk.org/jdk/pull/20845#discussion_r1836644539
PR Review Comment: https://git.openjdk.org/jdk/pull/20845#discussion_r1836644960