[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-openjfx-dev
Subject: Re: RFR: 8276056: Control.skin.setSkin(Skin) fails to call dispose() on discarded Skin
From: Kevin Rushforth <kcr () openjdk ! org>
Date: 2022-06-30 20:17:01
Message-ID: bWadRYIkKWFfqwdNLXfGrK-OEhqP_LLquFZvn6ReWdo=.511ef6cc-1c99-4eb0-ba03-79f27acc6675 () github ! com
[Download RAW message or body]
On Wed, 29 Jun 2022 13:35:15 GMT, Marius Hanl <mhanl@openjdk.org> wrote:
> For some reason the `skinProperty` did not allow to set a new skin which is the \
> same class as the previous one. This leads to multiple issues:
> 1. When creating a new skin (same class as previous), the skin will likely install \
> some children and listener but is then rejected when setting it due to the \
> `skinProperty` class constraint
> -> Control is in a weird state as the current skin was not disposed since it is \
> still set, although we already created and 'set' a new skin 2. A skin of the same \
> class can behave differently, so it is not really valid to reject a skin just \
> because it is the same class as the previous
> -> Just imagine we have the following skin class
>
> class MyCustomButtonSkin extends ButtonSkin {
> public MyCustomButtonSkin(Button button, boolean someFlag) { super(button); ... }
> }
>
> Now if we would change the skin of the Button two times like this:
>
> button.setSkin(new MyCustomButtonSkin(button, true));
> button.setSkin(new MyCustomButtonSkin(button, false));
>
> The second time the skin will be rejected as it is the same class, although I may \
> changed the skin behaviour, in this case demonstrated by a boolean flag for showing \
> purposes.
This looks simple enough that a single Reviewer should be sufficient. The remove code \
looks quite questionable to me, so getting rid of it seems like a good thing.
-------------
PR: https://git.openjdk.org/jfx/pull/806
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic