From fb002e361e63fb7c4f33bcf283e9eecbacc390db Mon Sep 17 00:00:00 2001
From: Xuanru Li <157947275+xuanruli@users.noreply.github.com>
Date: Sun, 26 Jul 2026 03:58:27 -0700
Subject: [PATCH] fix(lint): back readAttr with the HTML parser
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`readAttr` matched only quoted attribute values. Unquoted values are valid
HTML5 and the runtime honours them, so on a root written
`data-composition-id=main data-start=0 data-width=1920 data-height=1080` the
linter reported two errors for attributes that are present, while a file with
genuinely overlapping unquoted clips and no `class="clip"` linted clean. The
same rule set printed the composition id in its own message, because
`readDecodedAttr` — the htmlparser2-backed reader twenty lines below — parsed
that tag correctly, which isolated the defect to the regex.

Widening the regex with an unquoted alternative is worse, not better: it has no
notion of attribute-list position, so it captures `name=value` pairs out of
other attributes' quoted values. Measured on that version, two `<video>`
elements with distinct real ids sharing a `?id=9` source both read as id "9" —
`duplicate_media_id` fired on an id that does not exist and swallowed two
genuine `video_missing_muted` findings — and a video with no id at all but
`?id=7` in its URL had `media_missing_id` masked, hiding a frozen-video render
bug. A Google-Drive-style `?export=view&id=…` asset URL is enough to trigger it.

Delegating to the parser fixes the blind spot without that class of mis-read,
and drops the `=`-truncation and undecoded-entity divergences as well. `|| null`
keeps the existing "empty value means absent" contract.

Behaviour change worth surfacing in release notes: attributes that were
invisible are now linted, so a project written with unquoted attributes AND a
real violation will newly fail `lint` and `render --strict`. Correct
unquoted-attribute projects lint clean. The repo's own 13 examples and all
registry blocks produce byte-identical findings.

`readJsonAttr` is left alone; the parser makes it redundant, but removing it is
its own change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 packages/lint/src/utils.test.ts | 76 +++++++++++++++++++++++++++++++++
 packages/lint/src/utils.ts      | 16 +++----
 2 files changed, 84 insertions(+), 8 deletions(-)
 create mode 100644 packages/lint/src/utils.test.ts

