RFR: 8290844: Add Skin.install() method [v3]

Andy Goryachev angorya at openjdk.org
Mon Aug 15 15:55:28 UTC 2022


On Mon, 15 Aug 2022 15:37:15 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>>> A quick PoC is in [my fork off Andy's PR](https://github.com/kleopatra/jfx/tree/exp-skin-install-supportsInstall)
>> 
>> @kleopatra : 
>> Thank you so much for your effort to research the alternatives.
>> 
>> The main issue that necessitates creation of install() and moving it outside of the skin's constructor is the fact that certain code just cannot be inside of the skin's constructor - because the old skin is still attached.  So it must be called after the old skin has been removed in setSkin().  I don't think there is a way around it.
>> 
>> Those user-defined skins (and the affected core skins) that modify singletons or rely on internals of the superclass - must be refactored.  The others, which only add listeners and children nodes, can remain unchanged.
>
>> 
>> @kleopatra : Thank you so much for your effort to research the alternatives.
>> 
> 
> thinking about alternatives is our job, isn't it :)
> 
>> The main issue that necessitates creation of install() and moving it outside of the skin's constructor is the fact that certain code just cannot be inside of the skin's constructor - because the old skin is still attached. So it must be called after the old skin has been removed in setSkin(). I don't think there is a way around it.
> 
> There might be such code, but I doubt that there is anything (at least nothing validly installed) in our skins. Haven't looked into the install children that are defined in the control itself (like f.i. editors in the combo/spinner et al).
> 
>> 
>> Those user-defined skins (and the affected core skins) that modify singletons or rely on internals of the superclass - must be refactored. 
> 
> As long as custom classes play by the rules (aka: don't violate the spec) we'll have a hard time imposing code changes onto custom classes. Or if we do, we might see us in the middle of many pointed fingers. Like (from [kinds of compatibilty](https://wiki.openjdk.org/display/csr/Kinds+of+Compatibility))
> 
>> While an end-user may not care why a program does not work with a newer version of a library, what contracts are being followed or broken should determine which party has the onus for fixing the problem

@kleopatra : 

Please take a look at MenubuttonSkin:108
In there, we are setting an onAction handler, but only if it's not null.  This particular code cannot distinguish between onAction handler set by the user prior to any skin installed, and the handler installed by a previous skin in the case of replacing an existing skin.

How do you propose to fix it?

Even if we invent some kind of inner class to be able to do instanceOf, there is still a possibility of replacing a user-defined skin, where the old skin would set and remove its handler and we'll end up without a handler once new skin is installed.

No amount of hacks can fix a bad design, it seems.

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

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


More information about the openjfx-dev mailing list