On Mon, 17 Feb 2025 01:58:34 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> John Hendrikx has updated the pull request incrementally with five >> additional commits since the last revision: >> >> - Clean-up and add tests >> - Pass in listener data directly for fireValueChanged calls >> - Fix documentation in a few places >> - Remove unused interface >> - Update copyrights and add missing > > I'd say that the biggest surprise a user would have is about the nested > addition of listeners. By the way, your table says that added listeners don't > receive any event, but the "The PR" section says that they do. I've observed > that they don't. > I think that not receiving events for added listeners is confusing (we > discussed this briefly under https://github.com/openjdk/jfx/pull/837). Is > there a good reason for this? It prevents SO exceptions in some cases, but I > would say that that's the user's fault. I need to think about this more, but > was wondering what you concluded. @nlisker I found the problem, it was a slight ordering problem in the "optimized" code in `ListenerList`. It is supposed to obtain the current value from the observable in two cases; when it is the first time in the loop, or when a nested notification just completed. However, if the first listener was `null`d, then it skips the first iteration of the loop, and the logic that was supposed to get the current value on the first iteration is also skipped. The current value variable was then left `null`. I've added a regression test to guard for this and some explanatory comments. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1081#issuecomment-2663354032
