On Mon, 3 Apr 2023 06:29:29 GMT, Michael Strauß <mstra...@openjdk.org> wrote:

>> BitSet uses the SetListenerHelper abstraction to prevent allocating the 
>> listener arrays.
>> 
>> When removing listeners, the newly returned listener helper (which may be 
>> different from the one called) is not reassigned. This effectively means 
>> that removing the listener does not happen.
>> 
>> This fix correctly assigns the potentially changed SetListenerHelper 
>> instance to BitSet's helper field after listener removal.
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/css/BitSet.java line 603:
> 
>> 601:     @Override
>> 602:     public void removeListener(SetChangeListener<? super T> 
>> setChangeListener) {
>> 603:         if (setChangeListener != null) {
> 
> Even if not explicitly specified for 
> `ObservableSet.removeListener(SetChangeListener)`, implementations generally 
> reject `null` arguments by throwing NPE. This is the default behavior of 
> `SetListenerHelper`.

I could remove these checks in this PR, or as part of the larger `BitSet` 
clean-up PR.   The extra check doesn't break anything though, while this PR is 
mainly focused on fixing a long standing bug that everyone dealing with 
`ObservableSet<PseudoClass>` can encounter currently.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1071#discussion_r1155572378

Reply via email to