RFR: 8290310: ChangeListener events are incorrect or misleading when a nested change occurs [v3]

[John Hendrikx]

On Tue, 18 Feb 2025 00:38:14 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

> > > So in the first case, the value is corrected.
> > 
> > I'm unsure what you mean by corrected here
> 
> I mean compared to the old behavior. In the old behavior, the listener was notified with an incorrect old value, while with the prospective new code (with added listeners) the listener will be notified with a correct value. With the current new code the listener isn't notified at all. So, the behavioral change, instead of incorrect->nothing, will be incorrect->correct.

Well, it is not truly an incorrect old value for a **new** listener (in `ExpressionHelper`), as it is only notified that one time, so there is no basis for comparison.  I mean, the first call is always sort of correct (and `F` was only called one time in the nested notification, and was not called a 2nd time in the top level one).  Only from the 2nd call is where it starts to get interesting, where users will expect that the old value will be the new value of the previous call... but I think I see what you're saying.

> In the second case I listed, instead of the current nothing->nothing (no behavioral change), it will be nothing->correct.

Yes, and I thought this would be quite a common case... but see below.

> > But is it a specified behavior or just an implementation artifact of `ExpressionHelper`?
> 
> No, thankfully, very little is specified. This allows us to implement a more sensible behavior...

If there is one :)

> I'm not sure myself if it makes sense since I've never done this and cared about the behavior. I suppose it could go either way. If the first way is too complicated, the second one (this PR) is also reasonable.

I don't think that notifying a listener that was added during a notification of the same property would be too hard to implement.

I also think I misevaluated the risks involved (I feared FX wouldn't even build).  You'd only get an immediate notification if you add a listener to the **same** property that just notified you -- that's likely to be an exceedingly rare situation (after all, you already have a listener).  I mixed this up with the pattern where you use the old and new values to unregister/reregister listeners -- but those are different properties, and so would not be subject to this change that gives you immediate notification.

So, I now don't have a real preference anymore. Whether we should immediately notify a listener added during notification of the **same** property seems to be a very hypothetical case -- one which we're likely to leave unspecified no matter which solution we pick here.

> > One immediate problem however is that the `newValue` received may differ from a direct `get`
> 
> I think it should be specified that these are the same, no?

It's not specified currently (couldn't find it on `ObservableValue` or `ChangeListener` docs) -- however, I think it is strongly expected by users that this is the case :)

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

PR Comment: https://git.openjdk.org/jfx/pull/1081#issuecomment-2664346858