mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-11-11 19:09:20 +00:00
fix(lint): back readAttr with the HTML parser
`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>
This commit is contained in:
co-authored by
Claude Opus 5
parent
e2e61b0767
commit
fb002e361e
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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. */
|
||||
|
||||
Reference in New Issue
Block a user