On Tue, 4 Oct 2022 15:42:14 GMT, Jeanette Winzenburg <[email protected]> 
wrote:

>> Andy Goryachev has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains 20 additional 
>> commits since the last revision:
>> 
>>  - Merge branch 'openjdk:master' into 8290844.skin.install
>>  - 8290844: unit tests
>>  - Merge remote-tracking branch 'origin/master' into 8290844.skin.install
>>  - 8290844: review comments
>>  - 8290844: review comments
>>  - Merge remote-tracking branch 'origin/master' into 8290844.skin.install
>>  - 8290844: review comments
>>  - 8290844: review comments
>>  - Merge remote-tracking branch 'origin/master' into 8290844.skin.install
>>  - 8290844: javadoc
>>  - ... and 10 more: https://git.openjdk.org/jfx/compare/d49e0565...d954aafc
>
> modules/javafx.controls/src/main/java/javafx/scene/control/Control.java line 
> 226:
> 
>> 224:      * <p>
>> 225:      * To ensure a one-to-one relationship between a {@code Control} 
>> and its {@code Skin},
>> 226:      * {@link Control#setSkin(Skin)} will check the return value of 
>> {@link Skin#getSkinnable()}
> 
> strictly speaking, it's not the method setSkin but the property skin ..

so maybe `setting the {@link #skinProperty() skin property}`?

> modules/javafx.controls/src/main/java/javafx/scene/control/Control.java line 
> 233:
> 
>> 231:      * @return the skin property for this control
>> 232:      * @throws IllegalArgumentException if {@code skin != null && skin 
>> != getSkinnable()}
>> 233:      */
> 
> hmm ..  shouldn't the condition be `(skin != null && skin.getSkinnable() != 
> Control.this)`?

Oops. My mistake (since Andy copied what I gave him), and good catch. I think 
the `Control.` is not needed here, since this is the documentation of a 
property in Control, so maybe just:

`(skin != null && skin.getSkinnable() != this)`?

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

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

Reply via email to