diff --git a/temp-screenshots/root-relative-image/after-dark.png b/temp-screenshots/root-relative-image/after-dark.png new file mode 100644 index 00000000000..b64fd3d24c7 Binary files /dev/null and b/temp-screenshots/root-relative-image/after-dark.png differ diff --git a/temp-screenshots/root-relative-image/after-light.png b/temp-screenshots/root-relative-image/after-light.png new file mode 100644 index 00000000000..b0ccdab928c Binary files /dev/null and b/temp-screenshots/root-relative-image/after-light.png differ diff --git a/temp-screenshots/root-relative-image/before-dark.png b/temp-screenshots/root-relative-image/before-dark.png new file mode 100644 index 00000000000..6831f603f15 Binary files /dev/null and b/temp-screenshots/root-relative-image/before-dark.png differ diff --git a/website/capture/root-relative-image.html b/website/capture/root-relative-image.html new file mode 100644 index 00000000000..6eaa28c07e5 --- /dev/null +++ b/website/capture/root-relative-image.html @@ -0,0 +1,11 @@ + + + + + Root-relative markdown image capture + + +
+ + + diff --git a/website/capture/root-relative-image.tsx b/website/capture/root-relative-image.tsx new file mode 100644 index 00000000000..03a40b1bbeb --- /dev/null +++ b/website/capture/root-relative-image.tsx @@ -0,0 +1,61 @@ +/** + * Isolated capture entry for root-relative images in a previewed markdown file + * (issue #23623). + * + * The real MarkdownRenderer renders inside the same BasePathCtx the file + * preview (ContentRenderer) provides, for a file at `?file=`. The loads + * hit /api/file-raw, which the shoot script answers from a real directory on + * disk: a file that exists is served, any other path 404s, the same contract as + * the backend endpoint. So which candidate loads is decided by the filesystem, + * not by the entry. + * + * Theme comes from the query string: ?theme=dark|light + */ +import { createRoot } from 'react-dom/client' + +import { initI18n } from '../src/i18n/all' +import MarkdownRenderer, { BasePathCtx } from '../src/components/MarkdownRenderer' +import '../src/index.css' + +const params = new URLSearchParams(location.search) +const theme = params.get('theme') || 'dark' +const file = params.get('file') || '' + +document.documentElement.setAttribute('data-theme', theme === 'light' ? 'kiro-light' : 'kiro-dark') + +// The broken-image chip's HEAD probe goes through fetch, which page.route does +// not see for a same-origin dev-server request in every Chromium build; answer +// it the way the endpoint would for a path that does not exist. +const realFetch = globalThis.fetch.bind(globalThis) +globalThis.fetch = ((input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url + if (url.startsWith('/api/file-raw') && init?.method === 'HEAD') { + return Promise.resolve(new Response(null, { status: 404 })) + } + return realFetch(input, init) +}) as typeof globalThis.fetch + +const MD = `## Site-root image + +Written the way GitHub, Zenn and Docusaurus expect, from the root of the repository: + +![Architecture diagram](/images/post/architecture.png) + +## Image that is really missing + +![Screenshot that was never committed](/images/post/missing.png) +` + +async function main() { + initI18n('en') + const root = createRoot(document.getElementById('root')!) + root.render( +
+ + + +
, + ) +} + +void main() diff --git a/website/capture/shoot-root-relative-image.mjs b/website/capture/shoot-root-relative-image.mjs new file mode 100644 index 00000000000..d20f52e7f98 --- /dev/null +++ b/website/capture/shoot-root-relative-image.mjs @@ -0,0 +1,74 @@ +/** + * Screenshot harness for root-relative images in a previewed markdown file + * (issue #23623). + * + * Builds a throwaway repository on disk — `articles/post.md` plus + * `images/post/architecture.png` at the repository root, the Zenn/GitHub + * layout — then serves capture/root-relative-image.html from a programmatic + * Vite dev server on loopback. Every /api/file-raw request is answered from + * the real filesystem: an existing file is served, anything else is a 404, + * the same contract as the backend endpoint. Each requested path is printed, + * so the log shows the order the candidates were tried in. + * + * Usage (from website/): node capture/shoot-root-relative-image.mjs [outDir] [prefix] + */ +import { chromium } from 'playwright' +import { createServer } from 'vite' +import { copyFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, statSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { fileURLToPath } from 'node:url' +import { dirname, join, resolve } from 'node:path' +import { chromiumExecutable } from '../scripts/lib/chromium-executable.mjs' + +const here = dirname(fileURLToPath(import.meta.url)) +const OUT = process.argv[2] || resolve(here, '../../temp-screenshots/root-relative-image') +const PREFIX = process.argv[3] || 'after' +mkdirSync(OUT, { recursive: true }) + +const repo = mkdtempSync(join(tmpdir(), 'root-relative-image-')) +mkdirSync(join(repo, 'articles'), { recursive: true }) +mkdirSync(join(repo, 'images/post'), { recursive: true }) +writeFileSync(join(repo, 'articles/post.md'), '![Architecture diagram](/images/post/architecture.png)\n') +copyFileSync(resolve(here, '../public/icon-512.png'), join(repo, 'images/post/architecture.png')) +const file = join(repo, 'articles/post.md') + +const server = await createServer({ + configFile: './vite.config.ts', + server: { host: '127.0.0.1', port: 5199, strictPort: false }, + logLevel: 'warn', +}) +await server.listen() +const base = server.resolvedUrls?.local?.[0]?.replace(/\/$/, '') || `http://127.0.0.1:${server.config.server.port}` + +const browser = await chromium.launch({ + executablePath: chromiumExecutable(), + env: { ...process.env, LD_LIBRARY_PATH: '' }, +}) +const page = await browser.newPage({ viewport: { width: 760, height: 900 }, deviceScaleFactor: 2 }) +page.on('pageerror', e => console.log('PAGE EXCEPTION:', e.message)) +await page.route('**/api/file-raw*', route => { + const path = new URL(route.request().url()).searchParams.get('path') || '' + const hit = existsSync(path) && statSync(path).isFile() + console.log(`${hit ? '200' : '404'} ${path}`) + return hit + ? route.fulfill({ status: 200, contentType: 'image/png', body: readFileSync(path) }) + : route.fulfill({ status: 404, contentType: 'application/json', body: '{"error":"not found"}' }) +}) + +for (const theme of ['dark', 'light']) { + await page.goto(`${base}/capture/root-relative-image.html?theme=${theme}&file=${encodeURIComponent(file)}`) + // Settled when the site-root image has loaded (or been replaced by the chip) + // and the missing one shows its chip. + await page.waitForFunction(() => { + const imgs = [...document.querySelectorAll('img')] + const pending = imgs.some(i => !i.complete) + return !pending && document.body.textContent?.includes('Screenshot that was never committed') + && document.body.textContent?.match(/Image (file no longer exists|failed to load)/g) + }) + const shot = join(OUT, `${PREFIX}-${theme}.png`) + await page.locator('#root > div').screenshot({ path: shot }) + console.log('wrote', shot) +} + +await browser.close() +await server.close() diff --git a/website/docs/frontend-conventions.md b/website/docs/frontend-conventions.md index 3b0e944a8c6..eac15ab5faf 100644 --- a/website/docs/frontend-conventions.md +++ b/website/docs/frontend-conventions.md @@ -601,6 +601,25 @@ payloads. Widening that regex or its key scope is a security change — the same constant also decides which paths are treated as local file reads, so the two decisions must stay on the one exported copy in `urlTransform.ts`. +A root-relative image (`/images/a.png`) inside a previewed file (a +`BasePathCtx` is set) is tried as an absolute path first, then against each +directory above the file, nearest first, up to eight levels +(`siteRootImageCandidates`). The leading `/` there usually names the root of the +repository or site the markdown belongs to — GitHub, Zenn and Docusaurus all +write images that way (issue #23623) — and nothing in the file says where that +root is. A same-named image in a nearer directory therefore wins over one at +the real root. Every candidate still goes through `/api/file-raw`, so the fallback +reads nothing the absolute path could not already ask for. When every candidate +fails, the broken-image fallback names and probes the path as written, as it +does for any other image, so its "no longer exists" wording speaks for that path +alone. That fallback states the failure through an inline `ErrorNotice` beside +a chip that copies the on-disk path. The notice offers no agent hand-off: the +renderer also previews unsaved drafts (the meeting note's Preview), and the +navigation would unmount one. Every surface that +provides `BasePathCtx` gets this (the file preview, the file explorer, the +meeting notes sidebar); outside them (chat, artifacts) a leading-slash path +stays absolute only. + To SHOW a gateway path's file name, use `pathBasename` (and `stripTrailingSeparators` for a directory) from `utils/pathBasename.ts`, never `.split('/').pop()`: a Windows gateway reports `C:\repo\a.ts`, which a `/`-only diff --git a/website/src/components/markdown/ImgWithFallback.tsx b/website/src/components/markdown/ImgWithFallback.tsx index 1e854e8c44a..4ce96c7302a 100644 --- a/website/src/components/markdown/ImgWithFallback.tsx +++ b/website/src/components/markdown/ImgWithFallback.tsx @@ -4,6 +4,7 @@ import { Check, Copy, Image as ImageIcon, ImageOff } from 'lucide-react' import { getImageDims, rememberImageDims } from '../../utils/imageDims' import { WINDOWS_ABS_PATH_RE, decodeLocalPath } from '../../utils/urlTransform' import { i18nT } from '../../i18n/t' +import ErrorNotice from '../ErrorNotice' import { BasePathCtx, CompactImagesCtx, ImageVersionCtx, MdSourceCtx } from './contexts' import { useTitleCuedCopy } from './copyFeedback' import { basenameOf } from './pathReferences' @@ -27,10 +28,10 @@ import { dispatchLightbox } from './Lightbox' * Chat images are read from disk at VIEW time (`/api/file-raw`), not stored in * the message — so the dominant failure is a local file that no longer exists * (a screenshot written to a temp directory that has since been cleaned), long - * after the message rendered fine for its author. The chip names that - * condition, and the whole chip is click-to-copy for the on-disk path: - * recovery starts from knowing WHICH file is gone, and the path is the one - * thing the transcript still holds. + * after the message rendered fine for its author. The failure is stated by an + * inline `ErrorNotice`, and beside it a chip names + * the image and copies its on-disk path: recovery starts from knowing WHICH + * file is gone, and the path is the one thing the transcript still holds. * * The `` error event carries no status, so "file no longer exists" is * NOT asserted from the error alone — a backend hiccup, a sensitive-path @@ -71,26 +72,37 @@ function BrokenImage({ path, alt, probeUrl }: { path: string; alt?: string; prob const idle = alt ? `${path}\n${i18nT('components.markdownRenderer.click_to_copy')}` : i18nT('components.markdownRenderer.click_to_copy') + const failure = confirmedGone + ? i18nT('components.markdownRenderer.image_file_no_longer_exists') + : i18nT('components.markdownRenderer.image_failed_to_load') + // The load failure renders through ErrorNotice like every other failure + // (errors-use-error-notice); the chip beside it keeps the one recovery the + // transcript itself can offer, copying the on-disk path. return ( <> - { if (e.key === 'Enter' || e.key === ' ') handleCopy(e) }} - title={copied ? i18nT('components.markdownRenderer.copied') : idle} - > -