On Thu, 28 Jul 2022 14:59:06 GMT, Michael Strauß <mstra...@openjdk.org> wrote:

> The `focusVisible` flag is only set on a node when it acquires input focus 
> using a keyboard interaction, and it is cleared by mouse and touch 
> interactions.
> 
> It is not necessary for a node to lose input focus in order to lose 
> `focusVisible`. Currently, clicking on a region of the scene that does not 
> contain a focusable node does not clear the `focusVisible` flag.
> 
> Detecting clicks or touches that should clear `focusVisible` can be achieved 
> by adding an event filter for `MOUSE_PRESSED` and `TOUCH_PRESSED` to the 
> `Scene`.
> 
> There are two additional noteworthy changes with this PR:
> 1. Adding event filters to `Scene` causes many tests to fail due to a bug in 
> `MouseEventFirer` that was identified in 
> [8253769](https://bugs.openjdk.org/browse/JDK-8253769). Previously, mouse 
> events generated by `MouseEventFirer` skipped a code path that causes test 
> failures due to incorrect coordinates. With the added event filters, the code 
> path for mouse events is slightly different, exposing the incorrect 
> coordinates and causing tests to fail.
> This problem can be solved by making the "alternative" `MouseEvent` 
> generation path the default path.
> 
> 2. The changes around `KeyHandler` might seem to be excessive for the scope 
> of this PR. However, this is mostly just code that was moved to another place 
> (it was moved right next to the rest of the focus-related functionality in 
> `Scene`). `KeyHandler` was a dubious class before (since it didn't seem to 
> have a clear purpose, mixing focus handling with event propagation), but with 
> the need to call `KeyHandler.setFocusVisible` from mouse and touch event 
> handlers, its purpose became even less pronounced.
> I think that moving the focus-related functionality next to the other focus 
> functions in the `Scene` class is a safe and straightforward change, and it 
> makes it easier to work with the code in the future.
> With the focus-related functions gone, `KeyHandler` only contains a single, 
> small method that is called from `Scene.processKeyEvent`. For simplicity, I 
> decided to roll this code into `Scene.processKeyEvent` and remove the 
> now-empty `KeyHandler` class.
> 
> Note: Since this is a follow-up fix for 
> [8268225](https://bugs.openjdk.org/browse/JDK-8268225), I'm targeting this 
> fix for `jfx19`.

This will need to be very well-tested to ensure that there are no regressions. 
As part of the review, we will evaluate whether it is a safe enough fix to go 
into 19.

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

PR: https://git.openjdk.org/jfx/pull/852

Reply via email to