On Mon, 14 Sep 2026 19:18:58 GMT, Andy Goryachev <[email protected]> wrote:
>> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> bulk remove > > modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java > line 2855: > >> 2853: wasFocusOwner = wasFocusOwner || >> doesCellContainFocus(cell); >> 2854: cell.setVisible(false); >> 2855: > > there might be a problem: > - cell.setVisible(false) invokes a listener that let's say, scrolls and > removes the cell from pile, then reuses that cell for a different index > synchronously. > - the said cell is active, but later code removes it in L2861 > > the old code removed the pile after all the callbacks were done in L2855 Well, people can also install a listener on the `sceneProperty()` or `parentProperty()` of the cell and do weird things. So this seems to be a highly hypothetical scenario. With that said, I plan to remove the `setVisible` in a follow up. After https://github.com/openjdk/jfx/pull/1644, it serves no purpose anymore. See also `NOTE2` there, quoting: The fix is to remove those cells from the viewport when not needed anymore. ... NOTE2: This makes the logic to make cells invisible and visible again obsolete. This was there so those unused cells laying around in the VirtualFlow does not receive any Mouse or KeyEvents. This can be cleaned up in a follow up PR Since then it is on my list (like this PR), but I'll do that as soon as I have some more time again (so, pretty soon). Aside from cleaning things up (and resolving your hypothetical scenario), that should also improve performance a little! ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2308#discussion_r4009030942
