On Mon, 10 Apr 2023 22:07:41 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> As with the earlier comment, any changes to the behavior of a null listener
>> should be done separately and not part of this PR.
>
> Alright, I'll file another ticket for that. The general state of this class
> is quite poor, as it does not follow any of the contracts of the interfaces
> it implements (including `Collection`, `Set` and `ObservableSet`). This is
> bad because even though this class is an implementation detail, it does get
> exposed as an `ObservableSet` via `Node#getPseudoClassStates`. Things that
> it violates for example are:
>
> node.getPseudoClassStates().retainAll(null); // no exception
>
> Or:
>
> node.getPseudoClassStates().retainAll(set2); // does nothing if set2
> is not a BitSet
>
> Or:
>
> // fails, even if they're the same because `equals` assumes another
> BitSet
>
> node.getPseudoClassStates().equals(Set.of(PseudoClass.getPseudoClass("armed"));
>
> // works:
>
> Set.of(PseudoClass.getPseudoClass("armed")).equals(node.getPseudoClassStates());
>
> Same goes for the hash code value; these are strictly defined for sets, but
> `BitSet` violates it causing problems when using these sets as keys. This
> causes numerous odd problems that I had to track down while implementing
> #1076 -- I can only assume very few people are using these API's in the wild.
Filed https://bugs.openjdk.org/browse/JDK-8305816 so we don't forget.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1071#discussion_r1162156819
