Promote addEventHandler/removeEventHandler to EventTarget interface

[Kevin Rushforth]

An incompatible change in a fundamental interface such as EventTarget is 
not something we would want to do, so any proposed changes will need to 
retain compatibility (both binary and source). At a minimum, this means 
that any newly added methods would need to have a default 
implementation, and we would need to look closely at other aspects of 
compatibility.

-- Kevin


On 3/7/2023 1:24 AM, 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?