On Wed, 17 May 2023 21:59:58 GMT, John Hendrikx <[email protected]> wrote:
>> modules/javafx.graphics/src/main/java/com/sun/javafx/css/BitSet.java line
>> 266:
>>
>>> 264: @Override
>>> 265: public boolean addAll(Collection<? extends T> c) {
>>> 266: if (this.getClass() != c.getClass()) {
>>
>> same, handling of the null 'c' argument.
>> I wonder if this is intended behavior?
>
> I doubt the `Set` contract was breached here on purpose. More likely, the
> original implementation was never exposed, or the original developer didn't
> realize that `Set` has a very specific contract that you must follow, or it
> won't interact well with other collections.
To be clear, `BitSet` is currently exposed via `Node#getPseudoClassStates`.
I suspect it is almost never used by anyone, because there are a lot of bugs.
For example, equality won't work correctly:
node.getPseudoClassStates().equals(Set.of( ... )); // Always false, sets
are of different types
Nor will `containsAll`:
node.getPseudoClassStates().containsAll(Set.of( ... )); // Always false,
because Set is not a BitSet
node.getPseudoClassStates().containsAll(null); // Doesn't throw NPE,
returns false
All of these are `Set` contract violations, that I suppose were thought to be
irrelevant because it is a com.sun.* class.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1070#discussion_r1197116933
