fix(builder): include iframe padding in the frame's content origin

.changeset/frame-content-origin-padding.md

@@ -0,0 +1,29 @@
+---
+"nextly": patch
+"create-nextly-app": patch
+"@nextlyhq/admin": patch
+"@nextlyhq/admin-css": patch
+"@nextlyhq/blocks-engine": patch
+"@nextlyhq/blocks-react": patch
+"@nextlyhq/ui": patch
+"@nextlyhq/adapter-drizzle": patch
+"@nextlyhq/adapter-postgres": patch
+"@nextlyhq/adapter-mysql": patch
+"@nextlyhq/adapter-sqlite": patch
+"@nextlyhq/storage-s3": patch
+"@nextlyhq/storage-uploadthing": patch
+"@nextlyhq/storage-vercel-blob": patch
+"@nextlyhq/plugin-form-builder": patch
+"@nextlyhq/plugin-page-builder": patch
+"@nextlyhq/plugin-seo": patch
+"@nextlyhq/plugin-sdk": patch
+"@nextlyhq/eslint-config": patch
+"@nextlyhq/prettier-config": patch
+"@nextlyhq/telemetry": patch
+"@nextlyhq/tsconfig": patch
+"@nextlyhq/builder": patch
+---
+
+Correct the frame content origin to include the iframe's padding, and measure that inset in one place.
+
+An iframe's nested viewport begins at the content box, so padding displaces it exactly as a border does. Callers built the inset from `clientLeft`/`clientTop`, which report the border alone, so every frame-local point mapped toward the border by the scaled padding. `frameInsetOf` is now exported as the single reader, and both the README recipe and the `FrameGeometry` documentation name it instead of restating arithmetic three call sites had already got wrong.

e2e/tests/canvas/coordinate-mapping.spec.ts

