On Mon, 12 Oct 2026 18:35:31 GMT, Andy Goryachev <[email protected]> wrote:
>> This change might be OK given that it is intentional and you are documenting >> it. Are there cases where a change listener might want to read the new >> position? ChangeListeners handle this by passing in the old value, but I >> realize this isn't the same case. If you've thought this through, and this >> is the semantic behavior you want, then this is fine. >> >> It does highlight an implementation concern: if any listener throws an >> Exception, the marker position will not be updated. This means that an >> application bug or other problem in a listener could lead to stale markers, >> which I don't think is desirable. >> >> A preexisting problem is that an exception in one listener will prevent >> subsequent listeners from running. >> >> Both of these issues could be handled in a follow-up. > > this is another good point: what should be done if a listener throws a > `RuntimeException`? An `Exception`? An `Error`? > Should the caller just eat any `Throwable` silently? Log a stack trace? > > edit: we don't handle exceptions thrown during event handling, so why here? > if the handler throws an exception, we can assume the whole thing is broken > at this point. > > we could say "the listener is expected not to throw" in the spec, but then > again - there is nothing like that in the `EventHandler` or similar. There are two different, but related, issues to consider: 1. Should we maintaining consistent internal state when a listener or other callback throws an Exception? 2. Should we isolate Exceptions thrown by a listener such that subsequent listeners continue to run I'd say the first is a requirement, especially in a case like this where we intentionally delay some of the processing until after listeners have been notified. Whether that is via a try / finally or something else depends on the answer to 2. This might be better to address in this PR, since it is otherwise a regression, but could be done in a follow-up if you prefer. For the second question, this is more like a TextInputControl change listener -- informing caller(s) of a change that has happened -- than it is like an event handler or filter. The former uses ObservableValue's ChangeListener, which does catch Exception (but not Error) which ensures that subsequent listeners are called when a listener throws an Exception. I think this would be a good pattern to follow. I recommend filing a follow-up issue to address these issues. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2224#discussion_r4264381145
