[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: RFR: 8274061: Tree-/TableRowSkin: misbehavior on switching skin [v2]
From:       Marius Hanl <mhanl () openjdk ! java ! net>
Date:       2021-10-31 18:11:13
Message-ID: trF4x46Cau7N36NoeycWNjnjKX0_UxrbR1C1WCl3qvc=.cf30a225-4210-49cc-bc18-22ad452095a6 () github ! com
[Download RAW message or body]

On Wed, 27 Oct 2021 09:56:46 GMT, Jeanette Winzenburg <fastegal@openjdk.org> wrote:

> > Cleanup of Tree-/TableRowSkin to support switching skins
> > 
> > The misbehavior/s
> > - memory leaks due to manually registered listeners that were not removed
> > - side-effects due to listeners still active on old skin (like NPEs)
> > 
> > Fix
> > - use skin api for all listener registration (for automatic removal in dispose)
> > - do not install listeners that are not needed (fixedCellSize, same as in fix of \
> > ListCellSkin [JDK-8246745](https://bugs.openjdk.java.net/browse/JDK-8246745)) 
> > Added tests for each listener involved in the fix to guarantee it's still working \
> > and does not have unwanted side-effects after switching skins. 
> > Note: there are pecularities in row skins (like not updating themselves on \
> > property changes of its control, throwing NPEs when not added to a VirtualFlow) \
> > which are not part of this issue but covered in \
> > [JDK-8274065](https://bugs.openjdk.java.net/browse/JDK-8274065)
> 
> Jeanette Winzenburg has updated the pull request incrementally with one additional \
> commit since the last revision: 
> re-added forgotten code comments

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java \
line 365:

> 363:                 // Fix for RT-27782: Need to set isDirty to true, rather than \
>                 the
> 364:                 // cheaper updateCells, as otherwise the text indentation will \
>                 not
> 365:                 // be recalculated in TreeTableCellSkin.leftLabelPadding()

Actually this comment is not correct anymore since my PR got merged \
(https://github.com/openjdk/jfx/pull/568). Instead, it should be \
`TreeTableCellSkin.calculateIndentation()`.

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

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


[prev in list] [next in list] [prev in thread] [next in thread]