On Wed, 27 Oct 2021 16:12:11 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> Ajit Ghaisas has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fix review comments
>
> modules/javafx.controls/src/main/java/javafx/scene/control/CheckMenuItem.java
> line 89:
>
>> 87:
>> **************************************************************************/
>> 88: /**
>> 89: * Constructs a default {@code CheckMenuItem}.
>
> `Label` uses "Creates an empty label" for the default constructor because it
> has no text or graphic. Maybe it's more informative that way.
It does make sense to modify it. I will change it.
> modules/javafx.controls/src/main/java/javafx/scene/control/PopupControl.java
> line 1133:
>
>> 1131: */
>> 1132: public CSSBridge() {
>> 1133: }
>
> Looking at the [docs for
> 17](https://openjfx.io/javadoc/17/javafx.controls/javafx/scene/control/PopupControl.CSSBridge.html),
> the constructor there is `protected`, here it's `public`. Was this changed
> recently? If it's supposed to be `protected`, then the constructor is for
> subclasses.
This is a very good catch. Thanks!
Changing this to public was a mistake this PR was about to make.
In openjfx17, there is no constructor defined - that means - it gets generated
implicitly which is `protected`.
I introduced an explicit constructor in this PR while fixing the "missing
javadoc" comment. Inadvertently I had made it `public` in this PR.
Now, I have made it `protected` in the latest commit.
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualContainerBase.java
> line 67:
>
>> 65:
>> 66: /**
>> 67: * Constructs a {@code VirtualContainerBase}
>
> The class is `abstract`, so possibly the constructor should be `protected`,
> and we might want to say "Constructor for subclasses" anyway.
I have corrected the javadoc to match "Constructor for subclasses to call.".
I would like to avoid changing the access modifier to `protected` as part of
javadoc fix.
-------------
PR: https://git.openjdk.java.net/jfx/pull/646
