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

[Kevin Rushforth]

On Mon, 17 Apr 2023 06:00:19 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:
> 
>   doc changes

The spec looks fine except for a few missing `@since` tags that need to be restored, a suggestion on adding `@implSpec` (from Joe), and a couple unrelated doc changes that should be reverted. See inline.

As for the implementation, the event handling methods in the `Dialog` and `Tab` classes are new (although they just delegate to an existing instance of `EventHandlerManager`). Would it be possible to provide a test?

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

> 66:      * @param eventHandler the event handler
> 67:      * @throws NullPointerException if {@code eventType} or {@code eventHandler} is {@code null}
> 68:      * @throws UnsupportedOperationException if this target does not support event handlers

Joe Darcy made a recommendation while reviewing the CSR to add an `@implSpec` tag to the 4 added default methods documenting their behavior, which I think is a good suggestion. Here is a possible wording:


@implSpec The default implementation of this method throws {@code UnsupportedOperationException}.

modules/javafx.controls/src/main/java/javafx/scene/control/TableColumn.java line 85:

> 83:  *     <li>{@link #editCancelEvent()}
> 84:  * </ul>
> 85:  *

These doc additions are unrelated to the this PR.

modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableColumn.java line 80:

> 78:  *     <li>{@link #editCancelEvent()}
> 79:  * </ul>
> 80:  *

These doc additions are unrelated to the this PR.

modules/javafx.graphics/src/main/java/javafx/concurrent/Service.java line 744:

> 742:     }
> 743: 
> 744:     @Override

By removing the docs, you lost the `@since` for this method, so you will need to restore it. You might consider also adding `{@inheritDoc}` to avoid copying all of the description

modules/javafx.graphics/src/main/java/javafx/concurrent/Task.java line 1252:

> 1250: 
> 1251:     @Override
> 1252:     public final <T extends Event> void addEventHandler(

Same comment in this file about the need to restore the `@since tag`.

modules/javafx.graphics/src/main/java/javafx/scene/transform/Transform.java line 1915:

> 1913:      * <p>
> 1914:      * Currently the only event delivered to a {@code Transform} is the {@code TransformChangedEvent}
> 1915:      * with its single type {@code TRANSFORM_CHANGED}.

You need to restore the `@since`

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

Changes requested by kcr (Lead).

PR Review: https://git.openjdk.org/jfx/pull/1090#pullrequestreview-1472963662
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1224696281
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1224894283
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1224894379
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1224877090
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1224877506
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1224881183