RFR: 8306021: Add event handler management to EventTarget [v2]

[Nir Lisker]

On Sat, 15 Apr 2023 18:01:28 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> This PR adds the following methods to the `EventTarget` interface:
>> 1. `addEventHandler`
>> 2. `removeEventHandler`
>> 3. `addEventFilter`
>> 4. `removeEventFilter`
>
> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
> 
>   revert a change in Menu

Looks good overall.

modules/javafx.base/src/main/java/javafx/event/EventTarget.java line 79:

> 77:      * <p>
> 78:      * It is possible to register a single {@link EventHandler} instance for different event types,
> 79:      * so the caller needs to specify the event type from which the handler should be unregistered.

I would rephrase to "Since it is possible to..., the caller needs to..."

Same for filter.

modules/javafx.base/src/main/java/javafx/event/EventTarget.java line 95:

> 93:      * Registers an event filter for this target.
> 94:      * <p>
> 95:      * The filter is called when the node receives an {@link Event} of the specified

node -> target

modules/javafx.controls/src/main/java/javafx/scene/control/TableColumnBase.java line 737:

> 735:      * {@inheritDoc}
> 736:      * <p>
> 737:      * The {@code TableColumnBase} class allows registration of listeners which will be notified when editing occurs.

The edit events are defined only in the subclasses. Either, if possible, move the events to this class, or clarify that these events are defined in subclasses. `TreeItem` talks more about this - using the cell.

modules/javafx.controls/src/main/java/javafx/scene/control/TableColumnBase.java line 738:

> 736:      * <p>
> 737:      * The {@code TableColumnBase} class allows registration of listeners which will be notified when editing occurs.
> 738:      * Note that {@code TableColumnBase} is <b>not</b> a {@link Node}, and therefore no visual events will be fired on it.

Isn't this true for the other classes here as well, like `Tab`? Might it be worth saying this in the other classes too then, or even on `EventTarget` itself (something like "Note that implementing classes that are not a `Node` don't receive visual events")?
I would also give an example or 2 of what "visual events" are since this term is not defined anywhere that I know of.

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

PR Review: https://git.openjdk.org/jfx/pull/1090#pullrequestreview-1387150109
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1168092516
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1168092983
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1168115120
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1168115726