diff --git a/apps/server/src/htmlRender/HtmlRender.test.ts b/apps/server/src/htmlRender/HtmlRender.test.ts index 4a6eed34d1eee..40d877018c4e5 100644 --- a/apps/server/src/htmlRender/HtmlRender.test.ts +++ b/apps/server/src/htmlRender/HtmlRender.test.ts @@ -123,6 +123,28 @@ describe("HtmlRender", () => { }).pipe(Effect.provide(layerTest)), ); + it.effect("rejects viewport height units, which make the fitted frame grow without end", () => + Effect.gen(function* () { + const htmlRender = yield* HtmlRender.HtmlRender; + for (const page of [ + "", + "', + ); + yield* htmlRender.prepare( + "

Avoid 100vh here.

", + ); + yield* htmlRender.prepare(``); + }).pipe(Effect.provide(layerTest)), + ); + it.effect("inlines an SVG behind processing instructions and a doctype subset", () => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; diff --git a/apps/server/src/htmlRender/HtmlRender.ts b/apps/server/src/htmlRender/HtmlRender.ts index 41075df56006c..810143c75a796 100644 --- a/apps/server/src/htmlRender/HtmlRender.ts +++ b/apps/server/src/htmlRender/HtmlRender.ts @@ -68,6 +68,34 @@ export class HtmlRenderPageTooLargeError extends Schema.TaggedError()( + "HtmlRenderViewportHeightError", + { found: Schema.String }, +) { + override get message(): string { + return `The page uses a viewport height unit (${this.found}). The frame grows to fit the page, so a size that follows the viewport makes it grow again and again. Let content set the height, or use fixed pixel heights.`; + } +} + +// Only stylesheets and style attributes are checked, so page text that mentions +// a unit passes; comments match first so their contents are skipped. Scripts +// are still checked, since markup they insert applies. An unclosed comment, +// tag, or quote runs to the end of the page, which keeps both patterns linear +// on hostile input. +const CSS_SOURCE = + /|$)|]*(?:>|$)([\s\S]*?)(?:<\/style\s*>|$)|\sstyle\s*=\s*(?:"([^"]*)(?:"|$)|'([^']*)(?:'|$)|([^\s>]+))/gi; +// The lookbehind skips base64 runs, such as an inlined image that happens to contain "1vh". +const VIEWPORT_HEIGHT_UNIT = /(? { + for (const [, sheet, doubleQuoted, singleQuoted, unquoted] of html.matchAll(CSS_SOURCE)) { + const css = sheet ?? doubleQuoted ?? singleQuoted ?? unquoted ?? ""; + const found = VIEWPORT_HEIGHT_UNIT.exec(css)?.[0]; + if (found !== undefined) return Effect.fail(new HtmlRenderViewportHeightError({ found })); + } + return Effect.void; +}; + export class HtmlRenderStoreError extends Schema.TaggedError()( "HtmlRenderStoreError", { cause: Schema.Defect() }, @@ -80,7 +108,8 @@ export class HtmlRenderStoreError extends Schema.TaggedError 0) { return yield* new HtmlRenderImagesNotFoundError({ paths: inlined.missing }); @@ -447,6 +477,7 @@ const make = Effect.gen(function* () { Math.max(MIN_PREVIEW_WIDTH, Math.round(input.width ?? HTML_RENDER_COLUMN_WIDTH)), ); const appearance = input.appearance ?? "dark"; + yield* rejectViewportHeight(input.html); const inlined = yield* inline(input.html); const executable = yield* previewBrowser.executable; const theme = htmlRenderTheme( diff --git a/apps/server/src/mcp/toolkits/html/handlers.ts b/apps/server/src/mcp/toolkits/html/handlers.ts index 9ef61c1fcfab1..edd7146f81d00 100644 --- a/apps/server/src/mcp/toolkits/html/handlers.ts +++ b/apps/server/src/mcp/toolkits/html/handlers.ts @@ -10,6 +10,7 @@ const INVALID_PAGE_ERRORS = new Set([ "HtmlRenderImagesNotFoundError", "HtmlRenderImageTooLargeError", "HtmlRenderPageTooLargeError", + "HtmlRenderViewportHeightError", ]); const toFailure = (error: { readonly _tag: string; readonly message: string }) => diff --git a/apps/server/src/mcp/toolkits/html/tools.ts b/apps/server/src/mcp/toolkits/html/tools.ts index 4329ef5ab9e85..24859c5762886 100644 --- a/apps/server/src/mcp/toolkits/html/tools.ts +++ b/apps/server/src/mcp/toolkits/html/tools.ts @@ -2,6 +2,7 @@ import { OrchestratorMcpFailure } from "@t3tools/contracts"; import { HTML_RENDER_COLUMN_WIDTH, HTML_RENDER_LAYOUT_GUIDE, + HTML_RENDER_MAX_FIT_HEIGHT, HTML_RENDER_MAX_HEIGHT, HTML_RENDER_MAX_TITLE_LENGTH, HTML_RENDER_MIN_HEIGHT, @@ -71,7 +72,7 @@ export const HtmlPreviewTool = Tool.make("html_preview", { // Read-only in the MCP sense: it shows a page in the caller's own thread and // touches no workspace, so plan mode and read-only sandboxes can use it. const HtmlRenderTool = Tool.make(HTML_RENDER_TOOL_NAME, { - description: `Show a finished HTML page (chart, table, diagram, collage, mockup) inline in this thread, above your final text reply; call it before writing that reply. The reader already sees the page, so the reply should not announce it, say where it is, or restate it: add only what the page doesn't say. Preview with html_preview first. T3 fits the frame to the page's height at each reader's width. A height below the page's contentHeight caps the frame there, and the rest scrolls inside it. ${PAGE_RULES} ${HTML_RENDER_LAYOUT_GUIDE} ${HTML_RENDER_THEME_GUIDE}`, + description: `Show a finished HTML page (chart, table, diagram, collage, mockup) inline in this thread, above your final text reply; call it before writing that reply. The reader already sees the page, so the reply should not announce it, say where it is, or restate it: add only what the page doesn't say. Preview with html_preview first. T3 fits the frame to the page's height at each reader's width. On mobile, a height below the page's contentHeight caps the frame there, and the rest scrolls inside it; web and desktop fit the whole page, up to ${HTML_RENDER_MAX_FIT_HEIGHT}px. ${PAGE_RULES} ${HTML_RENDER_LAYOUT_GUIDE} ${HTML_RENDER_THEME_GUIDE}`, parameters: Schema.Struct({ html: Html, title: Schema.String.check( @@ -79,7 +80,7 @@ const HtmlRenderTool = Tool.make(HTML_RENDER_TOOL_NAME, { Schema.isMaxLength(HTML_RENDER_MAX_TITLE_LENGTH), ).annotate({ description: "Short name for the page." }), height: Schema.Int.annotate({ - description: `The frame height in CSS pixels, ${HTML_RENDER_MIN_HEIGHT}-${HTML_RENDER_MAX_HEIGHT}. Use html_preview's contentHeight, or less to make long content scroll inside the frame.`, + description: `The frame height in CSS pixels, ${HTML_RENDER_MIN_HEIGHT}-${HTML_RENDER_MAX_HEIGHT}. Use html_preview's contentHeight. A lower height makes long content scroll inside the frame on mobile only.`, }), }), success: Schema.Struct({ diff --git a/apps/web/src/components/chat/HtmlRenderFrame.browser.test.tsx b/apps/web/src/components/chat/HtmlRenderFrame.browser.test.tsx new file mode 100644 index 0000000000000..55e8a627690ca --- /dev/null +++ b/apps/web/src/components/chat/HtmlRenderFrame.browser.test.tsx @@ -0,0 +1,63 @@ +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, expect, it, vi } from "vite-plus/test"; +import { EnvironmentId } from "@t3tools/contracts"; +import { + htmlRenderTheme, + HTML_RENDER_DEFAULT_FONTS, + injectHtmlRenderBootstrap, +} from "@t3tools/shared/htmlRender"; +import { T3_CODE_DARK_THEME_COLORS } from "@t3tools/shared/themePalettes"; + +import "~/index.css"; + +import { HtmlRenderFrame } from "./HtmlRenderFrame"; + +const page = vi.hoisted(() => ({ url: "" })); +vi.mock(import("~/assets/assetUrls"), async (original) => ({ + ...(await original()), + useAssetUrlState: () => ({ + _tag: "Success", + url: page.url, + expiresAt: Date.now() + 60 * 60_000, + }), + useAssetUrlRefresh: () => vi.fn(), +})); +vi.mock(import("~/hooks/useHtmlRenderTheme"), async (original) => ({ + ...(await original()), + useHtmlRenderTheme: () => + htmlRenderTheme(T3_CODE_DARK_THEME_COLORS, "dark", HTML_RENDER_DEFAULT_FONTS), +})); + +let root: Root | undefined; +let container: HTMLDivElement | undefined; + +afterEach(async () => { + await act(() => root?.unmount()); + container?.remove(); + URL.revokeObjectURL(page.url); +}); + +it("grows past the agent's height to fit the page the client lays out", async () => { + page.url = URL.createObjectURL( + new Blob([injectHtmlRenderBootstrap('
')], { + type: "text/html", + }), + ); + container = document.createElement("div"); + document.body.append(container); + root = createRoot(container); + await act(() => + root!.render( + , + ), + ); + + const box = container.firstElementChild as HTMLElement; + await expect.poll(() => box.style.height).toBe("2345px"); + expect(box.querySelector("iframe")?.clientHeight).toBe(2345); +}); diff --git a/packages/shared/src/htmlRender.test.ts b/packages/shared/src/htmlRender.test.ts index fc45dc1f4b871..e2814ab52b6b5 100644 --- a/packages/shared/src/htmlRender.test.ts +++ b/packages/shared/src/htmlRender.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vite-plus/test"; import { - HTML_RENDER_MAX_HEIGHT, + HTML_RENDER_MAX_FIT_HEIGHT, htmlRenderFrameHeight, htmlRenderReferencesEqual, htmlRenderTheme, @@ -267,18 +267,22 @@ describe("htmlRenderFrameHeight", () => { const fitted = { ...measured, height: 1403 }; expect(htmlRenderFrameHeight(fitted, 728, 1415)).toBe(1415); expect(htmlRenderFrameHeight(fitted, 1400)).toBe(1660); - expect(htmlRenderFrameHeight(fitted, 728, 5000)).toBe(HTML_RENDER_MAX_HEIGHT); + expect(htmlRenderFrameHeight(fitted, 728, 5000)).toBe(5000); + expect(htmlRenderFrameHeight(fitted, 728, 99_999)).toBe(HTML_RENDER_MAX_FIT_HEIGHT); }); - it("keeps the agent's height when it asked for a scrolling frame or the page is unmeasured", () => { + it("fits the client's measured content over a shorter agent height", () => { const scrolling = { ...measured, height: 600 }; - expect(htmlRenderFrameHeight(scrolling, 728, 1415)).toBe(600); - expect(htmlRenderFrameHeight(scrolling, 1400)).toBe(600); - expect(htmlRenderFrameHeight(reference, 728)).toBe(reference.height); - expect(htmlRenderFrameHeight(reference, 728, 900)).toBe(reference.height); + expect(htmlRenderFrameHeight(scrolling, 728, 1415)).toBe(1415); + expect(htmlRenderFrameHeight(reference, 728, 900)).toBe(900); expect(htmlRenderFrameHeight(reference, 728, 300)).toBe(300); }); + it("keeps the agent's height without a client measurement", () => { + expect(htmlRenderFrameHeight({ ...measured, height: 600 }, 1400)).toBe(600); + expect(htmlRenderFrameHeight(reference, 728)).toBe(reference.height); + }); + it("drops a malformed table and compares tables by value", () => { expect(readHtmlRenderReference({ ...reference, heights: [[728, "x"]] })?.heights).toBe( undefined, diff --git a/packages/shared/src/htmlRender.ts b/packages/shared/src/htmlRender.ts index b27bb09e97a4a..cb01eab9601df 100644 --- a/packages/shared/src/htmlRender.ts +++ b/packages/shared/src/htmlRender.ts @@ -9,6 +9,9 @@ import { EVENT_CODE_SHORTCUT_KEYS } from "./keybindings.ts"; export const HTML_RENDER_TOOL_NAME = "html_render"; export const HTML_RENDER_MIN_HEIGHT = 80; export const HTML_RENDER_MAX_HEIGHT = 2000; +// A client fits the frame to the live page up to this height. It only stops a +// page that grows with its frame, since vh units are rejected at publish. +export const HTML_RENDER_MAX_FIT_HEIGHT = 20_000; export const HTML_RENDER_MAX_TITLE_LENGTH = 200; export interface HtmlRenderReference { @@ -90,22 +93,30 @@ function measuredHeight(heights: NonNullable, wi /** * A page even a few pixels taller than its frame scrolls inside it and takes - * the reader's scroll, so the frame fits the page. + * the reader's scroll, so the frame fits the page. A client that measured the + * live page passes `contentHeight`, which wins over the agent's height: client + * fonts often wrap the page taller than the server or agent measured it. */ export function htmlRenderFrameHeight( reference: HtmlRenderReference, width: number, contentHeight?: number, ) { + if (contentHeight !== undefined) { + return Math.min( + HTML_RENDER_MAX_FIT_HEIGHT, + Math.max(HTML_RENDER_MIN_HEIGHT, Math.ceil(contentHeight)), + ); + } const heights = reference.heights; if (heights === undefined || heights.length === 0) { - return clampHtmlRenderHeight(Math.min(reference.height, contentHeight ?? reference.height)); + return clampHtmlRenderHeight(reference.height); } const cap = measuredHeight(heights, HTML_RENDER_COLUMN_WIDTH) > reference.height ? reference.height : HTML_RENDER_MAX_HEIGHT; - return clampHtmlRenderHeight(Math.min(cap, contentHeight ?? measuredHeight(heights, width))); + return clampHtmlRenderHeight(Math.min(cap, measuredHeight(heights, width))); } export function htmlRenderFileName(title: string) { @@ -234,7 +245,7 @@ export const HTML_RENDER_LAYOUT_GUIDE = [ "Use a fluid width with no horizontal padding on the outermost element, and no outer card, border, or banner title: the page is part of your reply.", "If a box needs its own background (a mock of a specific screen, a panel that must stand apart), give it at least 16px of padding on every side and var(--radius) corners, so content never touches its edge.", "Give charts fixed pixel heights rather than heights that scale with width.", - "Let content set the page's height. Avoid viewport-based heights such as 100vh or height:100% on html or body; the frame grows to fit the page, so they can make it grow again and again.", + "Let content set the page's height. The frame grows to fit the page, so a height that follows the viewport makes it grow again and again: html_preview and html_render reject vh, svh, lvh, and dvh units, and height:100% on html or body has the same problem.", ].join(" "); const HOST_CONTEXT_CHANGED_METHOD = "ui/notifications/host-context-changed";