fix(studio,runtime): CSS.escape ids so digit-leading selectors don't crash

The runtime picker built raw `#${id}` selectors while its sibling
attribute-selector branches (data-composition-id, data-composition-src,
data-track-index) already CSS.escape'd their values. When a user
composition has an element with a digit-leading id (e.g. `id="0"`),
the picker emits the selector `#0` which is invalid per the CSS spec —
downstream `document.querySelector` throws SyntaxError.

Same failure mode reached the Studio thumbnail: getElementScreenshotClip
called `document.querySelectorAll(selector)` unguarded, so an invalid
selector bubbling out of page.evaluate failed the whole thumbnail and
returned 500 to the browser (broken thumbnail image).

Fixes:
- packages/core/src/runtime/picker.ts — CSS.escape the id, matching the
  sibling branches on lines 100/102/104.
- packages/studio-server/src/helpers/screenshotClip.ts — catch
  SyntaxError from an invalid selector and return undefined so the
  caller falls back to a full-page screenshot, so the user still sees
  a thumbnail instead of a broken image.

Regression tests for both.

Reported via #hf-cli-feedback (Slack ts=1784218060, darwin/arm64,
CLI 0.7.60): "digit-leading worker IDs broke Studio thumbnail
querySelectorAll".

— Via
This commit is contained in:
Via
2026-07-16 18:07:58 +00:00
parent 3bb26b0f08
commit 9bbdcc4ec9
4 changed files with 151 additions and 5 deletions
+80 -1
View File
@@ -1,6 +1,37 @@
import { describe, it, expect, vi, afterEach } from "vitest"; import { describe, it, expect, vi, afterEach, beforeAll } from "vitest";
import { createPickerModule } from "./picker"; import { createPickerModule } from "./picker";
// jsdom does not implement CSS.escape — polyfill with a spec-adjacent version.
// (Parallel polyfills already live in compositionLoader.test.ts /
// startResolver.test.ts, but each test file runs in an isolated environment.)
beforeAll(() => {
const css = globalThis.CSS as { escape?: (input: string) => string } | undefined;
if (!css || typeof css.escape !== "function") {
(globalThis as { CSS?: { escape: (input: string) => string } }).CSS = {
...(css ?? {}),
escape: (value: string) => {
let out = "";
for (let i = 0; i < value.length; i += 1) {
const ch = value[i] ?? "";
const code = ch.charCodeAt(0);
const isDigit = code >= 48 && code <= 57;
const isAlpha = (code >= 65 && code <= 90) || (code >= 97 && code <= 122);
const isWordSafe = isAlpha || code === 45 || code === 95 || code >= 128; // - _ non-ASCII
const leadingDigit = i === 0 && isDigit;
if (leadingDigit) {
out += `\\${code.toString(16)} `;
} else if (isDigit || isWordSafe) {
out += ch;
} else {
out += `\\${ch}`;
}
}
return out;
},
};
}
});
function createMockPostMessage() { function createMockPostMessage() {
return vi.fn(); return vi.fn();
} }
@@ -181,4 +212,52 @@ describe("createPickerModule", () => {
expect(document.body.classList.contains("__hf-pick-active")).toBe(true); expect(document.body.classList.contains("__hf-pick-active")).toBe(true);
}); });
}); });
describe("buildElementSelector escapes digit-leading ids", () => {
it('produces a CSS-valid selector for id="0" and picks the element back', () => {
// Regression: a user's HTML with id="0" (or any digit-leading id) used
// to produce the raw selector "#0", which is invalid per the CSS spec —
// downstream querySelector calls threw SyntaxError. buildElementSelector
// now CSS.escapes the id.
const picker = createPickerModule({ postMessage: createMockPostMessage() });
picker.installPickerApi();
const el = document.createElement("div");
el.id = "0";
Object.assign(el.style, {
position: "absolute",
left: "0px",
top: "0px",
width: "40px",
height: "40px",
});
document.body.appendChild(el);
// Force elementsFromPoint to hit our div so we exercise the real code
// path that calls buildElementSelector via extractElementInfo.
const originalElementsFromPoint = document.elementsFromPoint;
Object.defineProperty(document, "elementsFromPoint", {
configurable: true,
value: () => [el],
});
try {
const api = (
window as {
__HF_PICKER_API?: {
pickAtPoint?: (x: number, y: number) => { selector: string } | null;
};
}
).__HF_PICKER_API;
const picked = api?.pickAtPoint?.(10, 10);
expect(picked?.selector).toBe("#\\30 ");
// And the round trip must find the element back through querySelector.
expect(() => document.querySelector(picked?.selector ?? "")).not.toThrow();
expect(document.querySelector(picked?.selector ?? "")).toBe(el);
} finally {
Object.defineProperty(document, "elementsFromPoint", {
configurable: true,
value: originalElementsFromPoint,
});
}
});
});
}); });
+4 -1
View File
@@ -95,7 +95,10 @@ export function createPickerModule(deps: PickerModuleDeps): PickerModule {
function buildElementSelector(el: Element): string { function buildElementSelector(el: Element): string {
const htmlEl = el as HTMLElement; const htmlEl = el as HTMLElement;
if (htmlEl.id) return `#${htmlEl.id}`; // Escape the ID so digit-leading or otherwise CSS-illegal ids (e.g. `#0`,
// `#1`) produce valid selectors — `document.querySelector("#0")` throws
// SyntaxError per the CSS spec. Sibling branches below already escape.
if (htmlEl.id) return `#${CSS.escape(htmlEl.id)}`;
const compositionId = el.getAttribute("data-composition-id"); const compositionId = el.getAttribute("data-composition-id");
if (compositionId) return `[data-composition-id="${CSS.escape(compositionId)}"]`; if (compositionId) return `[data-composition-id="${CSS.escape(compositionId)}"]`;
const compositionSrc = el.getAttribute("data-composition-src"); const compositionSrc = el.getAttribute("data-composition-src");
@@ -0,0 +1,54 @@
import { afterEach, describe, expect, it } from "vitest";
import { getElementScreenshotClip } from "./screenshotClip";
afterEach(() => {
document.body.innerHTML = "";
});
describe("getElementScreenshotClip", () => {
it("returns undefined (not throws) when the selector is CSS-invalid", () => {
// Regression: an HTML element with `id="0"` produces the selector `#0`,
// which is invalid per the CSS spec — `document.querySelectorAll('#0')`
// throws SyntaxError. Puppeteer surfaces that as a page.evaluate error,
// which used to bubble up and fail the whole thumbnail. The clip helper
// now swallows the SyntaxError so callers fall back to a full-page shot.
const el = document.createElement("div");
el.id = "0";
Object.assign(el.style, {
width: "100px",
height: "80px",
});
document.body.appendChild(el);
expect(() => getElementScreenshotClip("#0")).not.toThrow();
expect(getElementScreenshotClip("#0")).toBeUndefined();
});
it("returns undefined (not throws) for garbage selectors", () => {
expect(() => getElementScreenshotClip("::: garbage :::")).not.toThrow();
expect(getElementScreenshotClip("::: garbage :::")).toBeUndefined();
});
it("returns a clip for a well-formed selector matching a visible element", () => {
const el = document.createElement("div");
el.id = "hero";
el.getBoundingClientRect = () =>
({
left: 10,
top: 20,
width: 100,
height: 80,
right: 110,
bottom: 100,
x: 10,
y: 20,
toJSON: () => ({}),
}) as DOMRect;
document.body.appendChild(el);
const clip = getElementScreenshotClip("#hero");
expect(clip).toBeDefined();
expect(clip?.width).toBeGreaterThan(0);
expect(clip?.height).toBeGreaterThan(0);
});
});
@@ -9,9 +9,19 @@ export function getElementScreenshotClip(
selector: string, selector: string,
selectorIndex?: number, selectorIndex?: number,
): ScreenshotClip | undefined { ): ScreenshotClip | undefined {
const matches = Array.from(document.querySelectorAll(selector)).filter( // Guard against invalid CSS selectors (e.g. `#0` — a digit-leading id from
(el): el is HTMLElement => el instanceof HTMLElement, // user HTML that upstream producers forgot to CSS.escape). querySelectorAll
); // throws SyntaxError on those, which bubbles out of page.evaluate and fails
// the whole thumbnail. Returning undefined here falls back to a full-page
// screenshot, so the user still sees a thumbnail instead of a broken image.
let matches: HTMLElement[];
try {
matches = Array.from(document.querySelectorAll(selector)).filter(
(el): el is HTMLElement => el instanceof HTMLElement,
);
} catch {
return undefined;
}
const safeIndex = Math.max(0, Math.min(matches.length - 1, Math.floor(selectorIndex ?? 0))); const safeIndex = Math.max(0, Math.min(matches.length - 1, Math.floor(selectorIndex ?? 0)));
const el = matches[safeIndex] ?? null; const el = matches[safeIndex] ?? null;
if (!(el instanceof HTMLElement)) return undefined; if (!(el instanceof HTMLElement)) return undefined;