On Fri, 16 Oct 2026 17:45:24 GMT, Marius Hanl <[email protected]> wrote:

> This PR fixes a regression from https://github.com/openjdk/jfx/pull/1830.
> 
> My suspicion was that this might be a preexisting after all, which just 
> happens to be much more easily to reproduce with `refresh()`.
> Turns out I was right, it could already happen before, without `refresh` 
> involved.
> So I wrote a second test for that scenario. 
> 
> Both tests (and the bug of course) are fixed when we always clear the index 
> of the cells that will land in the pile and to be removed from the viewport.
> This ensures that we will always update a cell we use from the pile. 
> 
> The reuse of the cells when scrolling a bit is unaffected. 
> Example: When we show 10 cells, scroll a bit down so that the first 3 items 
> are not visible anymore, we will still reuse the cells with index 3 - 9, and 
> update the other 3 cells with their new index 10,11,12. Nothing changed here. 
> If we now change the height so that cell index 12 is not visible anymore, it 
> will land in the pile and will get the index `-1`.
> 
> ---
> 
> Another advantage: Cells in the pile will not have a reference to their item 
> anymore. Cells can be in the pile forever, e.g. because the user change the 
> height of the application.
> 
> This might come in handy for 
> [JDK-8390878](https://bugs.openjdk.org/browse/JDK-8390878) as well.
> 
> ---
> 
> Note: I still decided to also the `resetIndex` call for the pile in my second 
> commit. It is not needed to fix the bug, but it makes sense. When we are 
> rebuilding the `VirtualFlow`, we can and should not trust any cell. It 
> therefore makes sense to reset the index pf pile cells here as well.
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

there seems to be a regression - it's a tricky case.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
 line 2857:

> 2855:             cell.setVisible(false);
> 2856:             if (cell.getIndex() != -1) {
> 2857:                 cell.updateIndex(-1);

we need to check whether the focus has moved elsewhere, for example in an 
`onEDitCancel` handler, otherwise L2870 will steal the focus back from where it 
was moved by the handler.

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

PR Review: https://git.openjdk.org/jfx/pull/2347#pullrequestreview-5552175627
PR Review Comment: https://git.openjdk.org/jfx/pull/2347#discussion_r4298067849

Reply via email to