On Mon, 17 Feb 2025 04:53:41 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
> What part in the PR section? I found this: > > > Added listeners are only called when a new non-nested (top level) > > notification starts Maybe I misunderstood what that section says. When I look at the list of differences it's written from the point of view of the new code: > * Provides correct old/new values to ChangeListeners under all circumstances > * Unnecessary change events are never sent > * Single invalidation or change listeners are inlined directly into the > observable class (in other words, properties with only a single listener > don't take up any extra space at all) > ... > * Added listeners are only called when a new non-nested (top level) > notification starts But the table says "New listeners are never called during the current notification regardless of nesting". This is the mismatch I noticed. I think that that bullet point talks about the old code. > Both `ExpressionHelper` and this implementation will not notify an added > listener within a callback for the same notification. `ExpressionHelper` will > notify the newly added listener though if a _nested_ change occurs before the > original notification completes, while this implementation considers this to > be integral with the first change. This is what I observed, along with the example you give later. I also agree that the only place to add the listener in this implementation would be at the end of the top level in order to avoid the incorrect old value nested notifications. Let's look at 2 cases where a listener is added inside another, one before a nested change happens and one without a nested change. In the case where a nested change happens: * old code: the new listener will receive an incorrect value in the nested level. * current new code: the new listener will not receive a notification. * prospective new code: the new listener will receive a correct value in the top-level. In the case where an addition happens without a nested change: * old code: the new listener will not receive a notification. * current new code: same as above. * prospective new code: the new listener will receive a correct value in the top-level. So in the first case, the value is corrected. I would say that the current code removing the (incorrect) notification of the new listener is a behavioral change. In the second case, the prospective code's new notification is a behavioral change. Looks like we'll have to decide where the divergence is. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1081#issuecomment-2663570744
