RFR: 8306119: Many components respond to a mouse event by requesting focus without supplying the MOUSE_EVENT cause [v3]

Jayathirth D V jdv at openjdk.org
Mon May 29 04:47:06 UTC 2023


On Wed, 24 May 2023 10:41:09 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> Many Swing components request a focus transfer in response to a mouse event, but fail to supply a Cause when requesting focus. A focus event listener will find the cause to be UNKNOWN, but MOUSE_EVENT would be more appropriate. 
>> Fixed by adding MOUSE_EVENT cause to requestFocus() when it is called for mouse events...
>> 
>> All jtreg/jck tests are ok..
>
> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   test fix

I see concerns related to SwingUtilities2 and Accessibility in https://bugs.openjdk.org/browse/JDK-8306119 description.
If we are not handling them in this fix, do we have separate JBS bug for them?

src/java.desktop/macosx/classes/com/apple/laf/AquaSpinnerUI.java line 606:

> 604: 
> 605:             if (child != null && SwingUtilities.isDescendingFrom(child, spinner)) {
> 606:                 child.requestFocus(FocusEvent.Cause.MOUSE_EVENT);

I see that this function is called only from mousePressed. This seems fine.

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicButtonListener.java line 325:

> 323:                 model.setPressed(true);
> 324:                 if(!b.hasFocus()) {
> 325:                     b.requestFocus(FocusEvent.Cause.MOUSE_EVENT);

Can this action be performed using a keyboard or this can be reached only using mouse press?
If this actionPerformed can be reached using keyboard, i think we should add appropriate checks before throwing the focus event cause.

src/java.desktop/share/classes/javax/swing/text/DefaultCaret.java line 585:

> 583:                                    component.isRequestFocusEnabled()) {
> 584:             if (inWindow) {
> 585:                 component.requestFocusInWindow(FocusEvent.Cause.MOUSE_EVENT);

Looks like this function is also called only in case of mouse events from mousePressed()->adjustCaretAndFocus() and mouseClicked(). Seems fine.

test/jdk/javax/swing/event/FocusEventCauseTest.java line 59:

> 57:                 frame = new JFrame("FocusEventCauseTest");
> 58:                 JPanel panel = new JPanel();
> 59:                 button1 = new JButton("Button1");

Generic test comment. Its good if we can check focus event for all UI components that are getting updated.

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

PR Review: https://git.openjdk.org/jdk/pull/14004#pullrequestreview-1448785377
PR Review Comment: https://git.openjdk.org/jdk/pull/14004#discussion_r1208862476
PR Review Comment: https://git.openjdk.org/jdk/pull/14004#discussion_r1208863379
PR Review Comment: https://git.openjdk.org/jdk/pull/14004#discussion_r1208874028
PR Review Comment: https://git.openjdk.org/jdk/pull/14004#discussion_r1208879547



More information about the client-libs-dev mailing list