diff --git a/src/gui/choiceList/ChoiceListItem.svelte b/src/gui/choiceList/ChoiceListItem.svelte index 783677ce3..efd7dffbd 100644 --- a/src/gui/choiceList/ChoiceListItem.svelte +++ b/src/gui/choiceList/ChoiceListItem.svelte @@ -35,7 +35,7 @@ // renderChoiceName writes to the DOM (it's a side effect) -> $effect, not $derived. $effect(() => { if (nameElement) { - renderChoiceName(choice.name, nameElement, cmp, app); + void renderChoiceName(choice.name, nameElement, cmp, app); } }); diff --git a/src/gui/choiceList/MultiChoiceListItem.svelte b/src/gui/choiceList/MultiChoiceListItem.svelte index 76b38052f..ea8246092 100644 --- a/src/gui/choiceList/MultiChoiceListItem.svelte +++ b/src/gui/choiceList/MultiChoiceListItem.svelte @@ -59,7 +59,7 @@ $effect(() => { if (nameElement) { - renderChoiceName(choice.name, nameElement, cmp, app); + void renderChoiceName(choice.name, nameElement, cmp, app); } }); diff --git a/src/gui/choiceList/renderChoiceName.test.ts b/src/gui/choiceList/renderChoiceName.test.ts index 88e713dea..bc12f0d41 100644 --- a/src/gui/choiceList/renderChoiceName.test.ts +++ b/src/gui/choiceList/renderChoiceName.test.ts @@ -21,12 +21,74 @@ describe("renderChoiceName", () => { return Promise.resolve(); }); - renderChoiceName("First choice", element, component, app); - renderChoiceName("Second choice", element, component, app); + void renderChoiceName("**First** choice", element, component, app); + void renderChoiceName("**Second** choice", element, component, app); expect(renderSpy).toHaveBeenCalledTimes(2); - expect(element.textContent).toBe("Second choice"); + expect(element.textContent).toBe("**Second** choice"); expect(element.querySelectorAll("strong")).toHaveLength(1); - expect(element.textContent).not.toContain("First choice"); + expect(element.textContent).not.toContain("First"); + }); + + // Every markdown render runs every plugin's post processors, and one that + // scans the vault per call made a 27-choice picker take ~1 s to open (#2251). + it.each([ + "Daily note", + "Daily note - work (inbox), v1.2; ok?", + "\"Quoted\" isn't markdown", + "2024 review", + "1.5 hours", + "example.com", + "Café 📅 → ünïcode", + ])("renders the plain name %j as text without the markdown pipeline", (name) => { + const element = document.createElement("span"); + element.textContent = "stale"; + const renderSpy = vi.spyOn(MarkdownRenderer, "render"); + + void renderChoiceName(name, element, new Component(), new App()); + + expect(renderSpy).not.toHaveBeenCalled(); + expect(element.innerHTML).toBe(`

${name}

