JDK-8221377 : Fix mistakes in FX API docs
  • Type: Bug
  • Component: javafx
  • Sub-Component: other
  • Affected Version: openjfx12
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2019-03-25
  • Updated: 2019-08-13
  • Resolved: 2019-07-27
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
openjfx13Fixed
Related Reports
Duplicate :  
JDK-8227655 - Incorrectly documented @defaultValue for Animation.cycleCount
Relates :  
JDK-8227765 - {@inheritDoc} does not work for methods inherited from the JDK
Relates :  
JDK-8227764 - Missing docs for getClassCssMetaData() and getControlCssMetaData()
Relates :  
JDK-8227767 - Generated HTML JavaDoc should have the same font as the JDK docs
Relates :  
JDK-8229472 - Deprecate for removal JavaBeanXxxPropertyBuilders constructors
Description
Issue for collecting API fixes for OpenJFX13:

1. TableColumn#cellFactoryProperty: "editing.There" missing space (Fixed)

2. Scene: "If a resizable node (layout Region or Control is set as the root, then" missing ')' (Fixed)

3. Stylesheet is missing some methods documentation (missed in https://github.com/javafxports/openjdk-jfx/pull/147 presumably).

4. getClassCssMetaData() and getControlCssMetaData() are missing docs in all (or almost all) of the classes. (Deferred to JDK-8227764)

5. Border and BorderStroke have missing method descriptions. This is because {@inheritDoc} does not work for methods inherited from the JDK since its source is not specified. Probably more classes are affected. (Deferred to JDK-8227765)

6. JavaBeanXxxPropertyBuilder: "you can reuse a JavaBeanXxxPropertyBuilder. by switching" unneeded '.' (Fixed)

7. JavaBeanXxxPropertyBuilder#bean, #name and #create methods are missing a period. (Fixed)

8. Border: "getInsets() are used" should be "insets". (Fixed + other corrections)

9. Border#getInsets: "The values in these outsets" should be "insets". (Fixed)

10. TransformationList class doc is missing a period at the end. (Fixed)

11. StackPane: code example has missing quotes: `new Label("Go!)` -> `new Label("Go!")` (GitHub #471) (Fixed)

12. TextFlow class doc:
    a. "plus it own width" -> "plus its own width"
    b. "due to wrapping and the visual location of Text node can differ" -> "due to wrapping, and the visual location of the Text node can differ"
    c. "Any other Node, rather than Text, will be treated as embedded object" -> "Any Node, other than Text, will be treated as an embedded object"
    d. "inside of a TextFlow some its properties" -> "inside of a TextFlow, some of its properties"
(Fixed all)

13. Text class:
    a. missing method docs for: caretBias, caretPosition, caretShape, selectionEnd, selectionShape and selectionStart. These properties are delegates from getTextAttribute() so the JavaDoc tool does not generate them automatically. (Fixed)
    b. wrappingWidth's first sentence description is malformed. (Fixed)
    c. fontSmoothingType has a space before ':'. (Fixed)

14. ChangeListener#change has a bad first sentence. Possibly, the 1st and 2nd sentences should be switched. (Fixed)

15. Animation#currentRateProperty: "currentRate may also point to different direction during reverse cycles when autoReverse is true" should be "a different" and a period. (Fixed)

16. Color is missing a period in its constructor doc. (Fixed)

17. Labeled#textAlignmentProperty(): "when text is multiline Unlike" missing period after "multiline". (Fixed)

18. DisplacementMap class doc has "&nbsp" strings and the code example has extra empty lines. (Fixed)

19. JDK-8227655 (Fixed)

20. Image and WriteableImage have constructors with "Construct" instead of "Constructs". (https://github.com/javafxports/openjdk-jfx/pull/472#discussion_r303857943) (Fixed)

21. PixelFormat<T> is missing an @param description tag for T (https://github.com/javafxports/openjdk-jfx/pull/472#discussion_r303583158) (Fixed)

22. Node#blenMode: "is treated as pass-though this means" missing comma or period (Fixed)

23. JavaBeanXxxPropertyBuilders use "Create" and "Set" instead of "Creates" and "Sets". There are also unneeded commas. (Fixed)

24. ObservableValue class doc: "They do not generate anymore invalidation events until...": anymore --> any more, or move "anymore" after "events" (Fixed)
Comments
Changeset: 8db7b189c393 Author: nlisker Date: 2019-07-27 13:26:29 +0300 URL: https://hg.openjdk.java.net/openjfx/13-dev/rt/rev/8db7b189c393 8221377: Fix mistakes in FX API docs Reviewed-by: kcr, arapte
27-07-2019
Hi Nir, here are few suggestions: ## JavaBeanBooleanPropertyBuilder.java ## Sets the Java Bean instance the adapter should connect to. => Sets the Java Bean instance, to which the adapter should connect. OR => Sets the Java Bean instance for the adapter to connect. Similar change for the other PropertyBuilder classes. ## Animation.java ## 1. {@code currentRate} may also point to a different direction during reverse cycles when {@code autoReverse} is {@code true}. => {@code currentRate} may also point in the opposite direction of {@code rate} during reverse cycles when {@code autoReverse} is {@code true}. The direction of rate is either forward(+) or backward(-). The direction of currentRate would be same as or opposite to rate's direction. So 'opposite' would sound more specific behavior than 'different'. 2. {@code currentRate} is not necessary equal to {@code rate}. => {@code currentRate} is not necessarily equal to {@code rate}. ## DisplacementMap.java ## {@code dst[x,y] = src[(x,y) + (offset+scale*map[x,y])*(srcw,srch)]} => Can add spaces with the comma and arithmetic operators. ## Color.java ## Creates a new instance of color. => Creates a new instance of {@code Color}. ## Text.java ## @return the {@code selectionShapeProperty} property => @return the {@code selectionShape} property selectionShape is defined in the private class TextAttribute, and similar change for the other properties.
27-07-2019
Looks good to me +1
27-07-2019
I have no problem if you want to keep "Sets the Java Bean instance the adapter should connect to.", although I do understand the reason behind the suggestion: it is to avoid ending a sentence with a preposition. FWIW, I usually try to avoid that, but do not consider it a hard-and-fast rule. The .01 webrev looks good to me. +1
26-07-2019
Thanks for the suggestions, Ambarish. I'm not sure about the rephrasing of "Sets the Java Bean instance the adapter should connect to." I don't see the benefit in the alternatives. The other suggestions I implemented. Note that the changes in Text don't matter because the return statement isn't generated because of the special treatment for properties (though I changed it anyway). The new webrev includes these changes and #24: http://cr.openjdk.java.net/~nlisker/8221377/webrev.01/
26-07-2019
The additional change above looks fine. +1
25-07-2019
I fixed #24 yesterday and didn't upload a new webrev. The changes are: @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2019, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -54,7 +54,7 @@ * Implementations of this class should strive to generate as few events as * possible to avoid wasting too much time in event handlers. Implementations in * this library mark themselves as invalid when the first invalidation event - * occurs. They do not generate anymore invalidation events until their value is + * occurs. They do not generate any more invalidation events until their value is * recomputed and valid again. * <p> * Two types of listeners can be attached to an {@code ObservableValue}:
24-07-2019
Note: when ready, this can go directly into 13-dev/rt (it will be synced from there to jfx-dev, so no need to push it to both places).
24-07-2019
PixelFormat: + * @param <T> the type of buffer that stores the pixel data. Only {@code ByteBuffer} and {@code IntBuffer} are used. I had originally thought to suggest changing this to match PixelBuffer, but I think that "used" is better than "supported" in this case, since it is the factory methods of PixelFormat that determine what is used. So I think the .00 webrev is fine as-is. This could use a second pair of eyes. Given where most of the changes are, I would ask [~arapte] to review. +1
24-07-2019
http://cr.openjdk.java.net/~nlisker/8221377/webrev.00/
20-07-2019
The FlowPane issue is a functional bug, not a doc issue. I filed JDK-8228453 to track it.
20-07-2019
3. A comment sounds good. I think we can still deprecate for removal in 14 and remove in 15. I also still need to look at the Flowpane issue: https://github.com/javafxports/openjdk-jfx/issues/348 I'll review next week when you send it out.
19-07-2019
1. Yeah, this doesn't appear to be simple. However, I think it can be fixed outside of the build. There are missing files (or wrongly placed files) in the generated docs, so maybe it could be fixed on the hosting server. 3. Didn't know it needed CSR, so let's not deal with it. I can add a comment on the constructors saying not to use them if that helps. 4. I did that and apparently no return statement is generated anyway, but the warning is gone. I'm ready to put this on the review queue.
18-07-2019
To answer your specific questions: 1. Thanks for filing the follow-up bugs. Unless JDK-8227767 is a a simple matter of adding flag (which I suspect it isn't), then I recommend to also target that one to openjfx14. 2. Completely forgot about that one. I'll take a look, but it won't be today. 3. Deprecate in 13 would be OK, but it will need a separate JBS issue with a CSR to deprecate them, and it's getting pretty late for that (but not out of the question). Regardless I think for-removal in 14 and remove in 15 is good -- removal in 14 is too early. 4. If the docs go on the xxxxProperty() method, which is a fine thing to do, you could add something like the following to ignore the warning: * @return the xxxx property
16-07-2019
Summary of the current situation: 1. I created 3 linked issues: JDK-8227765 and JDK-8227764 are targeted at 14; JDK-8227767 is targeted at 13. I don't know how to fix these right now. 2. There's still https://github.com/javafxports/openjdk-jfx/issues/348 to check if you want it fixed in 13. 3. JavaBeanXxxPropertyBuilders default public constructors can already be deprecate in 13 (even not for removal), then switch to for removal in 14 and remove in 15. That is if 14 is too early for the actual removal. 4. Fixing #13.a causes the javadoc task to spit warnings about missing @return statements when I put the docs on the property getter. Usually the docs go on the field, but the fields don't exist in the class. The getter and setter docs are generated correctly. Not sure what can be done about this.
16-07-2019
> I noticed that JavaBeanXxxPropertyBuilders have default public constructors. Ouch. We tried to avoid this, making them explicit where possible, but obviously missed these. > They should probably be explicitly private, but that could break backwards compatibility if anyone used them for some reason. This would indeed be an incompatible change. > Then again, it is documented to create an instance with: "To create a JavaBeanBooleanProperty one first has to call create() to generate a builder". Maybe deprecate them first? If we decide to go this route -- as opposed to just adding explicit constructors but recommending not to use them -- then yes, this would be the way to go: deprecate for removal in 14 and remove in, say, 15.
16-07-2019
I noticed that JavaBeanXxxPropertyBuilders have default public constructors. They should probably be explicitly private, but that could break backwards compatibility if anyone used them for some reason. Then again, it is documented to create an instance with: "To create a JavaBeanBooleanProperty one first has to call create() to generate a builder". Maybe deprecate them first?
16-07-2019
I would leave #4 for another time then. Ditto for the issues relating to inheritDoc from JDK classes. Those seem broader changes than we would want to address as part of fixing a number of typos and other doc cleanup.
15-07-2019
Considering the amount of methods with missing docs, it might be worth to write a script that searches the generated doc files and finds members without documentation, for whatever reason. I can't fix all the css methods' docs manually (#4).
15-07-2019
[~kcr] While trying to fix #5 I found that {@inheritDoc} does not generate anything if the overridden method is not in OpenJFX. That means that any overridden hashCode() with {@inheritDoc} is affected. After searching a bit, it seems that we need to point to the JDK source somewhere in the JavaDoc generation task.
03-07-2019
Reminder to check https://github.com/javafxports/openjdk-jfx/issues/348
14-06-2019