RFR: 8207759: VK_ENTER not consumed by TextField when default Button exists [v3]

[Martin Fox]

On Fri, 8 Nov 2024 20:44:17 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> @kleopatra Your explanation makes sense to me. Thank you for taking the time to write it up.
>> 
>> If so, the proposed fix, including adjusting the comment, would be:
>> 
>> 
>> --- a/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextFieldBehavior.java
>> +++ b/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextFieldBehavior.java
>> @@ -162,9 +162,8 @@ public class TextFieldBehavior extends TextInputControlBehavior<TextField> {
>> 
>>          textField.commitValue();
>>          textField.fireEvent(actionEvent);
>> -        // fix of JDK-8207759: reverted logic
>> -        // mapping not auto-consume and consume if handled by action
>> -        if (onAction != null || actionEvent.isConsumed()) {
>> +        // Consume the original event if the copied actionEvent was consumed.
>> +        if (actionEvent.isConsumed()) {
>>              event.consume();
>>          }
>>      }
>
> You are right, `&&` would be insufficient as there is another way to add action handlers.  I didn't really see that, although the fix I had in mind was to eliminate the `onAction` variable completely (it is unused in your fix as well), so probably would have ended up at the correct result :)
> 
> I can roll this into a small PR if you wish.

FWIW, checking the isConsumed flag will only work if the action is consumed by a handler registered directly with the TextField. If there's a filter or handler attached to some other part of the scene graph a copy of the event will be made and consumed and `actionEvent.isConsumed()` will return false. A more complete fix is to replace `textField.fireEvent` with a call to `EventUtil.fireEvent` and check for a `null` return value.

If I understand this thread correctly if a KeyEvent generates an ActionEvent the KeyEvent should only be consumed if the ActionEvent is. If so the behavior should be consistent across TextFields and the controls that manage TextFields internally (Spinner, ComboBox, and DatePicker). I'm already working on a test for Escape key handling for this set of controls (for Escape the issue is whether a formatter is attached or not). I'm willing to write up a similar test for Enter handling for the same set of controls. I just need to make sure I understand exactly what behavior is expected.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/15#discussion_r1835102922