Resolver filename fixes (#532)

Make flyout work when `readthedocs.resolver.filename` is `null`. In that
case, we get this value from the META tag added by Cloudflare.

This was the original intent here, but I missed allowing `null` in the
data validation. Besides, even if the data validation allows `null`,
there was another bug since I was re-defining the variable (`const ...`)
instead of overriding its value.

We didn't catch this locally because we have a different rule that makes
the request to always contain `url=` parameter and that case is not
affected by this.

Besides, I didn't find this when doing QA after deploy because I tested
with examples that sent `url=` parameter.

I'll see if I can write a test for this in another PR.

Author: humitos

src/data-validation.js

@@ -250,7 +250,7 @@ const addons_flyout = {
           type: "object",
           required: ["filename"],
           properties: {
-            filename: { type: "string" },
+            filename: { type: ["string", "null"] },
           },
         },
       },
@@ -463,7 +463,7 @@ const addons_notifications = {
           type: "object",
           required: ["filename"],
           properties: {
-            filename: { type: "string" },
+            filename: { type: ["string", "null"] },
           },
         },
       },

src/utils.js

@@ -360,9 +360,7 @@ export function getLinkWithFilename(url, resolverFilename) {
       // Normal case for most of the documentation tools.
       // Get the resolver's filename returned by the application (as HTTP header)
       // and injected by Cloudflare Worker as a meta HTML tag
-      const resolverFilename = getMetadataValue(
-        "readthedocs-resolver-filename",
-      );
+      resolverFilename = getMetadataValue("readthedocs-resolver-filename");
     }
   }
 

tests/flyout.test.html

@@ -10,9 +10,15 @@
       import * as flyout from "../src/flyout";
 
       let config;
+      let metaResolverFilename;
 
       runTests(async () => {
         beforeEach(() => {
+          metaResolverFilename = document.querySelector(
+            "meta[name='readthedocs-resolver-filename']",
+          );
+          metaResolverFilename.content = "/";
+
           config = {
             addons: {
               search: {
@@ -312,6 +318,42 @@
             );
             expect(searchForm).to.not.exist;
           });
+
+          it("flyout with readthedocs.resolver.filename being null", async () => {
+            metaResolverFilename.content = "/guides/tests.html";
+
+            // Disable the search from the config object
+            config.readthedocs.resolver.filename = null;
+
+            const addon = new flyout.FlyoutAddon(config);
+            const element = document.querySelector("readthedocs-flyout");
+            await elementUpdated(element);
+            const versionLink = element.shadowRoot.querySelector(
+              "main > dl.versions > dd > a",
+            );
+            expect(versionLink).dom.to.equal(
+              '<a href="https://project.readthedocs.io/en/stable/guides/tests.html">stable</a>',
+            );
+          });
+
+          it("flyout with readthedocs.resolver.filename and different meta.content", async () => {
+            metaResolverFilename.content = "/guides/tests.html";
+
+            // Add a different value for the filename than the meta tag.
+            // This happens on SPA documentation tools
+            config.readthedocs.resolver.filename =
+              "/examples/single-page-application.html";
+
+            const addon = new flyout.FlyoutAddon(config);
+            const element = document.querySelector("readthedocs-flyout");
+            await elementUpdated(element);
+            const versionLink = element.shadowRoot.querySelector(
+              "main > dl.versions > dd > a",
+            );
+            expect(versionLink).dom.to.equal(
+              '<a href="https://project.readthedocs.io/en/stable/examples/single-page-application.html">stable</a>',
+            );
+          });
         });
       });
     </script>

tests/utils.test.html

@@ -150,6 +150,7 @@
             const tool = new utils.DocumentationTool();
 
             expect(tool.getDocumentationTool()).to.be.equal("mkdocs-material");
+            expect(tool.isSinglePageApplication()).to.be.false;
             expect(tool.isSphinx()).to.be.false;
             expect(tool.isSphinxAlabasterLikeTheme()).to.be.false;
             expect(tool.isPelican()).to.be.false;
@@ -177,13 +178,36 @@
             const tool = new utils.DocumentationTool();
 
             expect(tool.getDocumentationTool()).to.be.equal("docusaurus");
+            expect(tool.isSinglePageApplication()).to.be.true;
             expect(tool.isJekyll()).to.be.false;
             expect(tool.isDocusaurus()).to.be.true;
             expect(tool.getRootSelector()).to.be.equal("article div.markdown");
             expect(tool.getLinkSelector()).to.be.equal(
               "article div.markdown p a",
             );
           });
+
+          it("VitePress", async () => {
+            element = document.createElement("meta");
+            element.name = "generator";
+            element.content = "VitePress generator 0.1.2";
+            document.head.appendChild(element);
+
+            const content = `<article id="content"><p>This is a <a href="#">link</a></p></article>`;
+            document.body.insertAdjacentHTML("beforeend", content);
+
+            // We can't rely on the DocumentationTool exported since the
+            // constructur is triggered before we add the required element to
+            // the DOM.
+            const tool = new utils.DocumentationTool();
+
+            expect(tool.getDocumentationTool()).to.be.equal("vitepress");
+            expect(tool.isSinglePageApplication()).to.be.true;
+            expect(tool.isJekyll()).to.be.false;
+            expect(tool.isDocusaurus()).to.be.false;
+            expect(tool.getRootSelector()).to.be.equal("article");
+            expect(tool.getLinkSelector()).to.be.equal("article p a");
+          });
         });
       });
     </script>