RFR: 8359599: Calling refresh() for all virtualized controls recreates all cells instead of refreshing the cells [v3]

Kevin Rushforth kcr at openjdk.org
Fri Oct 10 22:37:46 UTC 2025


On Fri, 10 Oct 2025 20:27:10 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Marius Hanl has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - remove newline from imports
>>  - assert row creation count as well
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableViewSkinBase.java line 305:
> 
>> 303:                 return;
>> 304:             }
>> 305:             if (Properties.RECREATE.equals(c.getKey())) {
> 
> I just have to raise this concern.
> 
> It looks weird to me to use properties as a roundabout way to invoke a hidden method in the skin.  Node properties, a public facility, can be easily mutated by unrelated code, making it easy to break the intended functionality.
> 
> Why not make these two methods explicitly a part of the public API by replacing `requestRebuildCells` and `requestRecreateCells` with
> 
> 
> protected VirtualContainerBase.rebuildCells()
> protected VirtualContainerBase.recreateCells()
> 
> 
> ? 
> 
> (requestXXX is a misnomer - it might suggest an async nature, whereas it these are synchronous methods)

Changing this is out of scope for this PR.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1830#discussion_r2422192916


More information about the openjfx-dev mailing list