On Mon, 14 Sep 2026 12:13:05 GMT, Kevin Rushforth <[email protected]> wrote:
>> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix logic error, luckily 5000 tests caught it > >> > The following behaviors are now specified: >> > >> > ... >> > * `oldValue` is documented as the preceding observed value, although this >> > doesn't allow for it being the initial value when the listener is first >> > notified; additionally, setting a value that is "equals" but not "==" to >> > the previous will suppress that notification, but cause a subsequent >> > notification to use the newer of the two equal-but-not-same objects (this >> > is a preexisting inconsistency that affects the new spec). >> >> I think since we've always defined change listeners in terms of `equals` (we >> don't fire if things are `equals`) this is not really inconsistent. > > Agreed. > >> Whether the old value reported is the oldest `==` variant or the newest `==` >> variant should not matter > > I think you mean the oldest `equals` variant vs the newest `equals` variant > (references that are `==` are indistinguishable, by definition). I agree that > it doesn't really matter. The only reason I brought this up is to keep in > mind when reviewing the final docs. As long as the docs give some wiggle room > on this, we are fine. > >> > * Later change listeners are documented as seeing changes made by earlier >> > change listeners (including the ability to veto, which prevents later >> > listeners from seeing intermediate values) >> > * The behavior of adding and removing change listeners during notification >> > >> > Implied, but not specified: >> > >> > * A ChangeListener will never be called with `oldValue` equal to >> > `newValue` (should this be made explicit?) >> >> We could do that, but that does break for the observable list/map/sets which >> don't truly provide old values (arguably, they should never have allowed >> change listeners, only their specific variants and invalidation listeners) > > Yes, good point. So we could either leave it implied, or specify that it > doesn't apply to observable collections. > >> > Most of the above behaviors of a ChangeListener only apply to >> > ObservableValueBase and other JavaFX properties and bindings that were >> > migrated to use the new listener manager implementation (not, for example, >> > JavaBeanObjectProperty, collection property classes, such as ListProperty, >> > and collection bindings, such as ListBinding). One solution would be to >> > move the guarantees to ObservableValue and list the classes that apply >> > those guarantees, perhaps in an @implNote. >> >> Agreed. > > That seems best. > >> > * invalidation listeners are called before change listeners >> >> We could specify this, as changing this now or in the future would likely >> ca... @kevinrushforth I've fixed the single listener -> listener list bug you found using a notifying boolean (as discussed) that is maintained now next to the listenerData field; in addition I updated the documentation in various places to detail what JavaFX properties will guarantee (with the exception of the collection types); to ensure the only exception is the collection types, I've converted the Java bean properties and the only two other uses in TextInputControl (and now removed `ExpressionHelper`) The collection types are better left for a follow-up PR (I noticed that many of the fixes done in `ExpressionHelper` in the past never made it to their specific helpers...) I think this is ready for you to have another look. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1081#issuecomment-5682565532
