RFR: JDK-8287524: Improve checkboxes to select releases on deprecated API page [v2]

Jonathan Gibbons jjg at openjdk.java.net
Wed Jun 1 18:18:38 UTC 2022


On Wed, 1 Jun 2022 14:55:57 GMT, Hannes Wallnöfer <hannesw at openjdk.org> wrote:

>> The original purpose of this issue was to fix the problems of elements on the deprecated API list that can't be deselected because their release is not included in the list of checkboxes at the top of the page. This is fixed here by adding a checkbox for all elements/releases not included in the list if necessary. 
>> 
>> While working on it I discovered a few corner cases that produce undesirable output for the deprecated list. This includes the case of a single release (where a checkbox doesn't make any sense) and the case of some or all releases not having any occurrences in the `since` element of deprecated elements (in which case the checkbox is useless as well). A more detailed description of these cases is available in the [JBS issue](https://bugs.openjdk.java.net/browse/JDK-8287524).
>> 
>> As examples for the output generated with this change I uploaded JDK API docs:
>> 
>> http://cr.openjdk.java.net/~hannesw/8287524/api.00/deprecated-list.html
>> 
>> To illustrate behaviour in corner cases mentioned above I also uploaded deprecated pages generated by the `TestNewAPIListWriter` test (which despite its name also tests the "extended" deprecated list).
>> 
>> http://cr.openjdk.java.net/~hannesw/8287524/test.00/TestNewApiList/out-multi/deprecated-list.html
>> http://cr.openjdk.java.net/~hannesw/8287524/test.00/TestNewApiList/out-none/deprecated-list.html
>> http://cr.openjdk.java.net/~hannesw/8287524/test.00/TestNewApiList/out-package/deprecated-list.html
>> http://cr.openjdk.java.net/~hannesw/8287524/test.00/TestNewApiList/out-single/deprecated-list.html
>
> Hannes Wallnöfer has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add comment

Some questions inline.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/DeprecatedListWriter.java line 88:

> 86:             for (int i = 0; i < releases.size(); i++) {
> 87:                 String release = releases.get(i);
> 88:                 String releaseIndex = release.isEmpty() ? "" : Integer.toString(i + 1);

See following comments about the questionable? use of an empty string

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/DeprecatedListWriter.java line 92:

> 90:                         ? contents.getContent("doclet.Deprecated_API_Checkbox_Other_Releases")
> 91:                         : Text.of(release);
> 92:                 HtmlId htmlId = HtmlId.of("release-" + releaseIndex);

Note that `releaseIndex` may be an empty string.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/DeprecatedListWriter.java line 97:

> 95:                                         .put(HtmlAttr.CHECKED, "")
> 96:                                         .put(HtmlAttr.ONCLICK,
> 97:                                                 "toggleGlobal(this, '" + releaseIndex + "', 3)"))

Note that `releaseIndex` may be an empty string

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SummaryListWriter.java line 136:

> 134:         bodyContents.addMainContent(content);
> 135:         // The script below enables checkboxes in the page and invokes their click handler
> 136:         // to restore any previous state, which unfortunately doesn't work on all browsers.

This comment looks worrying.
It might be worth identifying good or bad browsers, either here or in public-facing documentation.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/DeprecatedAPIListBuilder.java line 63:

> 61:             if (!releases.containsAll(foundReleases)) {
> 62:                 // Empty string is added for other releases, including the default value ""
> 63:                 releases.add("");

Is this instance of an empty string related to preceding uses of empty string, noted earlier in these review comments?

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

PR: https://git.openjdk.java.net/jdk/pull/8973


More information about the javadoc-dev mailing list