• Type: Bug
• Component: javafx
• Sub-Component: base
• Affected Version: 8,9

• Priority: P3
• Status: Resolved
• Resolution: Fixed

• Submitted: 2016-08-08
• Updated: 2016-08-15
• Resolved: 2016-08-15

JDK 9
9Fixed

Changeset: 382bd3f49907 Author: ckyang Date: 2016-08-15 14:01 -0700 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/382bd3f49907 15-08-2016 Looks good. +1 15-08-2016 Got it! Here is the revised webrev.02 and the delta between 01 and 02: http://cr.openjdk.java.net/~ckyang/JDK-8163383/webrev.02/ http://cr.openjdk.java.net/~ckyang/JDK-8163383/webrev-01_02.diff/ 13-08-2016 I only see three remaining issues: Bindings.java ------------- There are still many instances of: + * @throws IllegalArgumentException if (@code index &lt; 0} where the last character should be ')' not '}'. You only fixed one of them in webrev.01. ArrayChangeListener.java ------------------------ 36 * @param observableArray the the array that changed Typo: 'the the' StringConverter.java -------------------- 38 * @param object of the {@code T} to convert That isn't what I meant to suggest. Now it has the problem that the description starts with 'of'. Remember that the parameter itself is in bold on the left followed by a dash in the javadoc, so isn't part of the sentence. I meant something more like this: * @param object the object of type {@code T} to convert 12-08-2016 Thanks for the feedback. Here is the revised webrev.01: http://cr.openjdk.java.net/~ckyang/JDK-8163383/webrev.01/ 12-08-2016 Overall, this looks very good. I confirm that there are no more doclint errors or warnings in javafx.base. Here are a few specific comments: Bindings.java ------------- 318 * @param <T> the type of the wrapped {@code Object} This method binds an object, rather than wrapping it, so it would be more correct to say: * @param <T> the type of the bound {@code Object} Also, I noticed the following which was there previously, but which you might fix since you are touching those lines anyway: + * @throws IllegalArgumentException if (@code index &lt; 0} The last character should be ')' not '}'. This appears in many places in this file. MapExpression.java ------------------ 102 * @param <K> the type of the wrapped {@code Object} 103 * @param <V> the type of the wrapped {@code ObjectBinding} Better to just copy from the class header: * @param <K> the type of the key elements * @param <V> the type of the value elements NumberExpression.java --------------------- 866 * @param locale to be used That doesn't read well since it shows up like this in the docs: local - to be used Perhaps something like this: * @param locale the Locale to be used ObjectExpression.java --------------------- 210 * @param locale to be used same comment as in NumberExpression When.java --------- 864 * @param <T> the type of the wrapped {@code Object} ... 880 * @param <T> the type of the wrapped {@code Object} These methods to not wrap any objects, so this is wrong. ObservableArray.java -------------------- 36 * @param observableArray the {@code ObservableArray} This wouldbe more consistent if you said: * @param observableArray the array that changed ObservableArray.java -------------------- 76 * @param capacity the {@code capacity} of this array The second capacity should not be included in {@code ... } since it is referring to the English word and not the parameter name. ObservableListBase.java ----------------------- 238 * @return has a listener for this list That's awkward. Maybe something like this: * @return true if there is a listener for this list StringConverter.java -------------------- There is no '@param <T>' in the class header. I guess doclint doesn't check for this? 38 * @param object the the {@code T} to convert Typo: 'the the' Also, it would read better to say 'object of type {@code T}' DateTimeStringConverter.java ---------------------------- 200 * @return A {@code DateFormat} instance for formatting and parsing in this Generally we use lower case after the '@return' or '@param name' FormatStringConverter.java -------------------------- same comment as above NumberStringConverter.java -------------------------- same comment as above Several packages are included that are not part of javafx.base. They should be excluded from this patch since there are other bugs filed for these modules. modules/javafx.controls/src/main/java/javafx/scene/chart modules/javafx.controls/src/main/java/javafx/scene/control modules/javafx.controls/src/main/java/javafx/scene/control/cell modules/javafx.fxml/src/main/java/javafx/fxml modules/javafx.graphics/src/main/java/javafx/application modules/javafx.graphics/src/main/java/javafx/concurrent modules/javafx.graphics/src/main/java/javafx/css modules/javafx.graphics/src/main/java/javafx/css/converter modules/javafx.graphics/src/main/java/javafx/geometry modules/javafx.graphics/src/main/java/javafx/scene modules/javafx.graphics/src/main/java/javafx/scene/effect modules/javafx.graphics/src/main/java/javafx/scene/image modules/javafx.graphics/src/main/java/javafx/scene/input modules/javafx.graphics/src/main/java/javafx/scene/layout modules/javafx.graphics/src/main/java/javafx/scene/paint modules/javafx.media/src/main/java/javafx/scene/media 11-08-2016 Please review this proposed fix: http://cr.openjdk.java.net/~ckyang/JDK-8163383/webrev.00/ 09-08-2016