RFR: 8209788: Left/Right/Ctrl+A keys not working in editor of ComboBox if popup showing [v8]

Kevin Rushforth kcr at openjdk.java.net
Fri Aug 28 17:52:15 UTC 2020

On Fri, 28 Aug 2020 16:45:38 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:

>> The issue occurs because the key events are consumed by the `ListView` in `Popup`, which displays the items.
>> This is a regression of [JDK-8077916](https://bugs.openjdk.java.net/browse/JDK-8077916). This change aadded several
>> `KeyMapping`s for focus traversals to `ListView`, which consume the `Left`, `Right` and `Ctrl+A` key events.
>> Fix:
>> 1. Remove the four focus traversal arrow `KeyMapping`s from `ListViewBehavior`.
>> 2. Add the `Ctrl + A` `KeyMapping` to `ListViewBehavior` only if the `ListView`'s selection mode is set to
>> `SelectionMode.MULTIPLE`.  `ComboBox` uses the `ListView` with `SelectionMode.SINGLE` mode.
>> Change unrelated to fix:
>> `ComboBoxListViewBehavior` adds `KeyMapping` for `Up` and `Down` keys, which are not invoked when the `ComboBox` popup
>> is showing. When the popup is shown, the Up and Down key events are handled by the `ListView` and the `KeyMapping` code
>> from `ComboBoxListViewBehavior` does not get executed. These KeyMapping are removed. However this change is not needed
>> for the fix. But this seems to be dead code.   Verification:
>> Added new unit tests to verify the change.
>> Also verified that the behavior ListView behaves same before and after this change.
> Ambarish Rapte has updated the pull request incrementally with one additional commit since the last revision:
>   Fix for non editable ComboBox

I haven't tested it, but it looks like it should work. I left a couple of minor suggestions below.

Would it be possible to add some tests to verify the behavior of HOME and END for editable and non-editable ComboBox

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/ListViewBehavior.java line 143:

> 142:
> 143:         if (Boolean.FALSE.equals(control.getProperties().containsKey("excludeKeyMappingsForComboBoxEditor"))) {
> 144:             // This is not ComboBox's ListView

Unless I'm missing something, you don't need to compare with a `Boolean` at all since containsKey returns a `boolean`
primitive type, so I think this can be simplified to:

if (!control.getProperties().containsKey("excludeKeyMappingsForComboBoxEditor")) {

If instead you do want to check the value of the property, and not just its existence, you would need the following,
and it would be important to check `!TRUE.equals` rather than `FALSE.equals` so that an unset property would be treated
as `false`.

if (!Boolean.TRUE.equals(control.getProperties().get("excludeKeyMappingsForComboBoxEditor"))) {

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/ListViewBehavior.java line 359:

> 358:         Boolean isComboBoxEditable = (Boolean)getNode().getProperties().get("editableComboBoxEditor");
> 359:         if (isComboBoxEditable != null) {
> 360:             // This is ComboBox's ListView

Maybe simplify this as follows, to match the earlier logic?

if (Boolean.FALSE.equals(getNode().getProperties().get("editableComboBoxEditor"))) {


PR: https://git.openjdk.java.net/jfx/pull/172

More information about the openjfx-dev mailing list