OpenJDK › OpenJDK JavaDoc Development

RFR: JDK-8261499: Simplify HTML for javadoc links

Classic

List

Threaded

4 messages Options

Hannes Wallnöfer

RFR: JDK-8261499: Simplify HTML for javadoc links

This change gets rid of `` elements used in conjunction with HTML links whose only purpose is to apply a CSS class to the link. Instead, the CSS class attribute is applied directly to the link's `<a href="...">` element.

There are three CSS styles for links previously used with embedded or embedding `` elements:

- `type-name-link`, which was applied to a `` element inside the link. This was generated by passing `true` as argument for the `strong` parameter in various get*Link methods.
- `member-name-link` and `search-tag-link`, which were applied to a `` element wrapping the link.

I could find no visual changes in the generated documentation with one exception:

[Class and interface links in the second column of "Uses" page tables][1] were previously wrapped in a `` element causing the whole class name (including non-linked parts such as type parameters) to be displayed with a bold font. With this change, the `type-name-link` style is applied only to the active link, leaving non-linked parts such as type parameters with normal font-weight.

[1]: https://download.java.net/java/early_access/jdk17/docs/api/java.base/java/util/class-use/Map.html#java.util

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

Commit messages:
- JDK-8261499: Include search-tag-link
- JDK-8261499: Missing space
- JDK-8261499: Simplify HTML for javadoc links

Changes: https://git.openjdk.java.net/jdk/pull/2516/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2516&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8261499
Stats: 714 lines in 40 files changed: 30 ins; 44 del; 640 mod
Patch: https://git.openjdk.java.net/jdk/pull/2516.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/2516/head:pull/2516

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

Jonathan Gibbons-2

Re: RFR: JDK-8261499: Simplify HTML for javadoc links

On Wed, 10 Feb 2021 18:20:02 GMT, Hannes Wallnöfer <[hidden email]> wrote:

> This change gets rid of `` elements used in conjunction with HTML links whose only purpose is to apply a CSS class to the link. Instead, the CSS class attribute is applied directly to the link's `<a href="...">` element.
>
> There are three CSS styles for links previously used with embedded or embedding `` elements:
>
> - `type-name-link`, which was applied to a `` element inside the link. This was generated by passing `true` as argument for the `strong` parameter in various get*Link methods.
> - `member-name-link` and `search-tag-link`, which were applied to a `` element wrapping the link.
>
> I could find no visual changes in the generated documentation with one exception:
>
> [Class and interface links in the second column of "Uses" page tables][1] were previously wrapped in a `` element causing the whole class name (including non-linked parts such as type parameters) to be displayed with a bold font. With this change, the `type-name-link` style is applied only to the active link, leaving non-linked parts such as type parameters with normal font-weight.
>
> [1]: https://download.java.net/java/early_access/jdk17/docs/api/java.base/java/util/class-use/Map.html#java.util

Generally, very nice indeed. Another good cleanup/simplification/modernization.

There was one tiny suggestion for a whitespace cleanup.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java line 99:

> 97:
> 98: return writer.getDocLink(MEMBER_DEPRECATED_PREVIEW, utils.getEnclosingTypeElement(member),
> 99: member, content, null, false);

Depending how often you need to pass `null`, you might consider an overload that doesn't require a style.

Conversely, should all links have a style?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java line 114:

> 112: Content tdSummary) {
> 113: ExecutableElement ee = (ExecutableElement)member;
> 114: Content memberLink = writer.getDocLink(context, te, ee, name(ee), HtmlStyle.memberNameLink);

nice; I like the way this review is headed ;-)

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 783:

> 781: */
> 782: public Content getCrossClassLink(TypeElement classElement, String refMemName,
> 783: Content label, HtmlStyle style, boolean code) {

Yay!

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 875:

> 873: * @param context the id of the context where the link will be added
> 874: * @param typeElement the class to link to
> 875: * @param style optional style for the link

Just asking: when do we not want a style?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/LinkFactoryImpl.java line 115:

> 113: filename.fragment(classLinkInfo.where),
> 114: label,
> 115: classLinkInfo.style,

I guess we're lucky the code is downcasting the `LinkInfo` into `LinkInfoImpl`.

It would be a separate cleanup/rename, but this might be another place where the subtype might be better named `HtmlLinkInfo` rather than just `LinkInfoImpl` and this class might be `HtmlLinkFactory`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/Links.java line 163:

> 161: * @param link the details for the link
> 162: * @param label the content for the link
> 163: * @param style whether to wrap the {@code label} in a SPAN element

(very minor) white-space alignment

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/links/LinkInfo.java line 76:

> 74: */
> 75: public boolean excludeTypeBounds = false;
> 76:

I guess this is the counterpart on adding the style into the `LinkInfoImpl`. Yes, I agree this is the least ugly way to handle the improvement.

test/langtools/jdk/javadoc/doclet/testHtmlTableTags/TestHtmlTableTags.java line 574:

> 572: checkOutput("pkg1/C1.html", true,
> 573: """
> 574: <div class="col-first odd-row-color"><code><a href="../pkg2/C2.html" title="class in pkg2">C2</a></code></div>

I note that here is an example of a link without a style.

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

Marked as reviewed by jjg (Reviewer).

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

Hannes Wallnöfer

Re: RFR: JDK-8261499: Simplify HTML for javadoc links [v2]

In reply to this post by Hannes Wallnöfer

Hannes Wallnöfer has updated the pull request incrementally with one additional commit since the last revision:

JDK-8261499: Updates on feedback from code review
- Updated comments in markup.Links
- Rename LinkInfoImpl to HtmlLinkInfo and LinkFactoryImpl to HtmlLinkFactory
- Add HtmlDocletWriter.getDocLink(context, typeElement, element, label) overload

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

Changes:
- all: https://git.openjdk.java.net/jdk/pull/2516/files
- new: https://git.openjdk.java.net/jdk/pull/2516/files/875fb7da..e9f1c29f

Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2516&range=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2516&range=00-01

Stats: 203 lines in 24 files changed: 16 ins; 7 del; 180 mod
Patch: https://git.openjdk.java.net/jdk/pull/2516.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/2516/head:pull/2516

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

Hannes Wallnöfer

Integrated: JDK-8261499: Simplify HTML for javadoc links

In reply to this post by Hannes Wallnöfer

On Wed, 10 Feb 2021 18:20:02 GMT, Hannes Wallnöfer <[hidden email]> wrote:

This pull request has now been integrated.

Changeset: da9895a0
Author: Hannes Wallnöfer <[hidden email]>
URL: https://git.openjdk.java.net/jdk/commit/da9895a0
Stats: 896 lines in 52 files changed: 44 ins; 49 del; 803 mod

8261499: Simplify HTML for javadoc links

Reviewed-by: jjg

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

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