RFR: 8313956: focusWithin on parents of a newly-added focused node is not updated [v3]

[John Hendrikx]

On Fri, 18 Aug 2023 13:39:59 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> This PR fixes an issue with the way `focusWithin` bits are adjusted in the scene graph. Previously, the `focusWithin` counts of all parents of a removed node would be decreased if the removed node has a non-zero `focusWithin` count. This PR adds logic for the opposite scenario: the `focusWithin` count is increased on all parents of a newly-added node that is already focused (or contains other focused nodes).
>
> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Correctly handle added node with multiple focuses

LGTM

modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 8329:

> 8327:                 set(true);
> 8328:             } else if (count == 0) {
> 8329:                 set(false);

This looks a bit odd due to the use of two different variables, but it works (`count > 0` would also have worked).  I personally would favor something without any if statements as it leaves less room for misinterpretation in this case:

     count += change;

     set(count > 0);

modules/javafx.graphics/src/test/java/test/javafx/scene/FocusTest.java line 1146:

> 1144:         node3 = new N(
> 1145:             node4 = new N());
> 1146: 

ignore: I think this looks a bit inconsistent (in the first version not all parenthesis are closed on the last line, while they are in the second version).  Personally I favor this, similar to how curly braces are used):
Suggestion:

        scene.setRoot(
            node1 = new N(
                node2 = new N()
            )
        );

        node3 = new N(
            node4 = new N()
        );

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

Marked as reviewed by jhendrikx (Committer).

PR Review: https://git.openjdk.org/jfx/pull/1210#pullrequestreview-1585180011
PR Review Comment: https://git.openjdk.org/jfx/pull/1210#discussion_r1298838771
PR Review Comment: https://git.openjdk.org/jfx/pull/1210#discussion_r1298840452