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

Andy Goryachev angorya at openjdk.org
Thu Aug 8 16:26:37 UTC 2024


On Thu, 8 Aug 2024 15:57:04 GMT, Martin Fox <mfox at openjdk.org> wrote:

> This PR assumes that during dispatch events are copied using copyFor. But the documentation states that an EventDispatcher can do whatever it wants in `dispatchEvent`. A developer could provide a dispatcher that creates entirely new events rather than just copy existing ones and that would break this code. The only thing that’s guaranteed is that the dispatcher will return null to stop event dispatch and that's all that the solution in [JDK-8303209](https://bugs.openjdk.org/browse/JDK-8303209) relies on.

a) If the EventDispatcher does something different like creating the new events, it is up to that ED to deal with consequences.  It means the developer wants to create new events and not to have the link between the events.

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.

The bigger issue I see, as I mentioned earlier, is that 
1. the `Event` has the target field which is absolutely, totally unnecessary (the target already knows who the target is, it is the target) and is an anti-pattern because
2. it forces the dispatcher to create new event instances for new targets, breaking the `isConsumed` logic

I don't think we'll ever change the event dispatching mechanism and remove the target field from Event.  So the only remaining solutions are little hacks and workarounds here and there.

I don't think adding return value to the dispatcher is the right solution.

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

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


More information about the openjfx-dev mailing list