On Mon, 18 May 2026 19:17:57 GMT, Martin Fox <[email protected]> wrote:
>> This PR alters the way ComboBox and Spinner deliver KeyEvents to their >> TextField editors. When a ComboBox or Spinner is the focus owner it is the >> target of all key events. Currently the skin installs a filter to catch key >> events and re-fire most of them at the TextEdit. The skin copies the event, >> fires the copy at the TextField, and then consumes the original event. This >> confuses the system menu bar logic on macOS; only the original event can >> trigger a menu item and that event is always being consumed. >> >> In this PR only the original key event makes its way up and down the event >> dispatch chain. To drive the TextField the skin delivers the event copy >> directly to the TextField's event dispatcher and only consumes the original >> event if the TextField consumes the copy. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Martin Fox has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains five additional commits since > the last revision: > > - Merge remote-tracking branch 'upstream/master' into combospinnernofire > - Moved event dispatch to utility method in EventUtil. > - Clean up based on review feedback > - Only run tests on Mac > - New approach to forwarding key events to ComboBox and Spinner editors modules/javafx.controls/src/main/java/javafx/scene/control/skin/ComboBoxPopupControl.java line 183: > 181: > 182: // Returns 'true' if the event was consumed. > 183: private boolean forwardToTextField(KeyEvent event) { this method seems unnecessary, why not fold it into `EventUtil.dispatch()` including copying and and returning a boolean? and perhaps consuming the original event? modules/javafx.controls/src/main/java/javafx/scene/control/skin/ComboBoxPopupControl.java line 617: > 615: ke.consume(); > 616: } else if (textField != null) { > 617: forwardToTextField(ke); Should the `ke` event be consumed if its copy was consumed? ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2166#discussion_r3262005508 PR Review Comment: https://git.openjdk.org/jfx/pull/2166#discussion_r3261993988
