On Wed, 14 Apr 2021 11:58:14 GMT, Florian Kirmaier <fkirma...@openjdk.org> wrote:
>> Fixing ListCell editing status is true, when index changes while editing. > > Florian Kirmaier has updated the pull request incrementally with one > additional commit since the last revision: > > 8264127: > Added checks, whether the correction ammount of editStart/cancel events are > triggered - there's still a failing transition (and not covered by the latest test ;) `cellIndex == editingIndex` to `cellIndex == -1` - as to your fix: don't see a difference between calling `updateEditing `before/after `updateFocus` - simply moving it after the if block seems to solve all toEditing issues That said: I'm aware that getting this right is tedious and a bit hard, not because it's rocket science but because we need to cover the corner cases as completely as possible (and often fail in doing so, me included :) Chances are that if we detect a problem with a transition "-1 -> 1", there's a similar problem in the invers "1 -> -1". Even if there isn't, we should make sure there isn't by adding a test. That's why I personally prefer writing dedicated parameterized tests (vs. adding to the existing general purpose test), here in cell/editingIndex. Don't know if you noticed, yesterday I attached such a test to the issue (had to write it for [JDK-8265206](https://bugs.openjdk.java.net/browse/JDK-8265206) anyway, so no big deal to replace TableCell with ListCell) - whether you take it or adjust yours accordingly doesn't matter, as long as the coverage is complete. BTW, in TableCell the remaining issue (editingIndex -> -1) seems to be related to updateEditing backing out if the current index is -1. It probably should cleanup if in editing state (didn't try yet). ------------- PR: https://git.openjdk.java.net/jfx/pull/441
