JDK-8329537: Nested and enclosing classes should be linked separately…

… in breadcrumb navigation
hns · May 29, 2024 · 1468be3 · 1468be3
1 parent c8eea59
commit 1468be3
Show file tree

Hide file tree

Showing 10 changed files with 68 additions and 130 deletions.
diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassUseWriter.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassUseWriter.java
@@ -430,21 +430,5 @@ protected HtmlTree getClassUseHeader() {
         bodyContents.setHeader(getHeader(PageMode.USE, typeElement)).addMainContent(div);
         return body;
     }
-
-    @Override
-    protected Navigation getNavBar(PageMode pageMode, Element element) {
-        List<Content> subnavLinks = new ArrayList<>();
-        if (configuration.showModules) {
-            subnavLinks.add(getBreadcrumbLink(utils.elementUtils.getModuleOf(typeElement), false));
-        }
-        // We may generate a class-use page for an otherwise undocumented page in the condition below.
-        boolean isUndocumented = options.noDeprecated() && utils.isDeprecated(typeElement);
-        subnavLinks.add(getBreadcrumbLink(utils.containingPackage(typeElement), isUndocumented));
-        if (!isUndocumented) {
-            subnavLinks.add(getBreadcrumbLink(typeElement, true));
-        }
-
-        return super.getNavBar(pageMode, element).setSubNavLinks(subnavLinks);
-    }
 }
 
diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriter.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriter.java
@@ -25,7 +25,6 @@
 
 package jdk.javadoc.internal.doclets.formats.html;
 
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Set;
@@ -449,17 +448,6 @@ protected Content getClassContentHeader() {
         return getContentHeader();
     }
 
