mirror of
https://github.com/runbear-io/beardrive.git
synced 2026-08-25 08:08:08 +02:00
fix(webapp): a wikilink is a real link, not a wiki: string (BEA-136) (#151)
[[guide]] rendered as href="wiki:guide" — a pseudo-scheme no browser can resolve. The delegated click handler rescued a plain left-click, so the feature looked fine until someone copied the link, middle-clicked it, or opened it in a new tab and got a dead string. Resolution moves from click time to transform time: transformHTML (the pass that already rewrites this HTML before the mount) matches the target against flatFiles and writes the real urlForPath() URL, plus a data-wiki marker. A wikilink matching no file loses its href entirely and renders as .wiki-missing, so no "wiki:" survives into the DOM either way. The matching rules didn't change — they moved into a pure resolveWiki() in util.ts, where node --test covers the whole matrix without a browser. The consequence to get right is the click: real hrefs mean a plain click must be intercepted (or it does a full document load) and every modified click must be let through (or the fix buys nothing) — the same rule nav.ts:linkProps applies everywhere else. The guard sits above both branches, so cmd-clicking a relative markdown link now opens a tab too instead of SPA-navigating the current one. markdown.go is unchanged: wiki: stays the marker the server leaves behind because RenderMarkdown has no file tree. /s/<token> share pages keep their dead wikilinks by the spec's decision — the target isn't part of the share. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
fd392aa9b0
commit
398f30d64b
@@ -96,7 +96,7 @@ classDiagram
|
||||
}
|
||||
note for components "NewProjectDialog replaced ProjectNav's name-only modalPrompt: name + starting point, POSTing {name, template}. Its options come from useConfig()'s `templates`, never a hardcoded list, so a hub shipping another template needs no frontend change; the initial selection is options[0].value — the same array element the RECOMMENDED badge indexes, so the badged row and the checked row are one row by construction (on a template-less hub that row is 'I already have a folder', which still creates an empty project). modal.tsx keeps its one-field API — teaching it about choices would tax every other caller"
|
||||
note for components "HistoryFilters drives the SERVER (?q=/?user=/?since=/?until= on the history API), never the loaded page — filtering what is on screen would lie about everything below the fold and break next_cursor. Its state is Route.filters, so a narrowed feed is linkable, survives reload, and Back undoes it; the author list accumulates across fetches, because filtering by one author leaves only their rows loaded"
|
||||
note for components "FileView's transformHTML now drops `data:image/svg` from any rendered img and any `data:` href from any rendered link — goldmark admits them, and an inline SVG is a document rather than a picture (the same property the server's sandboxInline walls off). Insights builds its per-device folder bag with Object.create(null), since folder names come off a peer's journal and one named __proto__ silently emptied the matrix. style.css sets unicode-bidi isolate-override on the peer-authored strings a reader is expected to CHECK (listing rows, breadcrumb, history path/note/device) — journal.SafeText refuses the bidi CONTROLS, but a single strong-RTL LETTER is legal and still reorders a row"
|
||||
note for components "FileView's transformHTML resolves the server's `wiki:` marker against flatFiles into a real urlForPath() href (unresolvable ones lose the href and get .wiki-missing), so copy-link/middle-click/new-tab work and only a plain click reaches the delegated handler — resolution used to happen at click time, which left a dead `wiki:guide` string in the DOM (BEA-136). It also drops `data:image/svg` from any rendered img and any `data:` href from any rendered link — goldmark admits them, and an inline SVG is a document rather than a picture (the same property the server's sandboxInline walls off). Insights builds its per-device folder bag with Object.create(null), since folder names come off a peer's journal and one named __proto__ silently emptied the matrix. style.css sets unicode-bidi isolate-override on the peer-authored strings a reader is expected to CHECK (listing rows, breadcrumb, history path/note/device) — journal.SafeText refuses the bidi CONTROLS, but a single strong-RTL LETTER is legal and still reorders a row"
|
||||
note for components "HistoryView's RunGroup header carries the run-wide undo (POST undo-run, gated by the same write permission as the per-row restore/remove). It asks the SERVER for the file list first (preview: true) rather than deriving it from the loaded feed — that window is paged and filterable, so a client-computed list is wrong exactly when the run is old. modal.tsx's Confirm.message widened from string to ReactNode for it (the prompt's one-field API is untouched), so the dialog can show every path, its action, and the "changed after this run" warning inline"
|
||||
note for components "components/ui — shadcn/ui primitives (Radix, copied in), themed from BearDrive tokens in tw.css; rendered markdown is transformed as a string before mounting, link clicks delegated on the container — never patch the dangerouslySetInnerHTML subtree"
|
||||
|
||||
|
||||
@@ -234,7 +234,10 @@ func seedE2E(t *testing.T, state, prefix, projectID string) {
|
||||
Size: int64(len(content)), Mode: 0o644,
|
||||
})
|
||||
}
|
||||
put("index.md", "# Wiki\n\nStart at the [[guide]] or browse [notes](notes/readme.md).\n", 72*time.Hour)
|
||||
// The dangling [[nowhere]] is deliberate: the frontend has to render a
|
||||
// wikilink with no matching file as an unresolved anchor, not as a dead
|
||||
// "wiki:" href (BEA-136).
|
||||
put("index.md", "# Wiki\n\nStart at the [[guide]] or browse [notes](notes/readme.md). Nothing at [[nowhere]].\n", 72*time.Hour)
|
||||
put("guide.md", "# Guide\n\nFirst version of the guide.\n", 48*time.Hour)
|
||||
put("guide.md", "# Guide\n\nSecond version of the guide, with more detail.\n", 2*time.Hour)
|
||||
put("notes/readme.md", "# Notes\n\nNested folder content.\n", 24*time.Hour)
|
||||
|
||||
@@ -32,9 +32,33 @@ test("wikilink navigates to the target file", async ({ page }) => {
|
||||
await login(page);
|
||||
const pid = await wikiId(page);
|
||||
await page.goto(`/${pid}/index.md`);
|
||||
await page.click('#content a:has-text("guide")');
|
||||
const link = page.locator('#content a:has-text("guide")');
|
||||
// BEA-136: the href itself, not just the click. Copy-link-address,
|
||||
// middle-click and open-in-new-tab all read this attribute, and it used to
|
||||
// be the unresolvable string "wiki:guide".
|
||||
await expect(link).toHaveAttribute("href", `/${pid}/guide.md`);
|
||||
await page.evaluate(() => ((window as Window & { __spa?: number }).__spa = 1));
|
||||
await link.click();
|
||||
await page.waitForURL(`/${pid}/guide.md`);
|
||||
await expect(page.locator("#content")).toContainText("Second version");
|
||||
// Still the same document: a plain click must SPA-route, not reload.
|
||||
expect(await page.evaluate(() => (window as Window & { __spa?: number }).__spa)).toBe(1);
|
||||
});
|
||||
|
||||
// BEA-136: everything the rendered anchor has to get right besides the click.
|
||||
test("wikilinks: modified click is the browser's, a dangling one has no href", async ({ page }) => {
|
||||
await login(page);
|
||||
const pid = await wikiId(page);
|
||||
await page.goto(`/${pid}/index.md`);
|
||||
// A cmd/ctrl-click belongs to the browser (new tab), so THIS page stays put.
|
||||
await page.locator('#content a:has-text("guide")').click({ modifiers: ["ControlOrMeta"] });
|
||||
await page.waitForTimeout(300);
|
||||
expect(new URL(page.url()).pathname).toBe(`/${pid}/index.md`);
|
||||
// [[nowhere]] matches no file: unresolved, and no dead href to copy.
|
||||
const missing = page.locator("#content a.wiki-missing");
|
||||
await expect(missing).toHaveText("nowhere");
|
||||
expect(await missing.getAttribute("href")).toBeNull();
|
||||
expect(await page.locator("#content").innerHTML()).not.toContain("wiki:");
|
||||
});
|
||||
|
||||
test("folder listing: counts, change feed, heat dot on a read file", async ({ page }) => {
|
||||
|
||||
@@ -602,6 +602,7 @@ export default function Browser(props: {
|
||||
version={version}
|
||||
heatMap={heatMap}
|
||||
flatFiles={flatFiles}
|
||||
projectId={project?.id}
|
||||
onOpenFile={openPath}
|
||||
onMeta={setMeta}
|
||||
onRendered={onRendered}
|
||||
|
||||
@@ -13,8 +13,10 @@ import {
|
||||
TEXT_EXT,
|
||||
humanSize,
|
||||
joinPath,
|
||||
resolveWiki,
|
||||
whoChanged,
|
||||
} from "../util";
|
||||
import { urlForPath } from "../router";
|
||||
import { CSV_ROWS, parseDelimited, type Csv } from "../lib/csv";
|
||||
import { hasMermaid, renderMermaid } from "../lib/mermaid";
|
||||
|
||||
@@ -25,6 +27,8 @@ export function FileView(props: {
|
||||
version?: string;
|
||||
heatMap: HeatMap | null;
|
||||
flatFiles: Node[];
|
||||
// Hub mode only; absent in volume mode, where file URLs are "/<path>".
|
||||
projectId?: string;
|
||||
onOpenFile: (path: string) => void;
|
||||
onMeta: (meta: string) => void;
|
||||
onRendered?: () => void;
|
||||
@@ -130,7 +134,8 @@ function FileCard(props: {
|
||||
}
|
||||
|
||||
function MarkdownView(props: Parameters<typeof FileView>[0]) {
|
||||
const { apiBase, path, version, heatMap, flatFiles, onOpenFile, onMeta, onRendered } = props;
|
||||
const { apiBase, path, version, heatMap, flatFiles, projectId, onOpenFile, onMeta, onRendered } =
|
||||
props;
|
||||
const { data: doc, error } = useQuery({
|
||||
queryKey: ["render", apiBase, path, version || ""],
|
||||
queryFn: () =>
|
||||
@@ -148,8 +153,8 @@ function MarkdownView(props: Parameters<typeof FileView>[0]) {
|
||||
// update, silently discarding post-commit DOM patches. Link navigation
|
||||
// is delegated on the container for the same reason.
|
||||
const html = useMemo(
|
||||
() => (doc ? transformHTML(doc.html, path, apiBase) : ""),
|
||||
[doc, path, apiBase],
|
||||
() => (doc ? transformHTML(doc.html, path, apiBase, flatFiles, projectId) : ""),
|
||||
[doc, path, apiBase, flatFiles, projectId],
|
||||
);
|
||||
|
||||
// Diagrams are rendered into a NEW html string and fed back through state,
|
||||
@@ -197,27 +202,30 @@ function MarkdownView(props: Parameters<typeof FileView>[0]) {
|
||||
return (
|
||||
<div
|
||||
dangerouslySetInnerHTML={{ __html: diagrams ?? html }}
|
||||
onClick={(e) => handleLinkClick(e, path, flatFiles, onOpenFile)}
|
||||
onClick={(e) => handleLinkClick(e, path, onOpenFile)}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
/* Delegated click handling for rendered-markdown links: wiki: targets
|
||||
resolve by basename, relative links resolve against the current file's
|
||||
folder, everything else keeps its native behavior. */
|
||||
function handleLinkClick(
|
||||
e: React.MouseEvent,
|
||||
p: string,
|
||||
flatFiles: Node[],
|
||||
openFile: (path: string) => void,
|
||||
) {
|
||||
/* Delegated click handling for rendered-markdown links: wikilinks (already
|
||||
carrying a real in-app href, see transformHTML) and relative links route
|
||||
in-app on a plain click, everything else keeps its native behavior. */
|
||||
function handleLinkClick(e: React.MouseEvent, p: string, openFile: (path: string) => void) {
|
||||
const a = (e.target as HTMLElement).closest("a");
|
||||
if (!a || !(e.currentTarget as HTMLElement).contains(a)) return;
|
||||
// Same rule as nav.ts:linkProps — a modified or non-primary click belongs
|
||||
// to the browser (new tab, new window, download), which is the entire
|
||||
// point of these anchors carrying real URLs.
|
||||
if (e.metaKey || e.ctrlKey || e.shiftKey || e.altKey || e.button !== 0) return;
|
||||
const href = a.getAttribute("href") || "";
|
||||
const dir = p.includes("/") ? p.slice(0, p.lastIndexOf("/")) : "";
|
||||
if (href.startsWith("wiki:")) {
|
||||
// data-wiki, not "any root-absolute href": an author's own [x](/somewhere)
|
||||
// keeps its native behavior instead of quietly becoming an SPA route, and
|
||||
// the target path is read back directly instead of re-parsed out of a URL.
|
||||
const wiki = a.getAttribute("data-wiki");
|
||||
if (wiki !== null) {
|
||||
e.preventDefault();
|
||||
openWikilink(decodeURIComponent(href.slice(5)), flatFiles, openFile);
|
||||
openFile(wiki);
|
||||
} else if (!/^([a-z]+:|\/|#)/i.test(href)) {
|
||||
e.preventDefault();
|
||||
openFile(joinPath(dir, decodeURIComponent(href)));
|
||||
@@ -225,8 +233,15 @@ function handleLinkClick(
|
||||
}
|
||||
|
||||
/* String-level rewrite of the server's HTML: relative image sources point
|
||||
at the file API, external links open in a new tab. */
|
||||
function transformHTML(html: string, p: string, apiBase: string): string {
|
||||
at the file API, external links open in a new tab, and wikilinks get the
|
||||
real in-app URL of their target. */
|
||||
function transformHTML(
|
||||
html: string,
|
||||
p: string,
|
||||
apiBase: string,
|
||||
files: Node[],
|
||||
projectId?: string,
|
||||
): string {
|
||||
const dir = p.includes("/") ? p.slice(0, p.lastIndexOf("/")) : "";
|
||||
const fileURL = (path: string) => apiBase + "file?path=" + encodeURIComponent(path);
|
||||
const parsed = new DOMParser().parseFromString(html, "text/html");
|
||||
@@ -242,6 +257,25 @@ function transformHTML(html: string, p: string, apiBase: string): string {
|
||||
}
|
||||
for (const a of parsed.querySelectorAll("a")) {
|
||||
const href = a.getAttribute("href") || "";
|
||||
// The server has no file tree, so it leaves [[target]] as the marker
|
||||
// href="wiki:<target>" (markdown.go). Resolving it HERE — before the
|
||||
// mount — is what makes the anchor an ordinary link: copy-link,
|
||||
// middle-click and open-in-new-tab all read the attribute, and only a
|
||||
// plain click ever reaches the handler above.
|
||||
if (href.startsWith("wiki:")) {
|
||||
const hit = resolveWiki(decodeURIComponent(href.slice(5)), files);
|
||||
if (hit) {
|
||||
a.setAttribute("href", urlForPath(hit.path, projectId));
|
||||
a.setAttribute("data-wiki", hit.path);
|
||||
} else {
|
||||
// No file matches: an unusable "wiki:" string must not survive the
|
||||
// mount, so the anchor loses its href and says why on hover.
|
||||
a.removeAttribute("href");
|
||||
a.classList.add("wiki-missing");
|
||||
a.setAttribute("title", "No file matches this wikilink");
|
||||
}
|
||||
continue;
|
||||
}
|
||||
// goldmark's data: allowance exists for IMAGES and is applied to <a> as
|
||||
// well, so a rendered document can mount a link whose target is an
|
||||
// attacker-authored document. Browsers refuse a top-level data:
|
||||
@@ -344,14 +378,3 @@ function CsvTable({ csv }: { csv: Csv }) {
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
function openWikilink(target: string, flatFiles: Node[], openFile: (path: string) => void) {
|
||||
const want = target.toLowerCase();
|
||||
const hit =
|
||||
flatFiles.find((f) => f.path.toLowerCase() === want || f.path.toLowerCase() === want + ".md") ||
|
||||
flatFiles.find((f) => {
|
||||
const n = f.name.toLowerCase();
|
||||
return n === want || n === want + ".md";
|
||||
});
|
||||
if (hit) openFile(hit.path);
|
||||
}
|
||||
|
||||
@@ -1089,6 +1089,10 @@ a.ai-main:hover { color: var(--accent); }
|
||||
.markdown strong { color: var(--text); font-weight: 620; }
|
||||
.markdown a { color: var(--accent-bright); text-decoration: none; border-bottom: 1px solid rgba(245,166,35,.28); }
|
||||
.markdown a:hover { border-bottom-color: var(--accent); }
|
||||
/* A wikilink pointing at nothing. Without an href it loses link styling
|
||||
entirely, and a broken cross-reference silently reading as prose is worse
|
||||
than the dead URL this replaced. */
|
||||
.markdown a.wiki-missing { color: var(--text-faint); border-bottom: none; text-decoration: underline dotted; cursor: help; }
|
||||
.markdown ul, .markdown ol { margin: 0 0 1em; padding-left: 1.4em; }
|
||||
.markdown li { margin-bottom: .4em; }
|
||||
.markdown li::marker { color: var(--text-ghost); }
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
// cheap to pin: the bare run IS the "storage missing" case.
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { lastProject, rememberProject } from "./util.ts";
|
||||
import { lastProject, rememberProject, resolveWiki } from "./util.ts";
|
||||
|
||||
// Swap globalThis.localStorage for the length of one call, always putting the
|
||||
// original back — later tests in the same process must not inherit a stub.
|
||||
@@ -42,6 +42,42 @@ test("round-trips the last project through storage", () => {
|
||||
);
|
||||
});
|
||||
|
||||
/* The wikilink match matrix. It used to run at click time inside the file
|
||||
view, where a browser was the only way to reach it; the rules did not
|
||||
change when they moved (BEA-136), so this is the whole contract. */
|
||||
const FILES = [
|
||||
{ path: "guide.md", name: "guide.md" },
|
||||
{ path: "notes/readme.md", name: "readme.md" },
|
||||
{ path: "notes/deep/Topic.md", name: "Topic.md" },
|
||||
{ path: "LICENSE", name: "LICENSE" },
|
||||
// Same basename as a file higher up, to pin the path-before-basename rule.
|
||||
{ path: "archive/guide.md", name: "guide.md" },
|
||||
];
|
||||
|
||||
test("wikilink target resolves by path, then basename", () => {
|
||||
const hit = (t: string) => resolveWiki(t, FILES)?.path;
|
||||
assert.equal(hit("notes/readme.md"), "notes/readme.md"); // exact path
|
||||
assert.equal(hit("notes/readme"), "notes/readme.md"); // path, .md implied
|
||||
assert.equal(hit("readme"), "notes/readme.md"); // basename, .md implied
|
||||
assert.equal(hit("readme.md"), "notes/readme.md"); // basename
|
||||
assert.equal(hit("LICENSE"), "LICENSE"); // no extension at all
|
||||
assert.equal(hit("x y"), undefined); // spaces are just characters
|
||||
assert.equal(hit("nowhere"), undefined); // a miss is undefined, not a throw
|
||||
});
|
||||
|
||||
test("wikilink matching is case-insensitive on both sides", () => {
|
||||
const hit = (t: string) => resolveWiki(t, FILES)?.path;
|
||||
assert.equal(hit("GUIDE"), "guide.md");
|
||||
assert.equal(hit("NOTES/DEEP/topic"), "notes/deep/Topic.md");
|
||||
assert.equal(hit("topic.MD"), "notes/deep/Topic.md");
|
||||
});
|
||||
|
||||
test("an exact path beats a basename anywhere else in the tree", () => {
|
||||
assert.equal(resolveWiki("archive/guide", FILES)?.path, "archive/guide.md");
|
||||
// Bare "guide" is ambiguous by basename; the top-level path wins.
|
||||
assert.equal(resolveWiki("guide", FILES)?.path, "guide.md");
|
||||
});
|
||||
|
||||
test("storage that throws is swallowed on both sides", () => {
|
||||
const boom = () => {
|
||||
throw new Error("SecurityError");
|
||||
|
||||
@@ -32,6 +32,21 @@ export function joinPath(dir: string, rel: string): string {
|
||||
return out.join("/");
|
||||
}
|
||||
|
||||
/* Obsidian-style wikilink target -> file. Exact path first, then basename;
|
||||
".md" is optional on both; everything case-insensitive. The rules are the
|
||||
product decision, so they live in one place and get tested without a
|
||||
browser (node --test has no DOM, so this cannot live in FileView). */
|
||||
export function resolveWiki(target: string, files: { path: string; name: string }[]) {
|
||||
const want = target.toLowerCase();
|
||||
return (
|
||||
files.find((f) => f.path.toLowerCase() === want || f.path.toLowerCase() === want + ".md") ||
|
||||
files.find((f) => {
|
||||
const n = f.name.toLowerCase();
|
||||
return n === want || n === want + ".md";
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
/* clipboard copy that never throws on a non-HTTPS origin (where
|
||||
navigator.clipboard is undefined). Returns true on success. */
|
||||
export async function copyText(text: string): Promise<boolean> {
|
||||
|
||||
+1
-1
File diff suppressed because one or more lines are too long
+33
-33
File diff suppressed because one or more lines are too long
@@ -5,10 +5,10 @@
|
||||
<meta name="viewport" content="width=device-width, initial-scale=1">
|
||||
<title>BearDrive</title>
|
||||
<link rel="icon" href="data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 32 32' fill='%23f5a623'><rect x='4' y='4' width='5.6' height='24'/><rect x='11.2' y='4' width='14.4' height='11.2'/><rect x='11.2' y='16.8' width='16.8' height='11.2'/></svg>">
|
||||
<script type="module" crossorigin src="/assets/index-uWNParEm.js"></script>
|
||||
<script type="module" crossorigin src="/assets/index-J4RRWzcZ.js"></script>
|
||||
<link rel="modulepreload" crossorigin href="/assets/_commonjsHelpers-CqkleIqs.js">
|
||||
<link rel="modulepreload" crossorigin href="/assets/mermaid-DQuCJ8Gi.js">
|
||||
<link rel="stylesheet" crossorigin href="/assets/index-oAGwR7z_.css">
|
||||
<link rel="stylesheet" crossorigin href="/assets/index-DpPcGEM4.css">
|
||||
</head>
|
||||
<body>
|
||||
<div id="root"></div>
|
||||
|
||||
Reference in New Issue
Block a user