On Fri, 8 Nov 2024 20:44:17 GMT, John Hendrikx <jhendr...@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
