On Thu, 11 Jan 2024 20:13:09 GMT, Marius Hanl <mh...@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)
@Maran23
This is ready for integration. If you were waiting for me. Thanks, It looks
good, I don't have any comments.
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1331#issuecomment-1921183306