-    @Override
-    protected Navigation getNavBar(PageMode pageMode, Element element) {
-        List<Content> subnavLinks = new ArrayList<>();
-        if (configuration.showModules) {
-            subnavLinks.add(getBreadcrumbLink(utils.elementUtils.getModuleOf(typeElement), false));
-        }
-        subnavLinks.add(getBreadcrumbLink(utils.containingPackage(typeElement), false));
-        subnavLinks.add(getBreadcrumbLink(typeElement, true));
-        return super.getNavBar(pageMode, element).setSubNavLinks(subnavLinks);
-    }
-
     protected void addFooter() {
         bodyContents.addMainContent(MarkerComments.END_OF_CLASS_DATA);
         bodyContents.setFooter(getFooter());

diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/DocFilesHandler.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/DocFilesHandler.java
@@ -214,19 +214,6 @@ public DocFileWriter(HtmlConfiguration configuration, DocPath path, DocFileEleme
             this.dfElement = dfElement;
         }
 
-        @Override
-        protected Navigation getNavBar(PageMode pageMode, Element element) {
-            List<Content> subnavLinks = new ArrayList<>();
-            var pkg = dfElement.getPackageElement();
-            if (configuration.showModules) {
-                subnavLinks.add(getBreadcrumbLink(utils.elementUtils.getModuleOf(element), pkg.isUnnamed()));
-            }
-            if (!pkg.isUnnamed()) {
-                subnavLinks.add(getBreadcrumbLink(pkg, true));
-            }
-            return super.getNavBar(pageMode, element).setSubNavLinks(subnavLinks);
-        }
-
         @Override
         public void buildPage() throws DocFileIOException {
 

diff --git a/...jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java b/...jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
@@ -750,29 +750,6 @@ protected DocPath pathString(PackageElement packageElement, DocPath name) {
         return pathToRoot.resolve(docPaths.forPackage(packageElement).resolve(name));
     }
 
-    /**
-     * {@return a content element containing a breadcrumb navigtation link for {@code elem}}
-     * Only module, package and type elements can appear in breadcrumb navigation.
-     *
-     * @param elem the element
-     * @param selected whether to use the style for current page element
-     */
-    protected Content getBreadcrumbLink(Element elem, boolean selected) {
-        HtmlTree link = switch (elem) {
-            case ModuleElement mdle -> links.createLink(pathToRoot.resolve(docPaths.moduleSummary(mdle)),
-                    Text.of(mdle.getQualifiedName()));
-            case PackageElement pkg -> links.createLink(pathString(pkg, DocPaths.PACKAGE_SUMMARY),
-                    getLocalizedPackageName(pkg));
-            case TypeElement type -> links.createLink(pathString(type, docPaths.forName(type)),
-                    utils.getSimpleName(type));
-            default -> throw new IllegalArgumentException(Objects.toString(elem));
-        };
-        if (selected) {
-            link.setStyle(HtmlStyle.currentSelection);
-        }
-        return link;
-    }
-
     /**
      * {@return the link to the given package}
      *

diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ModuleWriter.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ModuleWriter.java
@@ -274,12 +274,6 @@ protected Content getModuleHeader(String heading) {
         return body;
     }
 
-    @Override
-    protected Navigation getNavBar(PageMode pageMode, Element element) {
-        return super.getNavBar(pageMode, element)
-                .setSubNavLinks(List.of(getBreadcrumbLink(mdle, true)));
-    }
-
     protected Content getContentHeader() {
         return new ContentBuilder();
     }

diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Navigation.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Navigation.java
@@ -27,8 +27,10 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
 
 import javax.lang.model.element.Element;
+import javax.lang.model.element.ElementKind;
 import javax.lang.model.element.ModuleElement;
 import javax.lang.model.element.PackageElement;
 import javax.lang.model.element.TypeElement;
@@ -68,7 +70,6 @@ public class Navigation {
     private final PageMode documentedPage;
     private Content userHeader;
     private final String rowListTitle;
-    private List<Content> subNavLinks = List.of();
 
     public enum PageMode {
         ALL_CLASSES,
@@ -120,11 +121,6 @@ public Navigation setUserHeader(Content userHeader) {
         return this;
     }
 
-    public Navigation setSubNavLinks(List<Content> subNavLinks) {
-        this.subNavLinks = subNavLinks;
-        return this;
-    }
-
     /**
      * Adds the links for the main navigation.
      *
@@ -320,26 +316,6 @@ private void addMainNavLinks(Content target) {
         }
     }
 
-    /**
-     * Adds the navigation Type detail link.
-     *
-     * @param kind the kind of member being documented
-     * @param link true if the members are listed and need to be linked
-     * @param listContents the list of contents to which the detail will be added.
-     */
-    protected void addTypeDetailLink(VisibleMemberTable.Kind kind, boolean link, List<Content> listContents) {
-        addContentToList(listContents, switch (kind) {
-            case CONSTRUCTORS -> links.createLinkOrLabel(HtmlIds.CONSTRUCTOR_DETAIL, contents.navConstructor, link);
-            case ENUM_CONSTANTS -> links.createLinkOrLabel(HtmlIds.ENUM_CONSTANT_DETAIL, contents.navEnum, link);
-            case FIELDS -> links.createLinkOrLabel(HtmlIds.FIELD_DETAIL, contents.navField, link);
-            case METHODS -> links.createLinkOrLabel(HtmlIds.METHOD_DETAIL, contents.navMethod, link);
-            case PROPERTIES -> links.createLinkOrLabel(HtmlIds.PROPERTY_DETAIL, contents.navProperty, link);
-            case ANNOTATION_TYPE_MEMBER -> links.createLinkOrLabel(HtmlIds.ANNOTATION_TYPE_ELEMENT_DETAIL,
-                    contents.navAnnotationTypeMember, link);
-            default -> Text.EMPTY;
-        });
-    }
-
     private void addContentToList(List<Content> listContents, Content source) {
         listContents.add(HtmlTree.LI(source));
     }
@@ -397,6 +373,46 @@ private void addOverviewLink(Content target) {
         }
     }
 
+    /**
+     * Adds a breadcrumb navigation link for {@code element} to {@code contents} after
+     * having recursively invoked itself with its {@code element}'s containing element.
+     * Only module, package and type elements are supported in breadcrumb navigation.
+     *
+     * @param elem a module, package or type element
+     * @param contents the list to which links are added
+     * @param selected {@code true} if elem is the current page element
+     */
+    protected void addBreadcrumbLinks(Element elem, List<Content> contents, boolean selected) {
+        if (elem == null) {
+            return;
+        } else if (elem.getKind() != ElementKind.MODULE) {
+            addBreadcrumbLinks(elem.getEnclosingElement(), contents, false);
+        } else if (!configuration.showModules) {
+            return;
+        }
+        var docPaths = configuration.docPaths;
+        HtmlTree link = switch (elem) {
+            case ModuleElement mdle -> links.createLink(pathToRoot.resolve(
+                    docPaths.moduleSummary(mdle)),
+                    Text.of(mdle.getQualifiedName()));
+            case PackageElement pkg -> links.createLink(pathToRoot.resolve(
+                    docPaths.forPackage(pkg).resolve(DocPaths.PACKAGE_SUMMARY)),
+                    pkg.isUnnamed()
+                            ? configuration.contents.defaultPackageLabel
+                            : Text.of(pkg.getQualifiedName()));
+            // Breadcrumb navigation displays nested classes as separate links.
+            // Enclosing classes may be undocumented, in which case we just display the class name.
+            case TypeElement type -> (configuration.isGeneratedDoc(type) && !configuration.utils.hasHiddenTag(type))
+                    ? links.createLink(pathToRoot.resolve(
+                            docPaths.forClass(type)), type.getSimpleName().toString())
+                    : HtmlTree.SPAN(Text.of(type.getSimpleName().toString()));
+            default -> throw new IllegalArgumentException(Objects.toString(elem));
+        };
+        if (selected) {
+            link.setStyle(HtmlStyle.currentSelection);
+        }
+        contents.add(link);
+    }
 
     private void addPageElementLink(Content list) {
         Content link = switch (element) {
@@ -530,7 +546,12 @@ public Content getContent() {
         navigationBar.add(HtmlTree.DIV(HtmlStyle.topNav, navContent).setId(HtmlIds.NAVBAR_TOP));
 
         var subNavContent = HtmlTree.DIV(HtmlStyle.navContent);
-
+        List<Content> subNavLinks = new ArrayList<>();
+        switch (documentedPage) {
+            case MODULE, PACKAGE, CLASS, USE, DOC_FILE, TREE -> {
+                addBreadcrumbLinks(element, subNavLinks, true);
+            }
+        }
         // Add the breadcrumb navigation links if present.
         var breadcrumbNav = HtmlTree.OL(HtmlStyle.subNavList);
         breadcrumbNav.addAll(subNavLinks, HtmlTree::LI);

diff --git a/...dk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageTreeWriter.java b/...dk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageTreeWriter.java
@@ -130,18 +130,6 @@ protected HtmlTree getPackageTreeHeader() {
         return body;
     }
 
-    @Override
-    protected Navigation getNavBar(PageMode pageMode, Element element) {
-        List<Content> subnavLinks = new ArrayList<>();
-        if (configuration.showModules) {
-            var mdle = utils.elementUtils.getModuleOf(packageElement);
-            subnavLinks.add(getModuleLink(mdle, Text.of(mdle.getQualifiedName())));
-        }
-        subnavLinks.add(links.createLink(pathString(packageElement, DocPaths.PACKAGE_SUMMARY),
-                getLocalizedPackageName(packageElement), HtmlStyle.currentSelection, ""));
-        return super.getNavBar(pageMode, element).setSubNavLinks(subnavLinks);
-    }
-
     /**
      * Add a link to the tree for all the packages.
      *

diff --git a/...jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageUseWriter.java b/...jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageUseWriter.java
@@ -215,14 +215,4 @@ private HtmlTree getBody() {
                 .addMainContent(div);
         return body;
     }
-
-    @Override
-    protected Navigation getNavBar(PageMode pageMode, Element element) {
-        List<Content> subnavLinks = new ArrayList<>();
-        if (configuration.showModules) {
-            subnavLinks.add(getBreadcrumbLink(utils.elementUtils.getModuleOf(packageElement), false));
-        }
-        subnavLinks.add(getBreadcrumbLink(packageElement, true));
-        return super.getNavBar(pageMode, element).setSubNavLinks(subnavLinks);
-    }
 }
diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageWriter.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageWriter.java
@@ -268,16 +268,6 @@ private List<PackageElement> findRelatedPackages() {
         return packages;
     }
 
-    @Override
-    protected Navigation getNavBar(PageMode pageMode, Element element) {
-        List<Content> subnavLinks = new ArrayList<>();
-        if (configuration.showModules) {
-            subnavLinks.add(getBreadcrumbLink(utils.elementUtils.getModuleOf(packageElement), false));
-        }
-        subnavLinks.add(getBreadcrumbLink(packageElement, true));
-        return super.getNavBar(pageMode, element).setSubNavLinks(subnavLinks);
-    }
-
     /**
      * Add the package deprecation information to the documentation tree.
      *

diff --git a/test/langtools/jdk/javadoc/doclet/testNavigation/TestNavigation.java b/test/langtools/jdk/javadoc/doclet/testNavigation/TestNavigation.java
@@ -24,7 +24,7 @@
 /*
  * @test
  * @bug      7025314 8023700 7198273 8025633 8026567 8081854 8196027 8182765
- *           8196200 8196202 8223378 8258659 8261976 8320458
+ *           8196200 8196202 8223378 8258659 8261976 8320458 8329537
  * @summary  Make sure the Next/Prev Class links iterate through all types.
  *           Make sure the navagation is 2 columns, not 3.
  * @library  /tools/lib ../../lib
@@ -178,6 +178,12 @@ public static class Y extends X {
         checkExit(Exit.OK);
 
         checkOrder("pkg1/A.X.html",
+                """
+                    <ol class="sub-nav-list">
+                    <li><a href="package-summary.html">pkg1</a></li>
+                    <li><a href="A.html">A</a></li>
+                    <li><a href="A.X.html" class="current-selection">X</a></li>
+                    </ol>""",
                 """
                     <ol class="toc-list">
                     <li><a href="#" tabindex="0">Description</a></li>
@@ -203,6 +209,12 @@ public static class Y extends X {
                     </ol>""");
 
         checkOrder("pkg1/A.Y.html",
+                """
+                    <ol class="sub-nav-list">
+                    <li><a href="package-summary.html">pkg1</a></li>
+                    <li><a href="A.html">A</a></li>
+                    <li><a href="A.Y.html" class="current-selection">Y</a></li>
+                    </ol>""",
                 """
                     <ol class="toc-list">
                     <li><a href="#" tabindex="0">Description</a></li>
@@ -218,6 +230,13 @@ public static class Y extends X {
                     </ol>""");
 
         checkOrder("pkg1/A.X.IC.html",
+                """
+                    <ol class="sub-nav-list">
+                    <li><a href="package-summary.html">pkg1</a></li>
+                    <li><a href="A.html">A</a></li>
+                    <li><a href="A.X.html">X</a></li>
+                    <li><a href="A.X.IC.html" class="current-selection">IC</a></li>
+                    </ol>""",
                 """
                     <ol class="toc-list">
                     <li><a href="#" tabindex="0">Description</a></li>