Fix logo paths for documents in project subdirectories

news/changelog-1.11.md

@@ -6,6 +6,7 @@ All changes included in 1.11:
 - ([#14731](https://github.com/quarto-dev/quarto-cli/pull/14731)): Fix `QUARTO_VENDOR_BINARIES=false` builds silently succeeding with missing binaries. The `configure` step's "is it on PATH?" check for `typst-gather`, `typst`, `pandoc` and `esbuild` compared an unawaited `Promise` against `undefined`, so the guard never fired. A build with `typst-gather` neither vendored nor on PATH reported success and produced a Quarto tree without it, surfacing only at render time as `typst-gather analyze failed; staging all packages as fallback`. The missing-binary error message now also names the path that was checked and the `QUARTO_TYPST_GATHER` environment variable.
 - ([#14741](https://github.com/quarto-dev/quarto-cli/issues/14741)): Fix PDF rendering of a cross-referenced table failing with `! Extra }, or forgotten \endgroup.` when the `endfloat` package defers `longtable` environments (`\DeclareDelayedFloatFlavor*{longtable}{table}`).
 - ([#14857](https://github.com/quarto-dev/quarto-cli/issues/14857)): Fix `quarto install chrome-headless-shell` (and `quarto install chromium`) failing with a 404 on Linux arm64 due to a stale Playwright CDN URL.
+- ([#14946](https://github.com/quarto-dev/quarto-cli/issues/14946)): Fix `revealjs` and `dashboard` logos resolving to broken paths in documents in a project subdirectory when the project has a brand (regression introduced in v1.8.22). Logo paths are now always relative to the document; a project-relative logo path in a website with a brand still works but prints a deprecation warning.
 - ([#14993](https://github.com/quarto-dev/quarto-cli/issues/14993)): Fix Typst output dropping a brand font from a `font-family` list when another document in the same directory or project was rendered first with different brand fonts.
 
 ## Accessibility

src/core/brand/brand.ts

@@ -29,17 +29,27 @@ import {
 } from "../../resources/types/zod/schema-types.ts";
 import { InternalError } from "../lib/error.ts";
 
-import { dirname, join, relative, resolve } from "../../deno_ral/path.ts";
+import {
+  dirname,
+  isAbsolute,
+  join,
+  relative,
+  resolve,
+} from "../../deno_ral/path.ts";
 import { warnOnce } from "../log.ts";
 import { isCssColorName } from "../css/color-names.ts";
-import { isExternalPath } from "../url.ts";
+import { isExternalPath, tryDecodeURI } from "../url.ts";
 import {
   LogoLightDarkSpecifierPathOptional,
   LogoOptionsPathOptional,
   LogoSpecifier,
   LogoSpecifierPathOptional,
 } from "../../resources/types/schema-types.ts";
-import { ensureLeadingSlash, pathWithForwardSlashes } from "../path.ts";
+import {
+  ensureLeadingSlash,
+  pathWithForwardSlashes,
+  safeExistsSync,
+} from "../path.ts";
 
 type ProcessedBrandData = {
   color: Record<string, string>;
@@ -439,6 +449,74 @@ export function logoAddLeadingSlashes(
 // left untouched.
 export function brandWithAbsoluteLogoPaths(
   brand: LightDarkBrand | undefined,
+): LightDarkBrand | undefined {
+  return brandWithLogoPaths(brand, ensureLeadingSlashIfNotExternal);
+}
+
+// Resolve the logo of an HTML document so that every path is relative to the
+// input document. Brand logo paths are project-relative and get rebased onto
+// the input directory before merging with the document logo, whose paths
+// (front matter, _quarto.yml, _metadata.yml, extensions) are already relative
+// to the input.
+export function resolveInputRelativeLogo(
+  brand: LightDarkBrand | undefined,
+  spec: LogoLightDarkSpecifier | undefined,
+  order: BrandNamedLogo[],
+  input: string,
+  isWebsite: boolean,
+): NormalizedLogoLightDarkSpecifier | undefined {
+  const inputDir = dirname(resolve(input));
+  const inputRelativeBrand = brandWithLogoPaths(
+    brand,
+    (path, projectDir) =>
+      isExternalPath(path)
+        ? path
+        : pathWithForwardSlashes(relative(inputDir, join(projectDir, path))),
+  );
+  const logo = resolveLogo(inputRelativeBrand, spec, order);
+  const projectDir = brand?.light?.projectDir ?? brand?.dark?.projectDir;
+  if (!logo || !projectDir || !isWebsite) {
+    return logo;
+  }
+  // Document logo paths that only exist relative to the project directory
+  // are deprecated: in a website with an active brand they still resolve,
+  // with a warning. A file relative to the document always takes precedence.
+  const projectRelativeFallback = (options: LogoOptions | undefined) => {
+    if (
+      !options || isExternalPath(options.path) || isAbsolute(options.path)
+    ) {
+      return options;
+    }
+    // Logo paths are URLs; check the files they point to, as HTML resource
+    // processing does, but keep the path encoded in the output. A query or
+    // fragment is not part of the file name.
+    const [, urlPath, suffix] = options.path.match(/^([^?#]*)(.*)$/)!;
+    const filePath = tryDecodeURI(urlPath) ?? urlPath;
+    if (
+      safeExistsSync(join(inputDir, filePath)) ||
+      !safeExistsSync(join(projectDir, filePath))
+    ) {
+      return options;
+    }
+    const path = pathWithForwardSlashes(
+      relative(inputDir, join(projectDir, urlPath)),
+    ) + suffix;
+    warnOnce(
+      `Logo path '${options.path}' for ${
+        pathWithForwardSlashes(relative(projectDir, input))
+      } was resolved relative to the project directory. This is deprecated: logo paths are relative to the document, use '${path}' instead.`,
+    );
+    return { ...options, path };
+  };
+  return {
+    light: projectRelativeFallback(logo.light),
+    dark: projectRelativeFallback(logo.dark),
+  };
+}
+
+function brandWithLogoPaths(
+  brand: LightDarkBrand | undefined,
+  transformPath: (path: string, projectDir: string) => string,
 ): LightDarkBrand | undefined {
   if (!brand) {
     return brand;
@@ -451,14 +529,14 @@ export function brandWithAbsoluteLogoPaths(
       if (oldLogo[size]) {
         logo[size] = {
           ...oldLogo[size],
-          path: ensureLeadingSlashIfNotExternal(oldLogo[size]!.path),
+          path: transformPath(oldLogo[size]!.path, b.projectDir),
         };
       }
     }
     for (const [key, value] of Object.entries(oldLogo.images)) {
       logo.images[key] = {
         ...value,
-        path: ensureLeadingSlashIfNotExternal(value.path),
+        path: transformPath(value.path, b.projectDir),
       };
     }
     const copy = Object.create(b) as Brand;

src/core/html.ts

@@ -9,6 +9,7 @@ import { Document, Element } from "./deno-dom.ts";
 
 import { pandocAutoIdentifier } from "./pandoc/pandoc-id.ts";
 import { isFileRef } from "./http.ts";
+import { tryDecodeURI } from "./url.ts";
 import { cssFileRefs } from "./css.ts";
 import { HtmlPostProcessResult } from "../command/render/types.ts";
 import { warning } from "../deno_ral/log.ts";
@@ -20,18 +21,14 @@ export function asHtmlId(text: string) {
 export function getDecodedAttribute(element: Element, attrib: string) {
   const value = element.getAttribute(attrib);
   if (value) {
-    try {
-      return decodeURI(value);
-    } catch (e) {
-      if (e instanceof URIError) {
-        warning(
-          `Invalid URI '${value}' in attribute '${attrib}' of element '${element.tagName}'`,
-        );
-        return value;
-      } else {
-        throw e;
-      }
+    const decoded = tryDecodeURI(value);
+    if (decoded === undefined) {
+      warning(
+        `Invalid URI '${value}' in attribute '${attrib}' of element '${element.tagName}'`,
+      );
+      return value;
     }
+    return decoded;
   } else {
     return value;
   }

src/core/url.ts

@@ -14,6 +14,18 @@ export function isExternalPath(path: string) {
   return /^\w+:/.test(path);
 }
 
+// Returns undefined when the value contains a malformed escape sequence.
+export function tryDecodeURI(value: string): string | undefined {
+  try {
+    return decodeURI(value);
+  } catch (e) {
+    if (e instanceof URIError) {
+      return undefined;
+    }
+    throw e;
+  }
+}
+
 export function joinUrl(baseUrl: string, path: string) {
   const baseHasSlash = baseUrl.endsWith("/");
 

src/format/dashboard/format-dashboard.ts

@@ -67,7 +67,7 @@ import { processToolbars } from "./format-dashboard-toolbar.ts";
 import { processDatatables } from "./format-dashboard-tables.ts";
 import { assert } from "testing/asserts";
 import { brandBootstrapSassBundles } from "../../core/sass/brand.ts";
-import { logoAddLeadingSlashes, resolveLogo } from "../../core/brand/brand.ts";
+import { resolveInputRelativeLogo } from "../../core/brand/brand.ts";
 
 const kDashboardClz = "quarto-dashboard";
 
@@ -131,14 +131,13 @@ export function dashboardFormat() {
             alt: format.metadata[kLogoAlt] as string,
           };
         }
-        let logo = resolveLogo(brand, logoSpec, [
-          "small",
-          "medium",
-          "large",
-        ]);
-        logo = logoAddLeadingSlashes(logo, brand, input);
-
-        format.metadata[kLogo] = logo;
+        format.metadata[kLogo] = resolveInputRelativeLogo(
+          brand,
+          logoSpec,
+          ["small", "medium", "large"],
+          input,
+          isWebsiteProject,
+        );
         const extras: FormatExtras = await baseHtmlFormat.formatExtras(
           input,
           markdown,

src/format/reveal/format-reveal.ts

@@ -77,10 +77,11 @@ import {
 } from "./constants.ts";
 import { revealMetadataFilter } from "./metadata.ts";
 import { ProjectContext } from "../../project/types.ts";
+import { projectIsWebsite } from "../../project/project-shared.ts";
 import { titleSlidePartial } from "./format-reveal-title.ts";
 import { registerWriterFormatHandler } from "../format-handlers.ts";
 import { pandocNativeStr } from "../../core/pandoc/codegen.ts";
-import { logoAddLeadingSlashes, resolveLogo } from "../../core/brand/brand.ts";
+import { resolveInputRelativeLogo } from "../../core/brand/brand.ts";
 
 export function revealResolveFormat(format: Format) {
   format.metadata = revealMetadataFilter(format.metadata);
@@ -301,7 +302,9 @@ export function revealjsFormat() {
                   theme["text-highlighting-mode"],
                 ),
               ],
-              [kMarkdownAfterBody]: [revealMarkdownAfterBody(format, input)],
+              [kMarkdownAfterBody]: [
+                revealMarkdownAfterBody(format, input, project),
+              ],
             },
           },
         );
@@ -392,7 +395,11 @@ export function revealjsFormat() {
   );
 }
 
-function revealMarkdownAfterBody(format: Format, input: string) {
+function revealMarkdownAfterBody(
+  format: Format,
+  input: string,
+  project: ProjectContext,
+) {
   let brandMode: "light" | "dark" = "light";
   if (format.metadata[kBrandMode] === "dark") {
     brandMode = "dark";
@@ -401,14 +408,15 @@ function revealMarkdownAfterBody(format: Format, input: string) {
   lines.push("::: {.quarto-auto-generated-content style='display: none;'}\n");
   const revealLogo = format
     .metadata[kSlideLogo] as (string | { path: string } | undefined);
-  let logo = resolveLogo(format.render.brand, revealLogo, [
-    "small",
-    "medium",
-    "large",
-  ]);
+  const logo = resolveInputRelativeLogo(
+    format.render.brand,
+    revealLogo,
+    ["small", "medium", "large"],
+    input,
+    projectIsWebsite(project),
+  );
   if (logo && logo[brandMode]) {
-    logo = logoAddLeadingSlashes(logo, format.render.brand, input);
-    const modeLogo = logo![brandMode]!;
+    const modeLogo = logo[brandMode]!;
     const altText = modeLogo.alt ? `alt="${modeLogo.alt}" ` : "";
     lines.push(
       `<img src="${modeLogo.path}" ${altText}class="slide-logo" />`,

tests/docs/smoke-all/2024/07/09/issue-10251/.gitignore

@@ -0,0 +1,2 @@
+/.quarto/
+**/*.quarto_ipynb

tests/docs/smoke-all/2024/10/30/issue-10864/.gitignore

@@ -0,0 +1,2 @@
+/.quarto/
+**/*.quarto_ipynb

tests/docs/smoke-all/brand/logo/brand-dark-only-logo-subdir/.gitignore

@@ -0,0 +1,3 @@
+/.quarto/
+**/*.quarto_ipynb
+/_site/

tests/docs/smoke-all/brand/logo/brand-dark-only-logo-subdir/_brand.yml

@@ -0,0 +1,2 @@
+color:
+  primary: "#00883A"

tests/docs/smoke-all/brand/logo/brand-dark-only-logo-subdir/_quarto.yml

@@ -0,0 +1,4 @@
+project:
+  type: website
+brand:
+  dark: _brand.yml

tests/docs/smoke-all/brand/logo/brand-dark-only-logo-subdir/sub/slides.qmd

@@ -0,0 +1,18 @@
+---
+title: "Project-relative logo in subdirectory with a dark-only brand (deprecated)"
+format: revealjs
+logo: img/top.png
+_quarto:
+  tests:
+    revealjs:
+      noErrors: true
+      printsMessage:
+        level: WARN
+        regex: 'img/top\.png.*project'
+      ensureFileRegexMatches:
+        -
+          - '<img src="\.\.(/|\\)img(/|\\)top\.png" class="slide-logo"'
+        - []
+---
+
+## Section

tests/docs/smoke-all/brand/logo/brand-logo-subdir-default-project/.gitignore

@@ -0,0 +1,2 @@
+/.quarto/
+**/*.quarto_ipynb

tests/docs/smoke-all/brand/logo/brand-logo-subdir-default-project/_brand.yml

@@ -0,0 +1,2 @@
+logo:
+  small: img/top.png

tests/docs/smoke-all/brand/logo/brand-logo-subdir-default-project/_quarto.yml

@@ -0,0 +1,2 @@
+project:
+  type: default

tests/docs/smoke-all/brand/logo/brand-logo-subdir-default-project/sub/projrel.qmd

@@ -0,0 +1,15 @@
+---
+title: "Project-relative logo in subdirectory of a default project is not rebased"
+format: revealjs
+logo: img/top.png
+_quarto:
+  tests:
+    revealjs:
+      noErrorsOrWarnings: true
+      ensureFileRegexMatches:
+        -
+          - '<img src="img(/|\\)top\.png" class="slide-logo"'
+        - []
+---
+
+## Section

tests/docs/smoke-all/brand/logo/brand-logo-subdir-default-project/sub/slides.qmd

@@ -0,0 +1,14 @@
+---
+title: "Brand logo in subdirectory of a default project"
+format: revealjs
+_quarto:
+  tests:
+    revealjs:
+      noErrorsOrWarnings: true
+      ensureFileRegexMatches:
+        -
+          - '<img src="\.\.(/|\\)img(/|\\)top\.png" class="slide-logo"'
+        - []
+---
+
+## Section

tests/docs/smoke-all/brand/logo/extension-format-logo-subdir/.gitignore

@@ -0,0 +1,3 @@
+/.quarto/
+**/*.quarto_ipynb
+/_site/

tests/docs/smoke-all/brand/logo/extension-format-logo-subdir/_brand.yml

@@ -0,0 +1,2 @@
+color:
+  primary: "#00883A"

tests/docs/smoke-all/brand/logo/extension-format-logo-subdir/_extensions/repro/_extension.yml

@@ -0,0 +1,9 @@
+title: Repro
+author: Quarto Dev Team
+version: 1.0.0
+contributes:
+  formats:
+    revealjs:
+      logo: logo.png
+    dashboard:
+      logo: logo.png

tests/docs/smoke-all/brand/logo/extension-format-logo-subdir/_quarto.yml

@@ -0,0 +1,3 @@
+project:
+  type: website
+brand: _brand.yml

tests/docs/smoke-all/brand/logo/extension-format-logo-subdir/collision/slides.qmd

@@ -0,0 +1,20 @@
+---
+title: "Logo path existing both relative to the document and to the project"
+format: revealjs
+logo: img/top.png
+_quarto:
+  tests:
+    revealjs:
+      noErrorsOrWarnings: true
+      fileExists:
+        outputPath: img/top.png
+      ensureFileRegexMatches:
+        -
+          - '<img src="img(/|\\)top\.png" class="slide-logo"'
+        -
+          - '<img src="\.\.(/|\\)img(/|\\)top\.png"'
+---
+
+`img/top.png` exists both next to this document and at the project root. Logo paths are relative to the document, so the document-relative file is used without a deprecation warning. Versions 1.8 to 1.10 picked the project-root file here, because they prefixed every logo path with `/` when the project had a brand.
+
+## Section