On Mon, 10 Apr 2023 20:45:22 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/java/com/sun/javafx/css/BitSet.java line
>> 617:
>>
>>> 615: @Override
>>> 616: public void removeListener(InvalidationListener
>>> invalidationListener) {
>>> 617: if (invalidationListener != null) {
>>
>> `Observable.removeListener(InvalidationListener)` is specified to reject
>> `null` by throwing NPE.
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1071#discussion_r1162126275