`); + }); + + // Each of these renders differently from its own text in Obsidian. + it.each([ + "**bold**", + "a _b_ c", + "[[Link]]", + "[a](b)", + "x", + "`code`", + "a ==b== c", + "#tag", + "# Heading", + "- item", + "-- -", + "+ item", + "1. item", + "1) item", + "> quote", + " indented", + "www.example.com", + "https://example.com", + "x\u2028www.example.com", + "x\u2029www.example.com", + "Q&A", + "$x$", + "a %%hidden%% b", + ])("renders %j through the markdown pipeline", (name) => { + const renderSpy = vi + .spyOn(MarkdownRenderer, "render") + .mockResolvedValue(undefined); + + void renderChoiceName(name, document.createElement("span"), new Component(), new App()); + + expect(renderSpy).toHaveBeenCalledWith( + expect.anything(), + name, + expect.anything(), + "/", + expect.anything(), + ); }); }); diff --git a/src/gui/choiceList/renderChoiceName.ts b/src/gui/choiceList/renderChoiceName.ts index e074c4091..5a554fcae 100644 --- a/src/gui/choiceList/renderChoiceName.ts +++ b/src/gui/choiceList/renderChoiceName.ts @@ -1,11 +1,42 @@ import { MarkdownRenderer, type App, type Component } from "obsidian"; +import { createOwnedElement } from "../../utils/activeWindow"; -export function renderChoiceName( +/** + * Characters no Markdown construct uses: letters, digits, spaces, `,.'"?;()-`, + * and anything outside ASCII. The Unicode line and paragraph separators are + * left to the renderer, since a parser may treat them as line breaks. + */ +const PLAIN_CHARACTERS = /^(?:[a-z0-9 ,.'"?;()-]|[^\x00-\x7f\u2028\u2029])+$/i; +/** Openings that still make a list or an indented code block from those characters. */ +const BLOCK_OPENING = /^(?:[-\s]|\d+[.)](?:\s|$))/; + +/** Whether Obsidian would render the name as exactly its own text. */ +function isPlainName(name: string): boolean { + return ( + PLAIN_CHARACTERS.test(name) && + !BLOCK_OPENING.test(name) && + !/www\./i.test(name) + ); +} + +/** + * Renders a choice name, which may hold Markdown or HTML. Plain names skip the + * Markdown renderer because it runs every plugin's post processors, so one slow + * post processor would otherwise be paid once per row. + */ +export async function renderChoiceName( choiceName: string, - element: HTMLSpanElement, + element: HTMLElement, component: Component, app: App, -): void { +): Promise { element.replaceChildren(); - void MarkdownRenderer.render(app, choiceName, element, "/", component); + if (isPlainName(choiceName)) { + const paragraph = createOwnedElement(element, "p"); + paragraph.dir = "auto"; + paragraph.textContent = choiceName; + element.appendChild(paragraph); + return; + } + await MarkdownRenderer.render(app, choiceName, element, "/", component); } diff --git a/src/gui/suggesters/choiceSuggester.test.ts b/src/gui/suggesters/choiceSuggester.test.ts index 677c96090..d3df117eb 100644 --- a/src/gui/suggesters/choiceSuggester.test.ts +++ b/src/gui/suggesters/choiceSuggester.test.ts @@ -1,7 +1,7 @@ // @vitest-environment jsdom import { createChoiceExecutor } from "../../../tests/helpers/createChoiceExecutor"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { App, Notice, prepareFuzzySearch } from "obsidian"; +import { App, MarkdownRenderer, Notice, prepareFuzzySearch } from "obsidian"; // Spy on the engine entry while keeping hasConfiguredTemplateFolders real (the // constructor's injection gate depends on it). @@ -819,6 +819,21 @@ describe("ChoiceSuggester", () => { expect(el.textContent).toBe("Top note"); }); + it("renders plain names without running other plugins' markdown post processors (#2251)", async () => { + const renderSpy = vi.spyOn(MarkdownRenderer, "render"); + const styled = choice("**Bold** note"); + const suggester = makeSuggester([topNote, styled]); + + const plainEl = await render(suggester, topNote); + const styledEl = await render(suggester, styled); + + expect(plainEl.textContent).toBe("Top note"); + expect(styledEl.querySelector("strong")?.textContent).toBe("Bold"); + expect(renderSpy.mock.calls.map(([, source]) => source)).toEqual([ + "**Bold** note", + ]); + }); + it("renders the default choice-type icon", async () => { const suggester = makeSuggester(rootChoices); diff --git a/src/gui/suggesters/choiceSuggester.ts b/src/gui/suggesters/choiceSuggester.ts index 8c88c9150..9e40a8c5f 100644 --- a/src/gui/suggesters/choiceSuggester.ts +++ b/src/gui/suggesters/choiceSuggester.ts @@ -2,7 +2,6 @@ import type { FuzzyMatch } from "obsidian"; import { Component, FuzzySuggestModal, - MarkdownRenderer, Notice, prepareFuzzySearch, setIcon, @@ -32,6 +31,7 @@ import { } from "../../engine/runTemplateFromFolder"; import { resolveChoiceIcon } from "../../utils/choiceUtils"; import { createOwnedElement } from "../../utils/activeWindow"; +import { renderChoiceName } from "../choiceList/renderChoiceName"; const backLabel = "← Back"; @@ -435,7 +435,7 @@ export default class ChoiceSuggester extends FuzzySuggestModal { cls: "quickadd-choice-suggestion-title quickadd-choice-suggestion-text", }); } - void MarkdownRenderer.render(this.app, item.item.name, nameEl, "", this.markdownComponent) + void renderChoiceName(item.item.name, nameEl, this.markdownComponent, this.app) .catch((error) => { nameEl.textContent = item.item.name; // renderSuggestion runs per row and re-runs on every keystroke, so a diff --git a/src/quickAddSettingsTab.test.ts b/src/quickAddSettingsTab.test.ts index d628d2919..a19f315f2 100644 --- a/src/quickAddSettingsTab.test.ts +++ b/src/quickAddSettingsTab.test.ts @@ -86,7 +86,7 @@ describe("settings user-authored DOM XSS safety", () => { const payload = ""; const container = document.createElement("span"); - renderChoiceName(`**Visible** ${payload}`, container, new Component(), new App()); + void renderChoiceName(`**Visible** ${payload}`, container, new Component(), new App()); await Promise.resolve(); expect(container.textContent).toContain("Visible"); @@ -95,7 +95,7 @@ describe("settings user-authored DOM XSS safety", () => { expectNoPayloadDom(container); const multiContainer = document.createElement("span"); - renderChoiceName(`Multi ${payload}`, multiContainer, new Component(), new App()); + void renderChoiceName(`Multi ${payload}`, multiContainer, new Component(), new App()); await Promise.resolve(); expect(multiContainer.textContent).toContain(`Multi ${payload}`); diff --git a/tests/e2e/choice-name-render.test.ts b/tests/e2e/choice-name-render.test.ts new file mode 100644 index 000000000..98458e51f --- /dev/null +++ b/tests/e2e/choice-name-render.test.ts @@ -0,0 +1,60 @@ +import { expect, it } from "vitest"; +import { MacroChoice } from "../../src/types/choices/MacroChoice"; +import { TemplateChoice } from "../../src/types/choices/TemplateChoice"; +import type { ICommand } from "../../src/types/macros/ICommand"; +import { createQuickAddE2EHarness, seedVaultFile } from "./e2eVault"; +import { jsLiteral, pressKey, waitForElement } from "./uiHelpers"; + +// The picker renders plain names without Obsidian's Markdown renderer (#2251), +// so each row has to match what that renderer makes of the same name. +const getContext = createQuickAddE2EHarness("choice-name-render"); + +const NAMES = [ + "Daily note", + "Daily note - work (inbox), v1.2; ok?", + "\"Quoted\" isn't markdown", + "1.5 hours", + "Café 📅 → ünïcode", + "**Bold** note", +]; + +it("renders each picker row as Obsidian's Markdown renderer renders its name", async () => { + const { obsidian, plugin, sandbox } = getContext(); + const script = await seedVaultFile(obsidian, sandbox, "keep-obsidian.js", + "module.exports = async ({ obsidian }) => { window.__qaObsidian = obsidian; };"); + const keep = new MacroChoice("Keep obsidian module"); + keep.macro.commands = [{ id: "keep-obsidian", name: "keep", type: "UserScript", path: script, settings: {} } as ICommand]; + await plugin.data<{ choices: unknown[] }>().patch((data) => { + data.choices = [keep, ...NAMES.map((name) => new TemplateChoice(name))]; + }); + await plugin.reload({ waitUntilReady: true }); + const run = await obsidian.execJson<{ ok: boolean }>("quickadd:run", { id: keep.id }); + expect(run, JSON.stringify(run)).toMatchObject({ ok: true }); + + await obsidian.dev.evalJson('(app.commands.executeCommandById("quickadd:runQuickAdd"), true)'); + try { + await waitForElement(obsidian, ".qa-choice-suggester .quickadd-choice-suggestion-title"); + const rows = await obsidian.dev.evalJsonAsync<{ name: string; row: string; renderer: string }[]>(`(async () => { + const { MarkdownRenderer, Component } = window.__qaObsidian; + const component = new Component(); + component.load(); + const titles = [...document.querySelectorAll(".qa-choice-suggester .quickadd-choice-suggestion-title")]; + const rows = []; + for (const name of ${jsLiteral(NAMES)}) { + const title = titles.find((el) => el.textContent === name.replace(/\\*\\*/g, "")); + const rendered = document.createElement("div"); + await MarkdownRenderer.render(app, name, rendered, "/", component); + rows.push({ name, row: title?.innerHTML ?? "missing", renderer: rendered.innerHTML }); + } + component.unload(); + delete window.__qaObsidian; + return rows; + })()`); + + for (const { name, row, renderer } of rows) { + expect(row, name).toBe(renderer); + } + } finally { + await pressKey(obsidian, "Escape"); + } +});