On Fri, 11 Jun 2021 07:14:42 GMT, Hannes Wallnöfer <hann...@openjdk.org> wrote:
> (This jdk17 PR is the continuation of PR openjdk/jdk#4459 in the mainline jdk
> repo, commits are identical at the point of transition.)
>
> This change fixes a whole slew of shortcomings in the redirection of relative
> links in doc comments. The basic idea is that relative links are authored to
> work in their "native primary" environment (e.g. the package summary page
> for a package or the class page for a class and its members), and have to be
> rewritten when used in other contexts such as "use" or index pages.
>
> A list of omissions that are fixed in this change:
>
> - Relative links in class or member comments were not redirected when
> inherited by other classes
> - Relative links in package comments were not rewritten when displayed in
> other package summaries as "related packages"
> - Fragment links used in foreign contexts were not completed with the file
> name
> - Relative links in module comments were not redirected at all
>
> While fixing above issues I also made sure link rewriting is kept to a
> minimum, avoiding it as much as possible for elements that live in the same
> package.
>
> Furthermore, the test for redirected relative links was a bit out of order.
> The `javadoc` command issued by the test returned `ERROR` because one of the
> source files contained non-valid HTML (an anchor with a `name` attribute to
> test whether that attribute would be modified). Because of this, the
> `checkLinks()` method was never invoked, which is a problem for a test that
> is supposed to make sure generated links are valid. I changed the test to use
> the valid `id` attribute instead of `name` and made sure `checkLinks()` is
> executed again.
>
> I also added checks for the newly supported cases. I added a whole new test
> for modules since retrofitting the existing test to cover modules would not
> have been practical.
Approved, but (as is often the case) there is potential for additional cleanup
downstream.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
line 1664:
> 1662: currentPageElement = moduleWriter.mdle;
> 1663: }
> 1664: }
File an RFE?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
line 1724:
> 1722: text = "{@" + (new DocRootTaglet()).getName() + "}/"
> 1723: + redirectPathFromRoot.resolve(text).getPath();
> 1724: text = replaceDocRootDir(text);
ah OK, I see you were just moving stuff around ;-)
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
line 1728:
> 1726: text = docPaths.forName(typeElement).getPath() + text;
> 1727: }
> 1728: }
This is OK, but would it be better to try and create a URI from the text, and
then use URI methods to check or manipulate the URI? Maybe an RFE...?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
line 1764:
> 1762: if (redirectPathFromRoot != null) {
> 1763: text = "{@" + (new DocRootTaglet()).getName() + "}/"
> 1764: + redirectPathFromRoot.resolve(text).getPath();
OK, but icky code ...
* icky to create an object to just get its name
* icky to create a comment fragment to get the docRoot behavior
I accept this is old code moved around. Maybe file an RFE for cleanup
test/langtools/jdk/javadoc/doclet/testRelativeLinks/TestRelativeLinks.java line
93:
> 91: checkOutput("index-all.html", true,
> 92: """
> 93: <div class="block"><a name="masters"></a>""");
wow!
test/langtools/jdk/javadoc/doclet/testRelativeLinks/TestRelativeLinks.java line
193:
> 191: public void checkLinks() {
> 192: // since the test uses explicit links to non-existent files,
> 193: // we create those files to avoid false positive errors from
> checkLinks
:-)
-------------
Marked as reviewed by jjg (Reviewer).
PR: https://git.openjdk.java.net/jdk17/pull/17
