On Wed, 24 May 2023 10:41:09 GMT, Prasanta Sadhukhan <[email protected]> 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
