RFR: 8187309: TreeCell must not change tree's data

[Ajit Ghaisas]

On Wed, 9 Feb 2022 14:04:55 GMT, Jeanette Winzenburg <fastegal at 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