On Fri, 28 Aug 2020 16:45:38 GMT, Ambarish Rapte <ara...@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 controls? 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
