On Wed, 9 Feb 2022 14:04:55 GMT, Jeanette Winzenburg <faste...@openjdk.org> wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java >> line 341: >> >>> 339: setFocusModel(new TreeViewFocusModel<T>(this)); >>> 340: >>> 341: setOnEditCommit(DEFAULT_EDIT_COMMIT_HANDLER); >> >> You are adding the default edit commit handler to TreeView. Although it >> seems to be correct, this default handler is likely to be overwritten if the >> user code already has a setOnEditCommit() call. This is shown by the >> test_rt_29650() failure in TreeviewTest.java which you have corrected. >> >> The changes done in this PR makes TreeView similar to ListView and TableView >> in terms of having the default edit commit handler. >> >> Unfortunately, I do not see how can we avoid user code accidentally >> overwriting the default edit commit handler. Documenting this possibility >> seems to be the only way ahead. >> Any other idea? > > Yes, the change might break application code, though that code would be > buggy. Actually, the behavior as implemented now, _already is_ documented :) > >> It is very important to note that if you call >> setOnEditCommit(javafx.event.EventHandler) with your own EventHandler, then >> you will be removing the default handler. Unless you then handle the >> writeback to the property (or the relevant data source), nothing will >> happen. > > so: if they have a custom handler that doesn't save the data, they were > violating the specification (though were getting away with it due to the bug). > > Personally, I think that we cannot keep backward compatibility to bugs. Thanks for the explanation. I see that it is already well documented. ------------- PR: https://git.openjdk.java.net/jfx/pull/724
