RFR: 6318027: BasicScrollBarUI does not disable timer when enclosing frame is disabled.

[Abhishek Kumar]

On Fri, 26 Jul 2024 06:03:28 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

> Issue is
> BasicScrollBarUI.ArrowButtonListener starts a timer in mousePressed(), and stops it in mouseReleased(). If the frame containing the scrollbar is disabled between the MOUSE_PRESSED and the MOUSE_RELEASED events, the mouseReleased() method is never called. If the frame is then re-enabled, the still-running timer causes it to scroll all the way to the end.
> Fix is to check if [ArrowButtonListener.handledEvent](https://github.com/openjdk/jdk/blame/ee839b7f0ebe471d3877cddd2c87019ccb8ee5ae/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicScrollBarUI.java#L1567) is still set when ActionEvent is processed then stop the timer and reset this variable.
> 
> CI testing is green and also SwingSet2 JScrollPane scrolling with this modification..

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

> 1612:             // mouseReleased is not called after mousePressed when
> 1613:             // this AcionEvent is being processed
> 1614:             if (buttonListener.handledEvent) {

Since the fix is added in BasicScrollBarUI, will it affect Aqua ScrollBar also?

test/jdk/javax/swing/JScrollBar/DisableFrameFromScrollBar.java line 81:

> 79:                     bar.getModel().addChangeListener(new DisableChangeListener(frame));
> 80:                     frame.getContentPane().setLayout(new FlowLayout());
> 81:                     frame.getContentPane().add(bar);

I guess `getContentPane` may be removed.

test/jdk/javax/swing/JScrollBar/DisableFrameFromScrollBar.java line 85:

> 83:                     frame.pack();
> 84:                     frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
> 85:                     frame.setSize(150, 150);

setting frame size explicitly after calling `frame.pack()` looks redundant.

test/jdk/javax/swing/JScrollBar/DisableFrameFromScrollBar.java line 138:

> 136:                 Point p = bar.getLocationOnScreen();
> 137:                 Rectangle rect = bar.getBounds();
> 138:                 result[0] = new Point((int) (p.x + rect.width/2),

Suggestion:

                result[0] = new Point((int) (p.x + rect.width / 2),

test/jdk/javax/swing/JScrollBar/DisableFrameFromScrollBar.java line 149:

> 147:     public static class DisableChangeListener implements ChangeListener {
> 148:         private final JFrame m_frame;
> 149:         private boolean m_done = false;

default value for a boolean is `false`, no need to set it explicitly.

test/jdk/javax/swing/JScrollBar/DisableFrameFromScrollBar.java line 156:

> 154: 
> 155:         public void stateChanged(ChangeEvent p_e) {
> 156:             if (! m_done) {

Suggestion:

            if (!m_done) {

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20346#discussion_r1699418618
PR Review Comment: https://git.openjdk.org/jdk/pull/20346#discussion_r1699424368
PR Review Comment: https://git.openjdk.org/jdk/pull/20346#discussion_r1699425151
PR Review Comment: https://git.openjdk.org/jdk/pull/20346#discussion_r1699426454
PR Review Comment: https://git.openjdk.org/jdk/pull/20346#discussion_r1699427400
PR Review Comment: https://git.openjdk.org/jdk/pull/20346#discussion_r1699427872