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