Integrated: JDK-8325402: TreeTableRow updateItem() does not check item with isItemChanged(..) unlike all other cell implementations

Marius Hanl mhanl at openjdk.org
Sat Mar 9 16:01:58 UTC 2024


On Thu, 8 Feb 2024 18:55:54 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

> `TreeTableRow` does not check the item with `isItemChanged(..)`, unlike all other implementations of the cell.
> 
> This also means that the `TreeTableRow` always updates the item, although it should not, resulting in a performance loss as a `TreeTableRow` will always call `updateItem(..)`.
> 
> It looks like that this was forgotten in [JDK-8092593](https://bugs.openjdk.org/browse/JDK-8092593).
> 
> Checking the whole history, it looks like the following was happening:
> 1. There was a check if the item is the same in all cell implementations (with `.equals(..)`)
> 2. Check was removed as it caused bugs
> 3. Check was readded, but instead we first check the index (same index) and then if we also have the same item (this time with `.isItemChanged(..)`, to allow developers to subclass it)
> 4. Forgotten for `TreeTableRow` inbetween this chaos.
> 
> Added many tests for the case where an item should be changed and where it should not.
> Improved existing tests as well. Using `stageLoader`, as before the tests only created a stage when doing the assert.
> 
> NOTE: The update logic is now the same as for all other 5 cell implementations. Especially `TreeCell` (since it is for a tree structure as well).

This pull request has now been integrated.

Changeset: 11305843
Author:    Marius Hanl <mhanl at openjdk.org>
URL:       https://git.openjdk.org/jfx/commit/11305843300cdbfed9bfa2120da1c7ecd361df39
Stats:     539 lines in 11 files changed: 511 ins; 2 del; 26 mod

8325402: TreeTableRow updateItem() does not check item with isItemChanged(..) unlike all other cell implementations

Reviewed-by: angorya, kpk

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

PR: https://git.openjdk.org/jfx/pull/1360


More information about the openjfx-dev mailing list