On Fri, 9 Apr 2021 10:40:19 GMT, Florian Kirmaier <fkirma...@openjdk.org> wrote:

> 
> 
> I've looked into it.
> This is somehow linked to the fact, that the default focusIndex for the list 
> is 0 instead of -1.
> If you change the updateDefaultFocus methods in ListView, then it works.
> 

good dig :) Wouldn't have expected the focus state to have any effect .. so we 
should make certain it's part of the test setup, f.i. by always setting the 
focusedIndex (of the model) to -1 (or whatever is required to make all pass) 
always and document why it's done.

> The tests are somehow synthetic and don't reflect the real world.
 
.. true, nevertheless they souldn't fail/pass on some magic number ;)

And yet another failing test, when changing cell index from -1 -> 
listEditingIndex:

    @Test
    public void testChangeCellIndexMinusOneToListEditingIndex0() {
        assertChangeIndexToEditing(-1, 1);
    }

This looks not fixable by pre-setting focusIndex ..

Coming back to our disagreement about event firing: I'm still weary with those 
intermediate events from simple re-use - but found a precedence in 
[TableCell](https://bugs.openjdk.java.net/browse/JDK-8150525): the same broken 
cell editing state led to data corruption and was fixed by calling 
updateEditing (including firing). So doing the same here looks okay after all. 
Should be documented though, at least by tests (for cancel, B in my very first 
comment with flipped assertion, similar for start edit).

-------------

PR: https://git.openjdk.java.net/jfx/pull/441

Reply via email to