diff --git a/packages/lint/src/utils.test.ts b/packages/lint/src/utils.test.ts
new file mode 100644
index 0000000000..f31f78138d
--- /dev/null
+++ b/packages/lint/src/utils.test.ts
@@ -0,0 +1,76 @@
+import { describe, expect, it } from "vitest";
+
+import { readAttr, readDecodedAttr } from "./utils.js";
+
+describe("readAttr", () => {
+  it("reads a double-quoted value", () => {
+    expect(readAttr('<div data-start="2.5">', "data-start")).toBe("2.5");
+  });
+
+  it("reads a single-quoted value", () => {
+    expect(readAttr("<div data-start='2.5'>", "data-start")).toBe("2.5");
+  });
+
+  // Unquoted values are valid HTML5 and the runtime reads them, so a
+  // quoted-only pattern both invents "missing attribute" errors and hides
+  // real violations on the attributes it cannot see.
+  it("reads an unquoted value", () => {
+    expect(readAttr("<div data-start=0>", "data-start")).toBe("0");
+  });
+
+  it("reads unquoted values from a tag that mixes quoting styles", () => {
+    const tag = `<div id="root" data-composition-id=main data-start=0 data-width=1920 data-height='1080'>`;
+    expect(readAttr(tag, "data-composition-id")).toBe("main");
+    expect(readAttr(tag, "data-start")).toBe("0");
+    expect(readAttr(tag, "data-width")).toBe("1920");
+    expect(readAttr(tag, "data-height")).toBe("1080");
+  });
+
+  it("agrees with the parser-backed reader across quoting styles and near-misses", () => {
+    const cases: Array<[string, string]> = [
+      ["<div data-track-index=2 data-duration=6>", "data-track-index"],
+      ["<div data-track-index=2 data-duration=6>", "data-duration"],
+      ['<div data-hf-src="x?id=5" id="root">', "id"],
+      ['<img src="x.png?class=big" class="clip">', "class"],
+      ["<div data-composition-src=scenes/a.html>", "data-composition-src"],
+    ];
+    for (const [tag, attr] of cases) {
+      expect(readAttr(tag, attr)).toBe(readDecodedAttr(tag, attr) || null);
+    }
+  });
+
+  it("reads unquoted values that sit next to other attributes", () => {
+    expect(readAttr("<div data-start=0 class=clip>", "class")).toBe("clip");
+    expect(readAttr("<div data-composition-src=scenes/a.html>", "data-composition-src")).toBe(
+      "scenes/a.html",
+    );
+  });
+
+  // A `name=value` pair inside another attribute's value is not an attribute.
+  // Reading it as one made `duplicate_media_id` fire on ids that do not exist
+  // and hid `media_missing_id` on a video that really had none.
+  it("does not read a name=value pair out of another attribute's value", () => {
+    expect(readAttr('<img src="https://cdn.x/i?id=123" class="clip">', "id")).toBeNull();
+    expect(readAttr('<img src="x.png?class=big" class="clip">', "class")).toBe("clip");
+    expect(readAttr('<div data-hf-src="clip.mp4?id=5" id="root">', "id")).toBe("root");
+    expect(readAttr('<video src="v.mp4?preload=none">', "preload")).toBeNull();
+    expect(readAttr('<div title="use id=hero">', "id")).toBeNull();
+  });
+
+  // The `(?<![\w-])` lookbehind, not `\b`: a hyphen would otherwise read as a
+  // word break and match the tail of a longer attribute name.
+  it("does not match the tail of a longer attribute name", () => {
+    expect(readAttr('<div data-hf-id="x">', "id")).toBeNull();
+    expect(readAttr("<div data-width=1920>", "width")).toBeNull();
+  });
+
+  it("returns null for an absent or valueless attribute", () => {
+    expect(readAttr("<video muted>", "muted")).toBeNull();
+    expect(readAttr("<div>", "data-start")).toBeNull();
+    expect(readAttr("", "data-start")).toBeNull();
+  });
+
+  it("treats an empty quoted value as absent, as before", () => {
+    expect(readAttr('<div data-start="">', "data-start")).toBeNull();
+  });
+});
diff --git a/packages/lint/src/utils.ts b/packages/lint/src/utils.ts
index 39b2315381..7a0f31ff94 100644
--- a/packages/lint/src/utils.ts
+++ b/packages/lint/src/utils.ts
@@ -162,15 +162,15 @@ export function findRootTag(source: string, parsedTags?: readonly OpenTag[]): Op
   return null;
 }
 
+/**
+ * Read an HTML attribute the way the runtime does. Parser-backed rather than a
+ * regex over the tag text: a regex cannot see unquoted values (valid HTML5, and
+ * the runtime honours them), so it both invents "missing attribute" errors and
+ * hides real violations — and widening it to accept unquoted values makes it
+ * capture `name=value` pairs out of other attributes' quoted values instead.
+ */
 export function readAttr(tagSource: string, attr: string): string | null {
-  if (!tagSource) return null;
-  const escaped = attr.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
-  // `(?<![\w-])` not `\b`: a plain `\b` boundary treats the hyphen in a longer
-  // attribute as a word break, so reading "id" would wrongly match the trailing
-  // `id="…"` inside `data-hf-id="…"` (and "width" inside `data-width`, etc.).
-  // The lookbehind requires the match to start a fresh attribute name.
-  const match = tagSource.match(new RegExp(`(?<![\\w-])${escaped}\\s*=\\s*["']([^"']+)["']`, "i"));
-  return match?.[1] || null;
+  return readDecodedAttr(tagSource, attr) || null;
 }
 
 /** Read an HTML attribute using browser-equivalent character-reference decoding. */