@@ -18,6 +18,7 @@ import { expect, test } from "@playwright/test";
 import { FLAT_LIST_FIXTURE, seedPage } from "./fixtures";
 import {
   frameContentOrigin,
+  frameInsetOf,
   mapFramePointToHost,
   mapFrameRectToHost,
   mapHostPointToFrame,
@@ -77,8 +78,11 @@ async function mapped(
   // The scale is passed through rather than measured so that `mapped(page, 1)`
   // stays a coherent "what a naive implementation computes" — it gets the wrong
   // origin AND the wrong mapping, which is what the control below asserts.
+  // The same shared reader the driver uses, passed into the page. Border AND
+  // padding: the nested viewport begins at the content box, so both displace
+  // it, and `clientLeft` alone is the reading that looks complete.
   const inset = await frameElement.evaluate<FrameInset, HTMLIFrameElement>(
-    el => ({ left: el.clientLeft, top: el.clientTop })
+    frameInsetOf
   );
 
   const contentOrigin =
@@ -207,7 +211,7 @@ test("point 5: the mapping survives scroll and scale together", async ({
   expect(delta).toBeLessThanOrEqual(1);
 });
 
-test("point 5: the mapping survives a bordered frame under scale", async ({
+test("point 5: the mapping survives a bordered, padded frame under scale", async ({
   page,
   request,
 }) => {
@@ -219,23 +223,29 @@ test("point 5: the mapping survives a bordered frame under scale", async ({
   // where the content origin and the border-box corner are the same point. That
   // makes them all agree whether or not the inset is scaled — so none of them
   // can see this, and the fault would ship on the first bordered canvas.
-  await page
-    .locator("iframe")
-    .evaluate(
-      (el: HTMLIFrameElement) => (el.style.border = "8px solid transparent")
-    );
+  //
+  // Padding as well as a border, because the nested viewport begins at the
+  // CONTENT box: padding displaces it exactly as a border does, and a case
+  // setting only a border passes whether or not padding is accounted for.
+  await page.locator("iframe").evaluate((el: HTMLIFrameElement) => {
+    el.style.border = "8px solid transparent";
+    el.style.padding = "12px";
+  });
   await driver.setZoom(0.5);
 
   // Precondition, not decoration. If the canvas stylesheet wins over the inline
-  // border, `clientLeft` is 0, this silently becomes the at-rest case, and it
+  // style, the inset is 0, this silently becomes the at-rest case, and it
   // passes while testing nothing at all.
-  const inset = await page
+  const applied = await page
     .locator("iframe")
-    .evaluate((el: HTMLIFrameElement) => el.clientLeft);
+    .evaluate((el: HTMLIFrameElement) => ({
+      border: el.clientLeft,
+      padding: parseFloat(getComputedStyle(el).paddingLeft || "0"),
+    }));
   expect(
-    inset,
-    "the border must actually apply for this to test anything"
-  ).toBe(8);
+    applied,
+    "the border and padding must actually apply for this to test anything"
+  ).toEqual({ border: 8, padding: 12 });
 
   const scaled = worstDelta(await mapped(page, 0.5), await groundTruth(page));
   const raw = worstDelta(
@@ -244,13 +254,13 @@ test("point 5: the mapping survives a bordered frame under scale", async ({
   );
 
   test.info().annotations.push({
-    type: "delta-bordered-scaled",
-    description: `scaled=${scaled} raw=${raw} inset=${inset}`,
+    type: "delta-bordered-padded-scaled",
+    description: `scaled=${scaled} raw=${raw} border=${applied.border} padding=${applied.padding}`,
   });
 
-  // Both halves, for the same reason as the scale test. The first says scaling
-  // the inset is right; the second says it MATTERS — an 8px border at 50% puts
-  // the raw sum 4px out, so a regression to it cannot pass this quietly.
+  // Both halves, for the same reason as the scale test. The first says the
+  // full inset scaled is right; the second says it MATTERS — 20px of inset at
+  // 50% puts the raw sum 10px out, so a regression cannot pass this quietly.
   expect(scaled).toBeLessThanOrEqual(1);
   expect(raw).toBeGreaterThan(1);
 });

e2e/tests/canvas/coordinate-mapping.ts

@@ -23,6 +23,7 @@
  */
 import {
   frameContentOrigin,
+  frameInsetOf,
   pointToCanvas,
   pointToHost,
   rectToHost,
@@ -78,4 +79,4 @@ export function mapFrameRectToHost(
  * site is how one of them ends up adding the inset unscaled. The measurement
  * stays in the driver; the sums stay in the editor's module.
  */
-export { frameContentOrigin, type FrameInset };
+export { frameContentOrigin, frameInsetOf, type FrameInset };

e2e/tests/canvas/poc-driver.ts

@@ -9,6 +9,7 @@ import { gotoAdmin } from "../support/admin";
 
 import {
   frameContentOrigin,
+  frameInsetOf,
   mapFramePointToHost,
   mapFrameRectToHost,
   type FrameInset,
@@ -184,14 +185,22 @@ export function createPocDriver(page: Page): CanvasDriver {
       // border has that gap from the first render, and it reads as "the
       // indicator feels slightly off" rather than as a fault.
       //
-      // Measured here, converted there. `clientLeft` is in the frame's own
-      // untransformed pixels while the box is post-transform, so the two cannot
-      // be added without the scale, and doing that sum at the call site is what
-      // put the same error in two files.
-      const inset = await frame.evaluate<FrameInset, HTMLIFrameElement>(el => ({
-        left: el.clientLeft,
-        top: el.clientTop,
-      }));
+      // Border AND padding. The nested viewport begins at the CONTENT box, so
+      // padding displaces it exactly as a border does — and `clientLeft` reports
+      // only the border, which is the reading that looks complete and is not.
+      //
+      // Measured here, converted there. These are the frame's own untransformed
+      // pixels while the box is post-transform, so the two cannot be added
+      // without the scale, and doing that sum at the call site is what put the
+      // same error in several files.
+      // The shared reader, PASSED INTO the page rather than called here.
+      // `evaluate` serializes the function and runs it in the browser, so this
+      // works only because `frameInsetOf` closes over nothing — it reads the
+      // element and globals and no module scope. That is what lets one
+      // definition serve both the editor and this harness.
+      const inset = await frame.evaluate<FrameInset, HTMLIFrameElement>(
+        frameInsetOf
+      );
       return frameContentOrigin(box, inset, await frameScale());
     },
 

packages/builder/README.md

@@ -90,12 +90,18 @@ Scroll inside the frame is deliberately not a field: a rectangle read from
 inside is already relative to the frame's viewport, so subtracting its scroll
 would count it twice.
 
-`frameContentOrigin(borderBox, inset, scale)` — build that origin from what the
-DOM reports. `getBoundingClientRect` gives the BORDER box while the inset
-(`clientLeft`/`clientTop`) is in the frame's own untransformed pixels, so the
-inset has to be scaled before it is added. Getting that wrong misplaces every
-overlay by `(1 - scale) * inset`, which is zero at 100% and therefore invisible
-in the state a canvas is developed in.
+`frameInsetOf(iframe)` — measure the inset. Border AND padding, because an
+iframe's nested viewport begins at the CONTENT box: `clientLeft`/`clientTop`
+report only the border, and a padded frame displaces the viewport further.
+Provided as a function rather than as a recipe because the recipe was
+documented and three call sites still got it wrong.
+
+`frameContentOrigin(borderBox, inset, scale)` — build the origin from that
+inset and the frame's measured box. `getBoundingClientRect` gives the BORDER
+box while the inset is in the frame's own untransformed pixels, so the inset
+has to be scaled before it is added. Getting that wrong misplaces every overlay
+by `(1 - scale) * inset`, which is zero at 100% and therefore invisible in the
+state a canvas is developed in.
 
 `pointToHost` / `pointToCanvas` — a point across the frame, in either
 direction. Exact inverses rather than two mappings written to match, because

packages/builder/src/geometry-dom.ts

@@ -0,0 +1,46 @@
+/**
+ * The one place a frame's inset is READ from the DOM.
+ *
+ * `geometry.ts` deliberately takes plain numbers so the mapping can be
+ * exercised without a browser. That leaves one question it cannot answer, and
+ * it is the question every caller gets wrong: which measurements add up to the
+ * offset between a frame's border box and its content viewport.
+ *
+ * Answering it in prose did not work. The contract was documented as
+ * `clientLeft`/`clientTop`, three call sites followed that recipe, and all
+ * three were wrong by the padding — because an iframe's nested viewport begins
+ * at the CONTENT box, so padding displaces it exactly as a border does. A
+ * recipe a caller applies is a recipe a caller can misapply; a function they
+ * call is not.
+ *
+ * Kept in its own module rather than folded into `geometry.ts` so that the
+ * arithmetic stays testable without a DOM. This is the edge; that is the pure
+ * part.
+ *
+ * @module geometry-dom
+ */
+
+import type { FrameInset } from "./geometry";
+
+/**
+ * How far a frame's content viewport sits inside its border box.
+ *
+ * Border plus padding, in the frame's own untransformed CSS pixels — which is
+ * what {@link FrameInset} means and what `frameContentOrigin` scales.
+ *
+ * `clientLeft`/`clientTop` report the border alone. Padding comes from the
+ * computed style because there is no element property that carries it, and a
+ * non-pixel value (a percentage, `auto`) resolves to pixels there too.
+ */
+export function frameInsetOf(frame: HTMLIFrameElement): FrameInset {
+  const style = frame.ownerDocument.defaultView?.getComputedStyle(frame);
+  // A frame detached from its document has no view and therefore no computed
+  // padding. Its border is still readable, so report what is knowable rather
+  // than guessing a padding that would silently displace every mapped point.
+  const paddingLeft = style === undefined ? 0 : parseFloat(style.paddingLeft);
+  const paddingTop = style === undefined ? 0 : parseFloat(style.paddingTop);
+  return {
+    left: frame.clientLeft + (Number.isFinite(paddingLeft) ? paddingLeft : 0),
+    top: frame.clientTop + (Number.isFinite(paddingTop) ? paddingTop : 0),
+  };
+}

packages/builder/src/geometry.ts

@@ -56,9 +56,11 @@ export interface Rect {
  * `origin` is where the frame's CONTENT viewport lands in host coordinates, and
  * the word content is load-bearing. `getBoundingClientRect` on an iframe reports
  * its BORDER box, while every rectangle read inside the frame is relative to the
- * content viewport — so on a frame with any border the two differ by
- * `clientLeft`/`clientTop`, and an overlay built from the border box sits a
- * couple of scaled pixels out at every point. A canvas that never sets
+ * content viewport — so on a frame with any border or padding the two differ,
+ * and an overlay built from the border box sits a couple of scaled pixels out
+ * at every point. Measure that difference with {@link frameInsetOf}; the inset
+ * is border PLUS padding, and `clientLeft`/`clientTop` alone are short by the
+ * padding. A canvas that never sets
  * `border: none` gets the browser default and the fault is present from the
  * first render, which is exactly the sort of near-miss that reads as "the
  * indicator feels slightly off" rather than as a bug.
@@ -117,10 +119,16 @@ function assertUsable(frame: FrameGeometry): void {
 /**
  * How far the frame's content viewport sits inside its border box.
  *
- * `clientLeft` and `clientTop` exactly as the DOM reports them, which is the
- * whole reason this type exists rather than the caller passing two numbers: they
- * are CSS pixels in the FRAME's own untransformed space. Every other coordinate
- * in this module is host space. Mixing the two is the mistake below.
+ * The COMPLETE offset, border plus padding. A border alone is the tempting
+ * reading, because `clientLeft`/`clientTop` report exactly that and are the
+ * obvious things to reach for — but an iframe's nested viewport begins at the
+ * content box, so padding displaces it too. Measured in Chromium: an 8px border
+ * with 12px padding puts the content 20px in, while `clientLeft` reports 8.
+ *
+ * In CSS pixels of the FRAME's own untransformed space, which is the whole
+ * reason this type exists rather than the caller passing two numbers. Every
+ * other coordinate in this module is host space, and mixing the two is the
+ * mistake {@link frameContentOrigin} exists to prevent.
  */
 export interface FrameInset {
   readonly left: number;
@@ -131,7 +139,7 @@ export interface FrameInset {
  * Where the frame's content viewport starts, in host coordinates.
  *
  * `borderBox` is the corner `getBoundingClientRect` reports for the frame
- * element, and `inset` is its border width. The border is laid out in the
+ * element, and `inset` is its border-to-content offset. That offset is laid out in the
  * frame's own pixels, so a host that has scaled the frame scales the border with
  * everything else: at 50% a 2px border occupies 1 host pixel. Adding the inset
  * unscaled therefore misplaces the origin by `(1 - scale) * inset`, which is

packages/builder/src/index.ts

@@ -62,3 +62,12 @@ export {
   type Point,
   type Rect,
 } from "./geometry";
+
+/**
+ * The DOM read the mapping cannot do for itself.
+ *
+ * Exported beside the geometry because the inset is the one input every caller
+ * has to measure, and the one they get wrong: documenting the recipe as
+ * `clientLeft`/`clientTop` left three call sites short by the padding.
+ */
+export { frameInsetOf } from "./geometry-dom";

packages/builder/src/layering.test.ts

@@ -4,7 +4,12 @@ import { fileURLToPath } from "node:url";
 
 import ts from "typescript";
 import { describe, expect, it } from "vitest";
-import { collectModules, TEST_MODULE } from "./source-modules";
+import {
+  collectModules,
+  MODULE_EXTENSIONS,
+  TEST_GLOBS,
+  TEST_MODULE,
+} from "./source-modules";
 
 /**
  * The package's layering contract, enforced rather than documented.
@@ -474,6 +479,27 @@ describe("the builder's layering contract", () => {
     expect(files.some(f => f.endsWith("index.ts"))).toBe(true);
   });
 
+  it("asks the runner to collect every extension it treats as a test", () => {
+    // The globs and the allowlist agree about the word "test". That is an
+    // internal-consistency property and it is worth checking here, but it is
+    // NOT what catches a narrowed extension list: this assertion lives in a
+    // file the globs decide whether to collect, so dropping `ts` un-collects
+    // the check along with everything else and the run reports `1 passed (1)`
+    // in green. Measured, not supposed.
+    //
+    // What survives that is in `vitest.global-setup.ts`, which runs before any
+    // file is collected and compares the globs against the tests on disk.
+    expect(TEST_GLOBS).toHaveLength(MODULE_EXTENSIONS.length);
+    for (const extension of MODULE_EXTENSIONS) {
+      expect(TEST_GLOBS).toContain(`src/**/*.test.${extension}`);
+    }
+    // And every glob names a file this package would classify as a test, so the
+    // runner and the allowlist cannot mean different things by the word.
+    for (const glob of TEST_GLOBS) {
+      expect(TEST_MODULE.test(glob)).toBe(true);
+    }
+  });
+
   it("reads every extension it claims to, not only the common ones", () => {
     // `length > 0` and "an index.ts is present" both survive a walk narrowed to
     // `.ts` alone, so neither separates full coverage from partial. A file in a

packages/builder/src/source-modules.ts

@@ -40,7 +40,7 @@
  * with either extension is a module the bundler follows, so a list omitting
  * them leaves that file invisible to every check in this package.
  */
-const MODULE_EXTENSIONS = [
+export const MODULE_EXTENSIONS = [
   "ts",
   "tsx",
   "mts",

packages/builder/vitest.config.ts

@@ -16,11 +16,19 @@ import { TEST_GLOBS } from "./src/source-modules";
  * and the guard have to mean the same thing by the word: a hand-written glob
  * that omitted an extension the guard accepted would let a file import `vitest`
  * and never be run. Both now come from one list in `src/source-modules.ts`.
+ *
+ * `globalSetup` is what keeps that derivation honest. Narrowing the one list
+ * narrows these globs too, and a suite that stops being collected reports the
+ * same green as a suite that passed — so the check that the runner still
+ * collects every test on disk cannot itself be a test, because the narrowing
+ * would un-collect it. It runs before collection instead, where no glob decides
+ * whether it executes.
  */
 
 export default defineConfig({
   test: {
     environment: "node",
     include: TEST_GLOBS,
+    globalSetup: ["./vitest.global-setup.ts"],
   },
 });