[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-openjfx-dev
Subject: Re: RFR: JDK-8323615: PopupControl.skin.setSkin(Skin) fails to call dispose() on discarded Skin
From: Michael =?UTF-8?B?U3RyYXXDnw==?= <mstrauss () openjdk ! org>
Date: 2024-01-30 5:38:41
Message-ID: i5DKsY_M0GpyJ_a_dtpYx3TANMfYTDiHL0p9VGJMPTA=.2bcc3730-f7eb-4f7c-adca-5d4eacbbf0ce () github ! com
[Download RAW message or body]
On Thu, 11 Jan 2024 20:13:09 GMT, Marius Hanl <mhanl@openjdk.org> wrote:
> For some reason the `skinProperty` did not allow to set a new skin when it 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 \
> listener but is then rejected when setting it due to the `skinProperty` class \
> constraint
> -> `PopupControl` 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 MyCustomSkin<C extends PopupControl> extends Skin<C> {
> public MyCustomSkin(C skinnable, boolean someFlag) { ... }
> }
>
> Now if we would change the skin of the `PopupControl` two times like this:
>
> popup.setSkin(new MyCustomSkin(popup, true));
> popup.setSkin(new MyCustomSkin(popup, 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 is the same issue and fix as done for `Control` in \
> [JDK-8276056](https://bugs.openjdk.org/browse/JDK-8276056) (PR: \
> https://github.com/openjdk/jfx/pull/806)
Looks good, and works as I expect it.
-------------
Marked as reviewed by mstrauss (Committer).
PR Review: https://git.openjdk.org/jfx/pull/1331#pullrequestreview-1850261757
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic