[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-openjfx-dev
Subject: Re: [jfx20] RFR: 8300013: Node.focusWithin doesn't account for nested focused nodes [v2]
From: Michael =?UTF-8?B?U3RyYXXDnw==?= <mstrauss () openjdk ! org>
Date: 2023-01-31 17:35:25
Message-ID: 0c5S8Bbh2lc0drm3HSs-7sXzKM01N_AiPcHdGqiaPUw=.a3d06bbd-165f-43c4-a5a7-12497087aa96 () github ! com
[Download RAW message or body]
On Tue, 31 Jan 2023 17:08:40 GMT, Michael Strauß <mstrauss@openjdk.org> wrote:
> > 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.
I've created a ticket to investigate this: \
https://bugs.openjdk.org/browse/JDK-8301556
-------------
PR: https://git.openjdk.org/jfx/pull/993
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic