RFR: 8337246: SpinnerSkin does not consume ENTER KeyEvent when editor ActionEvent is consumed

[Martin Fox]

On Thu, 8 Aug 2024 17:18:56 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>>> b) I am not convinced that returning null from a dispatcher is a good solution. We already have an API for that: `Event.isConsumed()`. Adding a new, parallel paradigm, in my opinion is unnecessary.
>> 
>> We would not be adding anything. Consuming an event is how a developer communicates to one EventDispatcher that the event was dealt with. But this information is propagated through the dispatch chain by returning null from dispatchEvent. That's all in place already.
>> 
>> There's a variant of `EventUtil.fireEvent` that returns the result of dispatchEvent so a client can check for null to see if the event was consumed. And there are already places in the code where this happens. [JDK-8303209](https://bugs.openjdk.org/browse/JDK-8303209) calls for this to be cleaned up and made public.
>
> @beldenfox :
> EventUtil is not a public API.  Checking the return value of the dispatchEvent is, in my opinion, an anti-pattern.  The right pattern is this (TabPaneBehavior:85):
> 
> 
>     public boolean canCloseTab(Tab tab) {
>         Event event = new Event(tab,tab,Tab.TAB_CLOSE_REQUEST_EVENT);
>         Event.fireEvent(tab, event);
>         return ! event.isConsumed();
>     }
> 
> 
> Using your own argument, a dev can write a dispatcher which returns garbage from its `dispatchEvent` method, breaking the thing.  So I am still not convinced.

@andy-goryachev-oracle I agree that propagating the consumed flag is clearer and would lead to less surprises. Once upon a time I also thought that I could call fireEvent and then check the event's consumed flag afterward. I had to dig around a bit to figure out that all these copies were being made. It is confusing.

For that matter you can't really infer that an event was consumed just because the dispatch chain returned null. The EventDispatcher documentation is a little hazy on this but in theory a dispatcher could return null for other reasons. It seems to be mostly a matter of convention that null indicates that an event was consumed.

With that said in this PR changing the consumed flag in one event can change the consumed state of another event and that also seems like unexpected and surprising behavior.

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

PR Comment: https://git.openjdk.org/jfx/pull/1523#issuecomment-2276584223