shell: Take docs from the correct manifest item

For example, "network/firewall" has no manifest entry of its
own. Rather, it is a sub-page of "network" and should show the same
documentation.

We also need to check the parent first now, since for example
"metrics" is a sub-page of "system", but still wants to show its own
docs.

Fixes cockpit-project#21040

pkg/shell/state.jsx

@@ -471,6 +471,18 @@ export function ShellState() {
      *
      * And then, the "metrics" path has the "Overview" item associated
      * with it, although the two come from different packages.
+     *
+     * - current_manifest
+     *
+     * The manifest corresponding to the "path" part of
+     * "current_location". The "current_manifest_item" is not
+     * necessarily part of this manifest, but this manifest is always
+     * from the same package as the files loaded for the current
+     * location.
+     *
+     * For example, for the "metrics" path the "current_manifest" will
+     * be for the "metrics" package, while "current_manifest_item" is
+     * for the "Overview" menu entry from the "system" package.
      */
 
     const self = {
@@ -486,6 +498,7 @@ export function ShellState() {
         current_machine: null,
         current_manifest_item: null,
         current_machine_manifest_items: null,
+        current_manifest: null,
 
         // Methods
         jump,
@@ -554,6 +567,7 @@ export function ShellState() {
         self.current_machine = machine;
         self.current_machine_manifest_items = compiled;
         self.current_manifest_item = item;
+        self.current_manifest = compiled.find_path_manifest(location.path);
 
         let frame = null;
         if (location.path && (machine.state == "connected" || machine.state == "connecting"))

pkg/shell/topnav.jsx

@@ -90,9 +90,9 @@ export class TopNav extends React.Component {
     render() {
         const Dialogs = this.context;
         const {
-            current_location,
             current_machine,
-            current_machine_manifest_items,
+            current_manifest_item,
+            current_manifest,
             current_frame,
         } = this.props.state;
 
@@ -113,27 +113,12 @@ export class TopNav extends React.Component {
             }
         }
 
-        // NOTE: The following is arguably wrong. We should not index
-        // the manifest items with the location path. Instead, we
-        // should use state.current_manifest_item.
-        //
-        // See https://github.com/cockpit-project/cockpit/issues/21040
-        //
-        const item = current_machine_manifest_items.items[current_location.path];
-        if (item && item.docs)
-            docs = item.docs;
-
-        // NOTE: The following is also arguably wrong. We should not
-        // index the manifests with the location path either. We
-        // should use only the first component after splitting the
-        // path at "/".  Also, we should look into
-        // current_machine.manifests instead of cockpit.manifests.
-        //
-        if (docs.length === 0) {
-            const comp = cockpit.manifests[current_location.path];
-            if (comp && comp.parent && comp.parent.docs)
-                docs = comp.parent.docs;
-        }
+        // Check first whether we have docs in the "parent" section of
+        // the manifest.
+        if (current_manifest.parent && current_manifest.parent.docs)
+            docs = current_manifest.parent.docs;
+        else  if (current_manifest_item.docs)
+            docs = current_manifest_item.docs;
 
         const docItems = [];
 

pkg/shell/util.jsx

@@ -86,11 +86,11 @@ export function push_window_location(location) {
     window.history.pushState(null, "", encode_location(location));
 }
 
-function CompiledComponents() {
+function CompiledComponents(manifests) {
     const self = this;
     self.items = {};
 
-    self.load = function(manifests, section) {
+    self.load = function(section) {
         Object.entries(manifests || { }).forEach(([name, manifest]) => {
             Object.entries(manifest[section] || { }).forEach(([prop, info]) => {
                 const item = {
@@ -167,21 +167,28 @@ function CompiledComponents() {
         }
 
         // Still don't know where it comes from, check for parent
-        if (!component) {
-            const comp = cockpit.manifests[path];
+        if (!component && manifests) {
+            const comp = manifests[path];
             if (comp && comp.parent)
                 component = comp.parent.component;
         }
 
         return self.items[component] || { path, label: path, section: "menu" };
     };
+
+    self.find_path_manifest = function(path) {
+        const parts = path.split("/");
+        const pkg = parts[0];
+
+        return (manifests && manifests[pkg]) || { };
+    };
 }
 
 export function compile_manifests(manifests) {
-    const compiled = new CompiledComponents();
-    compiled.load(manifests, "tools");
-    compiled.load(manifests, "dashboard");
-    compiled.load(manifests, "menu");
+    const compiled = new CompiledComponents(manifests);
+    compiled.load("tools");
+    compiled.load("dashboard");
+    compiled.load("menu");
     return compiled;
 }
 

test/verify/check-pages

@@ -130,6 +130,11 @@ OnCalendar=daily
         b.click("#toggle-docs")
         b.wait_not_present("#toggle-docs-menu")
         b.go("/network")
+        self.checkDocs(["Managing networking bonds", "Managing networking teams",
+                        "Managing networking bridges", "Managing VLANs", "Managing firewall"])
+        b.go("/system")
+        self.checkDocs(["Configuring system settings"])
+        b.go("/network/firewall")
         self.checkDocs(["Managing networking bonds", "Managing networking teams",
                         "Managing networking bridges", "Managing VLANs", "Managing firewall"])
         b.go("/system/services")