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

Andy Goryachev angorya at openjdk.org
Wed Aug 24 15:44:45 UTC 2022


On Wed, 24 Aug 2022 15:36:17 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>>> But: turned out that we currently don't have such a case :)
>> 
>> But we do!  (I this one of my earlier comments did not got lost by Jira, sorry).
>> 
>> In TextInputControlSkin : 334, we have
>> 
>> control.setInputMethodRequests(new ExtendedInputMethodRequests()
>> 
>> this sets a property with a complex object (not a listener).  the constructor cannot distinguish between this property set by the user, or by the still attached skin.  So if the user wants to set a custom input method requests it will get overwritten.
>> 
>> There is no way around it.
>
>>  the constructor cannot distinguish between this property set by the user
> 
> true, but currently it doesn't even _try_ to.
> 
> So IMO that's __not__ a problematic pattern (as already pointed out in our recent debate): the install sets whatever property __unconditionally__ - to safely cleanup behind itself, the old skin will simply check if the current value is the one it installed itself, if not leave it alone (which it currently doesn't but that's a bug that can be solved without new api).

@kleopatra ,
this will fail in the case of changing skins -
0. user sets the property (U)
1. skinA() sets the property (A)
2. skinB() sets the property (B)
3. skinA.dispose() does not reset property to U because it's now == B
4. skinB.dispose() fails to set the property to U because it does not know the initial value

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

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


More information about the openjfx-dev mailing list