Promote addEventHandler/removeEventHandler to EventTarget interface

[John Hendrikx]

Hi Michael,


Did you file a JIRA issue for this one?

I've recently also been doing some rewriting to use the Event system 
more.  I'm removing custom Scene walking code (and looking at 
Node.getProperties) to do "event handling", and I've now discovered that 
it could be done quite a bit nicer by using the provided event mechanism.

I've encountered a few things that annoy me about the event system:

1) I'm making use of Presentation classes (Models) that only associate 
with things in javafx.base (Event, EventTarget, Properties). These 
presentations need to register event handlers at some point, but this 
can only be done on Nodes; having this logic close to the Presentation 
code makes sense, but it would make them dependent on javafx.graphics; 
if I could do this via EventTarget, the dependency would not be needed.

2) When you fire an event (via Node.fireEvent for example), there is no 
feedback you can obtain whether the event was consumed or not as the 
final event is not returned to check `isConsumed` on (internal calls do 
have this information, but it is not exposed at the last step).

3) Related to 2), I think EventTarget could also have a method to 
dispatch events (so you can dispatch an event easily to any target, not 
just via the convenience method Node#fireEvent).  See this ticket: 
https://bugs.openjdk.org/browse/JDK-8303209

Perhaps we can work together on this, or if you're not currently working 
on it I could take a stab at solving all of these issues.

--John
On 07/03/2023 10:24, John Hendrikx wrote:
> Hi Michael,
>
> Did you file a JIRA issue for this one?
>
> I've recently also been doing some rewriting to use the Event system 
> more.  I'm removing custom Scene walking code (and looking at 
> Node.getProperties) to do "event handling", and I've now discovered 
> that it could be done quite a bit nicer by using the provided event 
> mechanism.
>
> I've encountered a few things that annoy me about the event system:
>
> 1) I'm making use of Presentation classes (Models) that only associate 
> with things in javafx.base (Event, EventTarget, Properties). These 
> presentations need to register event handlers at some point, but this 
> can only be done on Nodes; having this logic close to the Presentation 
> code makes sense, but it would make them dependent on javafx.graphics; 
> if I could do this via EventTarget, the dependency would not be needed.
>
> 2) When you fire an event (via Node.fireEvent for example), there is 
> no feedback you can obtain whether the event was consumed or not as 
> the final event is not returned to check `isConsumed` on (internal 
> calls do have this information, but it is not exposed at the last step).
>
> 3) Related to 2), I think EventTarget could also have a method to 
> dispatch events (so you can dispatch an event easily to any target, 
> not just via the convenience method Node#fireEvent).  See this ticket: 
> https://bugs.openjdk.org/browse/JDK-8303209
>
> Perhaps we can work together on this, or if you're not currently 
> working on it I could take a stab at solving all of these issues.
>
> --John
>
> On 17/03/2022 21:01, Michael Strauß wrote:
>> I'm working on an application that uses the JavaFX event system
>> extensively, and I'm finding it quite hard to use common code for
>> event handler registrations.
>>
>> The problem is that the `EventTarget` interface contains no
>> addEventHandler/removeEventHandler methods, and as a consequence of
>> that, code that uses `EventTarget` ends up requiring lots of brittle
>> instanceof tests to call these methods on all the different
>> implementations of `EventTarget`.
>>
>> There are three buckets of `EventTarget` implementations:
>>
>> 1) Implementations that declare the following methods:
>>      <T extends Event> void addEventHandler(EventType<T>,
>> EventHandler<? super T>);
>>      <T extends Event> void removeEventHandler(EventType<T>,
>> EventHandler<? super T>);
>>      <T extends Event> void addEventFilter(EventType<T>, EventHandler<?
>> super T>);
>>      <T extends Event> void removeEventFilter(EventType<T>,
>> EventHandler<? super T>);
>> --> Node, Scene, Window, Transform, Task, Service
>>
>> 2) Implementations that declare the following methods:
>>      <T extends Event> void addEventHandler(EventType<T>, 
>> EventHandler<T>);
>>      <T extends Event> void removeEventHandler(EventType<T>, 
>> EventHandler<T>);
>> --> MenuItem, TreeItem, TableColumnBase
>>
>> (Note that the EventHandler argument ist parameterized as
>> EventHandler<T>, not EventHandler<? super T> as in the first set of
>> methods.)
>>
>> 3) Implementations that don't declare any methods to add or remove
>> event handlers:
>> --> Dialog, Tab
>>
>>
>> I think the situation can be improved by promoting the bucket 1
>> methods to the `EventTarget` interface, so they can be used
>> consistently across all implementations of `EventTarget`.
>>
>> This works seamlessly for bucket 1 and bucket 3 implementations.
>>
>> With bucket 2, there's the problem that, inconsistently, the
>> EventHandler<T> argument is not a lower-bounded wildcard.
>> Unfortunately, a method with an EventHandler<T> parameter cannot
>> implement an interface method that expects EventHandler<? super T>.
>>
>> However, since the erasure of the method remains the same, changing
>> the method signature would technically be a binary-compatible change.
>>
>> Do you think this is a useful improvement?