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