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

[John Hendrikx]

On Thu, 8 Feb 2024 21:25:41 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

> I completely forgot about this PR, but it looks very interesting, especially about the nested events. If helpful, I can test this soon in a bigger application, especially for any regressions. And of course also review the code, but need more time for that.

@Maran23 both will be appreciated; I've tested this with my own FX applications for a while, and haven't encountered anything unexpected.

> Tested with a big application, did not find any regression. All listeners still work as expected, tested especially a lot of 'if something is selected, this button should be enabled/disabled' and the like. I did not checked the code yet, just a little bit. One question: Should I test something special/do you see a case which could cause a problem here?

I think testing it with a big application is excellent (I do the same here with my own large application), and I'm happy to hear you did not spot any regressions!

This change does change (unspecified) behavior a little bit, although I think well within the documented contract of how change listeners operate (ie. applications should not be relying on the unspecified behavior).

It will be hard to notice any change directly, as the changes are all related to advanced usage of listeners (adding/removing listeners during listener callbacks) or nested changes (the same property gets changed **during** a callback, directly or indirectly).

1. During a listener callback, you remove a listener that is currently being notified (ie. yourself, or any listener that may have triggered your callback that is further up the call stack)
  - Before this change, there is a chance that such a removed listener may still be called a few more times, despite it being removed in nested cases; its doubtful anyone is relying on that behavior, and (IMHO) these extra callbacks are more likely to break things, because things have been cleaned up (who assumes that their listener might still get called after removal?)
2. During a listener callback, you add a listener that is currently being notified (this can't be yourself, but it could be a listener on a property that triggered your callback that is further up the stack)
  - Before this change, this newly added listener may start participating in the current nested listener notification (ie. it would start participating halfway during a chain of nested changes).  After this change, such a newly added listener would only be notified on the **next** top level change.  It's unlikely anyone is relying on this behavior.
3. During a listener callback, you change the value the property it is attached to (or any property that may have led to your callback being called further up the callstack).
  - The most "common" scenario for this is veto-ing changes.  For example, a property that can't be empty may get set to a non-empty value (or its original value) in a listener, triggering another notification.  Your listener would not notice any differences (it would get called immediately with the change that you just did), but any listeners added after yours may not "see" the empty value because it was changed to something else before they were called.  They will only see the new value (and the corresponding **correct** old value since last they were called) -- they may also be not called at all if the value was changed back to its original, and so they would be completely unaware of the temporary change.

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

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