[jfx20] RFR: 8300013: Node.focusWithin doesn't account for nested focused nodes [v2]

Michael Strauß mstrauss at openjdk.org
Tue Jan 31 17:14:20 UTC 2023


On Tue, 31 Jan 2023 16:42:32 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   refactoring
>
> modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 8325:
> 
>> 8323:             } else if (count == 0) {
>> 8324:                 set(false);
>> 8325:             }
> 
> Is it worth adding a check for `count < 0` and logging a warning (possibly treating it as if it were 0)? In theory, it shouldn't happen, but if it ever did, `focusWithin` would be wrong after that. This could be done as a P4 follow-up for 21, unless you are certain that it cannot ever happen.

On the one hand, this might make the code a little bit more robust, but on the other hand, it might hide a bug elsewhere. Surely `count` should never be negative. I lean slightly towards not protecting code against bugs in this way, mostly because it might expose potential bugs sooner.

If we want to validate that this method is not called with an incorrect argument, maybe just throwing an exception would be better.

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

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


More information about the openjfx-dev mailing list