From 651f294ab5ab3f6b046ddae329d0f1d77951d9ee Mon Sep 17 00:00:00 2001 From: Ben Kennedy Date: Sat, 31 Oct 2026 00:17:36 +0000 Subject: [PATCH 1/7] Skip onEndTag on void elements in range replacement `element.onEndTag` throws "No end tag" on HTML void elements. The marker pass called it on every element under a range container, and the replacement pass on each fromMarker element, relying on try/catch. Skip the call for the standard void tags so the runtime no longer raises an exception per void element. Only elements in the HTML namespace are void: a `` or `` inside `` has an end tag and keeps the old path. An element that doesn't report its namespace is never treated as void. --- .changeset/skip-endtag-void-elements.md | 5 + src/adapters/HtmlRewriterAdapter.ts | 2 + src/stages/buildReplacementHandlers.ts | 3 +- src/stages/findMarkerPositions.ts | 3 +- src/stages/voidElements.ts | 29 +++++ test/fakes/LolHtmlRewriter.ts | 3 + test/stages/voidElements.spec.ts | 159 ++++++++++++++++++++++++ 7 files changed, 202 insertions(+), 2 deletions(-) create mode 100644 .changeset/skip-endtag-void-elements.md create mode 100644 src/stages/voidElements.ts create mode 100644 test/stages/voidElements.spec.ts diff --git a/.changeset/skip-endtag-void-elements.md b/.changeset/skip-endtag-void-elements.md new file mode 100644 index 00000000..4386c61e --- /dev/null +++ b/.changeset/skip-endtag-void-elements.md @@ -0,0 +1,5 @@ +--- +"@monetizationos/proxy": patch +--- + +Range replacement uses less CPU on pages with many void elements (`img`, `br`, `meta`, ...). Output is unchanged. diff --git a/src/adapters/HtmlRewriterAdapter.ts b/src/adapters/HtmlRewriterAdapter.ts index bd88c5cd..a8aca81a 100644 --- a/src/adapters/HtmlRewriterAdapter.ts +++ b/src/adapters/HtmlRewriterAdapter.ts @@ -45,6 +45,8 @@ export interface ContentOptions { export interface RewriterElement { readonly removed: boolean readonly tagName: string + /** Lets callers tell HTML void elements from same-named SVG/MathML ones; without it no element is treated as void. */ + readonly namespaceURI?: string getAttribute(name: string): string | null hasAttribute(name: string): boolean setAttribute(name: string, value: string): void diff --git a/src/stages/buildReplacementHandlers.ts b/src/stages/buildReplacementHandlers.ts index 2acd9a26..00df50f0 100644 --- a/src/stages/buildReplacementHandlers.ts +++ b/src/stages/buildReplacementHandlers.ts @@ -3,6 +3,7 @@ import type { HtmlStageContext } from '../context' import type { SubSurfaceBehaviorApi } from '../types' import { renderElement } from './elements' import type { ComponentRange } from './findMarkerPositions' +import { isVoidElement } from './voidElements' type ReplacementState = { operation: 'SCANNING' | 'REMOVING' | 'REPLACED' @@ -69,7 +70,7 @@ export const buildReplacementHandlers = ( const [content, options] = renderElement(ctx, after) element.after(content, options) } - if (element.onEndTag) { + if (element.onEndTag && !isVoidElement(element)) { try { element.onEndTag(() => { state.operation = 'REMOVING' diff --git a/src/stages/findMarkerPositions.ts b/src/stages/findMarkerPositions.ts index f64e933b..4a115fea 100644 --- a/src/stages/findMarkerPositions.ts +++ b/src/stages/findMarkerPositions.ts @@ -1,6 +1,7 @@ import type { ElementHandlers, HtmlRewriterAdapter, RewriterElement } from '../adapters/HtmlRewriterAdapter' import type { HtmlStageContext } from '../context' import type { SurfaceDecisionResponse } from '../types' +import { isVoidElement } from './voidElements' export type SurfaceMarkers = { componentsWithInvalidSelectors: string[] @@ -81,7 +82,7 @@ export const findMarkerPositions = async ( nextEndMarker: null, } currentRange.markers[elementCounter] = marker - if (element.onEndTag) { + if (element.onEndTag && !isVoidElement(element)) { try { element.onEndTag(() => { marker.endTag = elementCounter diff --git a/src/stages/voidElements.ts b/src/stages/voidElements.ts new file mode 100644 index 00000000..a9a5c38d --- /dev/null +++ b/src/stages/voidElements.ts @@ -0,0 +1,29 @@ +import type { RewriterElement } from '../adapters/HtmlRewriterAdapter' + +const VOID_ELEMENTS = new Set([ + 'area', + 'base', + 'br', + 'col', + 'embed', + 'hr', + 'img', + 'input', + 'link', + 'meta', + 'param', + 'source', + 'track', + 'wbr', +]) + +const HTML_NAMESPACE = 'http://www.w3.org/1999/xhtml' + +/** + * HTML void elements have no end tag, and rewriter runtimes throw when `onEndTag` is called on them. Only elements in + * the HTML namespace are void: a `` inside `` has an end tag, so an element that doesn't report its namespace is + * never treated as void. Self-closing foreign elements (`` in + * SVG) throw too but cannot be recognised by name, so callers keep a try/catch. + */ +export const isVoidElement = (element: RewriterElement): boolean => + VOID_ELEMENTS.has(element.tagName) && element.namespaceURI === HTML_NAMESPACE diff --git a/test/fakes/LolHtmlRewriter.ts b/test/fakes/LolHtmlRewriter.ts index 8e2e4231..6f0501d3 100644 --- a/test/fakes/LolHtmlRewriter.ts +++ b/test/fakes/LolHtmlRewriter.ts @@ -100,6 +100,9 @@ const adaptElement = (element: WasmElement): RewriterElement => ({ get tagName() { return element.tagName }, + get namespaceURI() { + return element.namespaceURI + }, getAttribute: (name) => element.getAttribute(name), hasAttribute: (name) => element.hasAttribute(name), setAttribute: (name, value) => { diff --git a/test/stages/voidElements.spec.ts b/test/stages/voidElements.spec.ts new file mode 100644 index 00000000..a7be2df6 --- /dev/null +++ b/test/stages/voidElements.spec.ts @@ -0,0 +1,159 @@ +import { describe, expect, it } from 'vitest' +import type { ElementHandlers, HtmlRewriterAdapter, HtmlRewriterSession, RewriterElement } from '../../src/adapters/HtmlRewriterAdapter' +import { normalizeMOSConfig } from '../../src/config' +import type { PipelineContext } from '../../src/context' +import handleSurfaceComponents from '../../src/stages/surfaceComponents' +import { isVoidElement } from '../../src/stages/voidElements' +import type { SurfaceDecisionResponse } from '../../src/types' +import { LolHtmlRewriter } from '../fakes/LolHtmlRewriter' + +const ctx: PipelineContext = { + config: normalizeMOSConfig({ + originUrl: 'https://origin.example.com', + surfaceSlug: 'web', + mosHost: 'https://api.monetizationos.com', + mosSecretKey: 'sk_env_test_abc', + anonymousSessionCookieName: 'anon-session', + authenticatedUserJwtCookieName: '__session', + }), + logger: { log() {} }, +} + +/** Wraps the real rewriter and records the tag of every element `onEndTag` is called on. */ +class EndTagSpyRewriter implements HtmlRewriterAdapter { + readonly inner = new LolHtmlRewriter() + readonly capabilities = this.inner.capabilities + readonly endTagCalls: string[] = [] + + create(): HtmlRewriterSession { + const session = this.inner.create() + const spy: HtmlRewriterSession = { + on: (selector: string, handlers: ElementHandlers) => { + session.on(selector, { + ...handlers, + element: handlers.element + ? (element) => + handlers.element?.({ + get removed() { + return element.removed + }, + get tagName() { + return element.tagName + }, + get namespaceURI() { + return element.namespaceURI + }, + getAttribute: (name) => element.getAttribute(name), + hasAttribute: (name) => element.hasAttribute(name), + setAttribute: (name, value) => element.setAttribute(name, value), + removeAttribute: (name) => element.removeAttribute(name), + before: (content, options) => element.before(content, options), + after: (content, options) => element.after(content, options), + prepend: (content, options) => element.prepend(content, options), + append: (content, options) => element.append(content, options), + replace: (content, options) => element.replace(content, options), + remove: () => element.remove(), + onEndTag: (callback) => { + this.endTagCalls.push(element.tagName.toLowerCase()) + element.onEndTag?.(callback) + }, + }) + : undefined, + }) + return spy + }, + transform: (response) => session.transform(response), + } + return spy + } +} + +const decisions = (replaceRange: { fromMarker?: string; toMarker?: string }, cssSelector = 'div.hero'): SurfaceDecisionResponse => ({ + status: 'success', + identity: { identifier: 'anon', isAuthenticated: false, authType: 'anonymous', jwtClaims: {} }, + features: {}, + customer: { hasProducts: false }, + surfaceBehavior: {}, + componentsSkipped: false, + componentBehaviors: { + hero: { + metadata: { cssSelector }, + content: { replaceRange: { ...replaceRange, replaceWith: [{ type: 'html', content: 'NEW' }] } }, + }, + }, +}) + +const run = async (body: string, replaceRange: { fromMarker?: string; toMarker?: string }, cssSelector?: string) => { + const rewriter = new EndTagSpyRewriter() + const response = new Response(body, { headers: { 'Content-Type': 'text/html; charset=utf-8' } }) + const result = await handleSurfaceComponents(ctx, response, decisions(replaceRange, cssSelector), rewriter) + return { body: await result.text(), endTagCalls: rewriter.endTagCalls } +} + +describe('range replacement with void elements', () => { + const VOID_TAGS = ['br', 'img', 'hr', 'input', 'meta', 'link', 'wbr'] + + it('does not call onEndTag on void elements and replaces the range as before', async () => { + const { body, endTagCalls } = await run( + '
S

kill

E
', + { fromMarker: 'span.start', toMarker: 'span.end' }, + ) + expect(body).toBe('
SNEWE
') + for (const tag of VOID_TAGS) { + expect(endTagCalls).not.toContain(tag) + } + expect(endTagCalls).toContain('p') + }) + + it('still calls onEndTag on a void-named element inside SVG, which has an end tag', async () => { + const { body, endTagCalls } = await run( + '
xE
', + { fromMarker: 'source.start', toMarker: 'span.end' }, + ) + expect(body).toBe('
NEWE
') + expect(endTagCalls).toContain('source') + }) + + it('handles a void element as the fromMarker without calling onEndTag on it', async () => { + const { body, endTagCalls } = await run( + '

keep

kill

E
', + { fromMarker: 'hr.start', toMarker: 'span.end' }, + ) + expect(body).toBe('

keep

NEWE
') + expect(endTagCalls).not.toContain('hr') + }) +}) + +describe('isVoidElement', () => { + const element = (tagName: string, namespaceURI?: string) => ({ tagName, namespaceURI }) as RewriterElement + const HTML = 'http://www.w3.org/1999/xhtml' + const SVG = 'http://www.w3.org/2000/svg' + + it.each([ + 'area', + 'base', + 'br', + 'col', + 'embed', + 'hr', + 'img', + 'input', + 'link', + 'meta', + 'param', + 'source', + 'track', + 'wbr', + ])('treats an HTML <%s> as void', (tag) => { + expect(isVoidElement(element(tag, HTML))).toBe(true) + }) + + it('does not treat elements with an end tag as void', () => { + expect(isVoidElement(element('p', HTML))).toBe(false) + expect(isVoidElement(element('link', SVG))).toBe(false) + }) + + it('does not treat an element with no namespace as void', () => { + expect(isVoidElement(element('br'))).toBe(false) + }) +}) From 3fd62dee2e3e63721faf975d58850467306d5635 Mon Sep 17 00:00:00 2001 From: Ben Kennedy Date: Wed, 4 Nov 2026 23:25:45 +0000 Subject: [PATCH 2/7] Don't fail the transform when a range container has no end tag A replaceRange selector that matches an element with no end tag (`img`, or a self-closing SVG element) made onEndTag throw out of the parent handler and failed the whole page transform. Such an element has no content to cut, so leave it alone. --- .changeset/range-container-no-end-tag.md | 5 ++++ src/stages/buildReplacementHandlers.ts | 17 ++++++----- test/stages/voidElements.spec.ts | 36 ++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 .changeset/range-container-no-end-tag.md diff --git a/.changeset/range-container-no-end-tag.md b/.changeset/range-container-no-end-tag.md new file mode 100644 index 00000000..8ff95443 --- /dev/null +++ b/.changeset/range-container-no-end-tag.md @@ -0,0 +1,5 @@ +--- +"@monetizationos/proxy": patch +--- + +A `replaceRange` component whose selector matches an element with no end tag (`img`, or a self-closing SVG element such as ``) no longer fails the whole page transform. That element has no content to cut, so it is left as it is. diff --git a/src/stages/buildReplacementHandlers.ts b/src/stages/buildReplacementHandlers.ts index 00df50f0..26d71cd3 100644 --- a/src/stages/buildReplacementHandlers.ts +++ b/src/stages/buildReplacementHandlers.ts @@ -110,17 +110,20 @@ export const buildReplacementHandlers = ( const state: ReplacementState = { operation: modification.fromMarker ? 'SCANNING' : 'REMOVING' } states.push(state) - if (state.operation === 'REMOVING') { - for (const prepend of (modification.replaceWith ?? []).slice().reverse()) { - const [content, options] = renderElement(ctx, prepend) - element.prepend(content, options) + try { + if (state.operation === 'REMOVING') { + for (const prepend of (modification.replaceWith ?? []).slice().reverse()) { + const [content, options] = renderElement(ctx, prepend) + element.prepend(content, options) + } } - } - if (element.onEndTag) { - element.onEndTag(() => { + element.onEndTag?.(() => { states.pop() }) + } catch { + // A container with no end tag (`img`, or a self-closing ``) has no content to cut, and the runtime throws on it. + states.pop() } }, diff --git a/test/stages/voidElements.spec.ts b/test/stages/voidElements.spec.ts index a7be2df6..51e70bd8 100644 --- a/test/stages/voidElements.spec.ts +++ b/test/stages/voidElements.spec.ts @@ -157,3 +157,39 @@ describe('isVoidElement', () => { expect(isVoidElement(element('br'))).toBe(false) }) }) + +describe('range replacement when the container has no end tag', () => { + it.each([ + ['without markers', {}], + ['with a fromMarker', { fromMarker: 'span' }], + ])('leaves a void container alone %s', async (_, replaceRange) => { + const html = '

a

b

' + const { body } = await run(html, replaceRange, 'img.hero') + expect(body).toBe(html) + }) + + it('leaves a self-closing foreign container alone', async () => { + const html = '

b

' + const { body } = await run(html, {}, 'path.hero') + expect(body).toBe(html) + }) + + it('pops the state of a void container nested in a matching container', async () => { + const { body } = await run( + '
s

x

', + { fromMarker: '.s' }, + '.hero', + ) + expect(body).toBe('
sNEW
') + }) + + it('still replaces the content of a void-named container inside SVG', async () => { + const { body } = await run('', {}, 'link.hero') + expect(body).toBe('NEW') + }) + + it('still replaces a later container once an earlier match had no end tag', async () => { + const { body } = await run('

x

', {}, '.hero') + expect(body).toBe('
NEW
') + }) +}) From e15ff7445faf7157df58c06fbb94e71bd8fc6520 Mon Sep 17 00:00:00 2001 From: Ben Kennedy Date: Thu, 5 Nov 2026 10:06:37 +0000 Subject: [PATCH 3/7] Start a range at the first start marker, in the outermost container A start marker nested inside the first one started the range again: the replacement went in twice and the rest of the outer marker's content was cut. An IN_START state now holds the range until the first start marker's end tag. A container nested inside another matching one got a range of its own. A `${selector} ${selector}` handler, registered ahead of the container handler in both passes, now marks it as part of the outer range. --- .changeset/range-nested-matches.md | 5 +++ src/stages/buildReplacementHandlers.ts | 19 ++++++++++- src/stages/findMarkerPositions.ts | 15 +++++++++ test/stages/surfaceComponents.spec.ts | 44 ++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 .changeset/range-nested-matches.md diff --git a/.changeset/range-nested-matches.md b/.changeset/range-nested-matches.md new file mode 100644 index 00000000..63229969 --- /dev/null +++ b/.changeset/range-nested-matches.md @@ -0,0 +1,5 @@ +--- +"@monetizationos/proxy": patch +--- + +A range starts at the first start marker in its container, so a start marker nested inside it no longer inserts the replacement a second time. A range also applies to the outermost matching container: a container nested inside another one is part of the outer container's range instead of getting a cut of its own. diff --git a/src/stages/buildReplacementHandlers.ts b/src/stages/buildReplacementHandlers.ts index 26d71cd3..a02ff8c5 100644 --- a/src/stages/buildReplacementHandlers.ts +++ b/src/stages/buildReplacementHandlers.ts @@ -6,7 +6,8 @@ import type { ComponentRange } from './findMarkerPositions' import { isVoidElement } from './voidElements' type ReplacementState = { - operation: 'SCANNING' | 'REMOVING' | 'REPLACED' + // `IN_START`: a start marker nested in the first one doesn't start the range again + operation: 'SCANNING' | 'IN_START' | 'REMOVING' | 'REPLACED' } export type ReplacementHandler = { selector: string; handler: ElementHandlers } @@ -70,6 +71,7 @@ export const buildReplacementHandlers = ( const [content, options] = renderElement(ctx, after) element.after(content, options) } + state.operation = 'IN_START' if (element.onEndTag && !isVoidElement(element)) { try { element.onEndTag(() => { @@ -100,8 +102,15 @@ export const buildReplacementHandlers = ( } } + // A nested container is part of the outer one's range; set by the handler registered ahead of `parentHandler` + let nested = false + const parentHandler: ElementHandlers = { element(element: RewriterElement) { + if (nested) { + nested = false + return + } rangeIndex++ if (element.removed) { return @@ -140,6 +149,14 @@ export const buildReplacementHandlers = ( return [] } return [ + { + selector: `${selector} ${selector}`, + handler: { + element() { + nested = true + }, + }, + }, { selector, handler: parentHandler }, { selector: `${selector} *`, handler: { element: onChildElement } }, ] diff --git a/src/stages/findMarkerPositions.ts b/src/stages/findMarkerPositions.ts index 4a115fea..6978c0be 100644 --- a/src/stages/findMarkerPositions.ts +++ b/src/stages/findMarkerPositions.ts @@ -53,13 +53,28 @@ export const findMarkerPositions = async ( const { toMarker, fromMarker } = modification let elementCounter = 0 let currentRange: ComponentRange | null = null + // A nested container is part of the outer one's range; set by the handler registered ahead of the container's + let nested = false const entries: (Handler | null)[] = [ + { + selector: `${componentBehavior.metadata.cssSelector} ${componentBehavior.metadata.cssSelector}`, + componentKey, + handler: { + element: () => { + nested = true + }, + }, + }, { selector: `${componentBehavior.metadata.cssSelector}`, componentKey, handler: { element: () => { + if (nested) { + nested = false + return + } currentRange = { markers: [] } markers[componentKey]?.push(currentRange) }, diff --git a/test/stages/surfaceComponents.spec.ts b/test/stages/surfaceComponents.spec.ts index 5bf53baa..5c421320 100644 --- a/test/stages/surfaceComponents.spec.ts +++ b/test/stages/surfaceComponents.spec.ts @@ -332,6 +332,50 @@ describe('handleSurfaceComponents', () => { expect(events).toContainEqual(expect.objectContaining({ level: 'warn', code: 'replacement-markers-identical' })) }) + describe('replaceRange with nested matches', () => { + const replaceRange = async (body: string, range: { fromMarker?: string }) => { + const decision = decisions({ + range: { + metadata: { cssSelector: '.c' }, + content: { replaceRange: { ...range, replaceWith: [{ type: 'html', content: 'NEW' }] } }, + }, + }) + return (await handleSurfaceComponents(baseCtx, html(body), decision, new LolHtmlRewriter())).text() + } + + it('starts the range at the first start marker when it holds another', async () => { + const body = await replaceRange('

a

b

c

', { + fromMarker: '.s', + }) + expect(body).toBe('

a

b

NEW
') + }) + + it('starts the range at the first start marker when both are nested in the container', async () => { + const body = await replaceRange( + '

a

b

c

', + { + fromMarker: '.s', + }, + ) + expect(body).toBe('

a

NEW
') + }) + + it('applies the range to the outermost of nested containers', async () => { + const body = await replaceRange( + '

a

b

c

d

e

f

', + { fromMarker: 'p.s' }, + ) + expect(body).toBe( + '

a

NEW

e

NEW
', + ) + }) + + it('replaces the whole outermost container when nested containers have no markers', async () => { + const body = await replaceRange('

a

b

c

', {}) + expect(body).toBe('
NEW
') + }) + }) + it('falls back gracefully when findMarkerPositions rejects a marker selector — other components still apply', async () => { const { events, logger } = createRecordingLogger() const response = html('
leave
orig
') From 0597c9e80159c1b3eabe805cede1a06b331e221f Mon Sep 17 00:00:00 2001 From: Ben Kennedy Date: Thu, 5 Nov 2026 11:09:49 +0000 Subject: [PATCH 4/7] Merge the range replacement changesets --- .changeset/range-container-no-end-tag.md | 5 ----- .changeset/range-nested-matches.md | 5 ----- .changeset/range-replacement-fixes.md | 10 ++++++++++ .changeset/skip-endtag-void-elements.md | 5 ----- 4 files changed, 10 insertions(+), 15 deletions(-) delete mode 100644 .changeset/range-container-no-end-tag.md delete mode 100644 .changeset/range-nested-matches.md create mode 100644 .changeset/range-replacement-fixes.md delete mode 100644 .changeset/skip-endtag-void-elements.md diff --git a/.changeset/range-container-no-end-tag.md b/.changeset/range-container-no-end-tag.md deleted file mode 100644 index 8ff95443..00000000 --- a/.changeset/range-container-no-end-tag.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@monetizationos/proxy": patch ---- - -A `replaceRange` component whose selector matches an element with no end tag (`img`, or a self-closing SVG element such as ``) no longer fails the whole page transform. That element has no content to cut, so it is left as it is. diff --git a/.changeset/range-nested-matches.md b/.changeset/range-nested-matches.md deleted file mode 100644 index 63229969..00000000 --- a/.changeset/range-nested-matches.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@monetizationos/proxy": patch ---- - -A range starts at the first start marker in its container, so a start marker nested inside it no longer inserts the replacement a second time. A range also applies to the outermost matching container: a container nested inside another one is part of the outer container's range instead of getting a cut of its own. diff --git a/.changeset/range-replacement-fixes.md b/.changeset/range-replacement-fixes.md new file mode 100644 index 00000000..444e6cae --- /dev/null +++ b/.changeset/range-replacement-fixes.md @@ -0,0 +1,10 @@ +--- +"@monetizationos/proxy": patch +--- + +Range replacement fixes: + +- Uses less CPU on pages with many void elements (`img`, `br`, `meta`, ...). Output is unchanged. +- A range container with no end tag (`img`, or a self-closing SVG element such as ``) is left as it is instead of failing the whole page transform. +- A range starts at the first start marker, so a start marker nested inside it no longer inserts the replacement a second time. +- A range applies to the outermost matching container. A container nested inside another one is part of the outer container's cut. diff --git a/.changeset/skip-endtag-void-elements.md b/.changeset/skip-endtag-void-elements.md deleted file mode 100644 index 4386c61e..00000000 --- a/.changeset/skip-endtag-void-elements.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@monetizationos/proxy": patch ---- - -Range replacement uses less CPU on pages with many void elements (`img`, `br`, `meta`, ...). Output is unchanged. From d5d878d86d3c00ee1dc93152f4e6245a163adb0b Mon Sep 17 00:00:00 2001 From: Ben Kennedy Date: Thu, 5 Nov 2026 11:12:48 +0000 Subject: [PATCH 5/7] Track nested range containers by depth instead of a doubled selector --- src/stages/buildReplacementHandlers.ts | 42 ++++++++++++-------------- src/stages/findMarkerPositions.ts | 28 ++++++++--------- test/stages/surfaceComponents.spec.ts | 32 ++++++++++++++++++-- 3 files changed, 64 insertions(+), 38 deletions(-) diff --git a/src/stages/buildReplacementHandlers.ts b/src/stages/buildReplacementHandlers.ts index a02ff8c5..71bc8b61 100644 --- a/src/stages/buildReplacementHandlers.ts +++ b/src/stages/buildReplacementHandlers.ts @@ -102,37 +102,43 @@ export const buildReplacementHandlers = ( } } - // A nested container is part of the outer one's range; set by the handler registered ahead of `parentHandler` - let nested = false + // Open containers; a container inside another one is part of the outer one's range + let depth = 0 const parentHandler: ElementHandlers = { element(element: RewriterElement) { - if (nested) { - nested = false + if (depth > 0) { return } rangeIndex++ - if (element.removed) { - return + // A removed container still counts towards `depth`, so the ranges line up with the marker pass + const state: ReplacementState | null = element.removed ? null : { operation: modification.fromMarker ? 'SCANNING' : 'REMOVING' } + if (state) { + states.push(state) } - const state: ReplacementState = { operation: modification.fromMarker ? 'SCANNING' : 'REMOVING' } - states.push(state) - try { - if (state.operation === 'REMOVING') { + if (state?.operation === 'REMOVING') { for (const prepend of (modification.replaceWith ?? []).slice().reverse()) { const [content, options] = renderElement(ctx, prepend) element.prepend(content, options) } } - element.onEndTag?.(() => { - states.pop() - }) + if (element.onEndTag) { + element.onEndTag(() => { + depth-- + if (state) { + states.pop() + } + }) + depth++ + } } catch { // A container with no end tag (`img`, or a self-closing ``) has no content to cut, and the runtime throws on it. - states.pop() + if (state) { + states.pop() + } } }, @@ -149,14 +155,6 @@ export const buildReplacementHandlers = ( return [] } return [ - { - selector: `${selector} ${selector}`, - handler: { - element() { - nested = true - }, - }, - }, { selector, handler: parentHandler }, { selector: `${selector} *`, handler: { element: onChildElement } }, ] diff --git a/src/stages/findMarkerPositions.ts b/src/stages/findMarkerPositions.ts index 6978c0be..a06347fb 100644 --- a/src/stages/findMarkerPositions.ts +++ b/src/stages/findMarkerPositions.ts @@ -53,30 +53,30 @@ export const findMarkerPositions = async ( const { toMarker, fromMarker } = modification let elementCounter = 0 let currentRange: ComponentRange | null = null - // A nested container is part of the outer one's range; set by the handler registered ahead of the container's - let nested = false + // Open containers; a container inside another one is part of the outer one's range + let depth = 0 const entries: (Handler | null)[] = [ - { - selector: `${componentBehavior.metadata.cssSelector} ${componentBehavior.metadata.cssSelector}`, - componentKey, - handler: { - element: () => { - nested = true - }, - }, - }, { selector: `${componentBehavior.metadata.cssSelector}`, componentKey, handler: { - element: () => { - if (nested) { - nested = false + element: (element: RewriterElement) => { + if (depth > 0) { return } currentRange = { markers: [] } markers[componentKey]?.push(currentRange) + if (element.onEndTag && !isVoidElement(element)) { + try { + element.onEndTag(() => { + depth-- + }) + depth++ + } catch { + // No end tag, so nothing can be nested in it + } + } }, }, }, diff --git a/test/stages/surfaceComponents.spec.ts b/test/stages/surfaceComponents.spec.ts index 5c421320..b7b79220 100644 --- a/test/stages/surfaceComponents.spec.ts +++ b/test/stages/surfaceComponents.spec.ts @@ -333,10 +333,10 @@ describe('handleSurfaceComponents', () => { }) describe('replaceRange with nested matches', () => { - const replaceRange = async (body: string, range: { fromMarker?: string }) => { + const replaceRange = async (body: string, range: { fromMarker?: string }, cssSelector = '.c') => { const decision = decisions({ range: { - metadata: { cssSelector: '.c' }, + metadata: { cssSelector }, content: { replaceRange: { ...range, replaceWith: [{ type: 'html', content: 'NEW' }] } }, }, }) @@ -374,6 +374,34 @@ describe('handleSurfaceComponents', () => { const body = await replaceRange('

a

b

c

', {}) expect(body).toBe('
NEW
') }) + + it('still cuts a later container after one removed by another component', async () => { + const decision = decisions({ + gone: { metadata: { cssSelector: '#gone' }, content: { remove: true } }, + range: { + metadata: { cssSelector: '.c' }, + content: { replaceRange: { fromMarker: 'p.s', replaceWith: [{ type: 'html', content: 'NEW' }] } }, + }, + }) + const page = + '

a

b

e

f

' + const result = await handleSurfaceComponents(baseCtx, html(page), decision, new LolHtmlRewriter()) + expect(await result.text()).toBe('

e

NEW
') + }) + + it('applies the range to a container matched by a selector list', async () => { + const body = await replaceRange('

a

b

', { fromMarker: 'p.s' }, '.x, .c') + expect(body).toBe('

a

NEW
') + }) + + it('applies the range to the outermost of nested containers matched by a descendant selector', async () => { + const body = await replaceRange( + '

a

b

c

d

', + { fromMarker: 'p.s' }, + 'main .c', + ) + expect(body).toBe('

a

NEW
') + }) }) it('falls back gracefully when findMarkerPositions rejects a marker selector — other components still apply', async () => { From a2e76e1f5044b8cae9f5ea69411ef010551f7c2a Mon Sep 17 00:00:00 2001 From: Ben Kennedy Date: Thu, 5 Nov 2026 14:48:42 +0000 Subject: [PATCH 6/7] Recognise void elements by tag name alone --- src/adapters/HtmlRewriterAdapter.ts | 2 -- src/stages/voidElements.ts | 11 +++------- test/fakes/LolHtmlRewriter.ts | 3 --- test/stages/voidElements.spec.ts | 32 ++++------------------------- 4 files changed, 7 insertions(+), 41 deletions(-) diff --git a/src/adapters/HtmlRewriterAdapter.ts b/src/adapters/HtmlRewriterAdapter.ts index a8aca81a..bd88c5cd 100644 --- a/src/adapters/HtmlRewriterAdapter.ts +++ b/src/adapters/HtmlRewriterAdapter.ts @@ -45,8 +45,6 @@ export interface ContentOptions { export interface RewriterElement { readonly removed: boolean readonly tagName: string - /** Lets callers tell HTML void elements from same-named SVG/MathML ones; without it no element is treated as void. */ - readonly namespaceURI?: string getAttribute(name: string): string | null hasAttribute(name: string): boolean setAttribute(name: string, value: string): void diff --git a/src/stages/voidElements.ts b/src/stages/voidElements.ts index a9a5c38d..a77c09e0 100644 --- a/src/stages/voidElements.ts +++ b/src/stages/voidElements.ts @@ -17,13 +17,8 @@ const VOID_ELEMENTS = new Set([ 'wbr', ]) -const HTML_NAMESPACE = 'http://www.w3.org/1999/xhtml' - /** - * HTML void elements have no end tag, and rewriter runtimes throw when `onEndTag` is called on them. Only elements in - * the HTML namespace are void: a `` inside `` has an end tag, so an element that doesn't report its namespace is - * never treated as void. Self-closing foreign elements (`` in - * SVG) throw too but cannot be recognised by name, so callers keep a try/catch. + * HTML void elements have no end tag, and rewriter runtimes throw when `onEndTag` is called on them. Self-closing + * foreign elements (`` in SVG) throw too but cannot be recognised by name, so callers keep a try/catch. */ -export const isVoidElement = (element: RewriterElement): boolean => - VOID_ELEMENTS.has(element.tagName) && element.namespaceURI === HTML_NAMESPACE +export const isVoidElement = (element: RewriterElement): boolean => VOID_ELEMENTS.has(element.tagName) diff --git a/test/fakes/LolHtmlRewriter.ts b/test/fakes/LolHtmlRewriter.ts index 6f0501d3..8e2e4231 100644 --- a/test/fakes/LolHtmlRewriter.ts +++ b/test/fakes/LolHtmlRewriter.ts @@ -100,9 +100,6 @@ const adaptElement = (element: WasmElement): RewriterElement => ({ get tagName() { return element.tagName }, - get namespaceURI() { - return element.namespaceURI - }, getAttribute: (name) => element.getAttribute(name), hasAttribute: (name) => element.hasAttribute(name), setAttribute: (name, value) => { diff --git a/test/stages/voidElements.spec.ts b/test/stages/voidElements.spec.ts index 51e70bd8..f4d5b242 100644 --- a/test/stages/voidElements.spec.ts +++ b/test/stages/voidElements.spec.ts @@ -40,9 +40,6 @@ class EndTagSpyRewriter implements HtmlRewriterAdapter { get tagName() { return element.tagName }, - get namespaceURI() { - return element.namespaceURI - }, getAttribute: (name) => element.getAttribute(name), hasAttribute: (name) => element.hasAttribute(name), setAttribute: (name, value) => element.setAttribute(name, value), @@ -105,15 +102,6 @@ describe('range replacement with void elements', () => { expect(endTagCalls).toContain('p') }) - it('still calls onEndTag on a void-named element inside SVG, which has an end tag', async () => { - const { body, endTagCalls } = await run( - '
xE
', - { fromMarker: 'source.start', toMarker: 'span.end' }, - ) - expect(body).toBe('
NEWE
') - expect(endTagCalls).toContain('source') - }) - it('handles a void element as the fromMarker without calling onEndTag on it', async () => { const { body, endTagCalls } = await run( '

keep

kill

E
', @@ -125,9 +113,7 @@ describe('range replacement with void elements', () => { }) describe('isVoidElement', () => { - const element = (tagName: string, namespaceURI?: string) => ({ tagName, namespaceURI }) as RewriterElement - const HTML = 'http://www.w3.org/1999/xhtml' - const SVG = 'http://www.w3.org/2000/svg' + const element = (tagName: string) => ({ tagName }) as RewriterElement it.each([ 'area', @@ -144,17 +130,12 @@ describe('isVoidElement', () => { 'source', 'track', 'wbr', - ])('treats an HTML <%s> as void', (tag) => { - expect(isVoidElement(element(tag, HTML))).toBe(true) + ])('treats <%s> as void', (tag) => { + expect(isVoidElement(element(tag))).toBe(true) }) it('does not treat elements with an end tag as void', () => { - expect(isVoidElement(element('p', HTML))).toBe(false) - expect(isVoidElement(element('link', SVG))).toBe(false) - }) - - it('does not treat an element with no namespace as void', () => { - expect(isVoidElement(element('br'))).toBe(false) + expect(isVoidElement(element('p'))).toBe(false) }) }) @@ -183,11 +164,6 @@ describe('range replacement when the container has no end tag', () => { expect(body).toBe('
sNEW
') }) - it('still replaces the content of a void-named container inside SVG', async () => { - const { body } = await run('', {}, 'link.hero') - expect(body).toBe('NEW') - }) - it('still replaces a later container once an earlier match had no end tag', async () => { const { body } = await run('

x

', {}, '.hero') expect(body).toBe('
NEW
') From 95d3573c4448c4133be3628dbb2f64089e8b0e4d Mon Sep 17 00:00:00 2001 From: Ben Kennedy Date: Thu, 5 Nov 2026 16:33:05 +0000 Subject: [PATCH 7/7] Skip void range containers up front and spy on end tags through a Proxy --- src/stages/buildReplacementHandlers.ts | 6 ++- test/stages/voidElements.spec.ts | 54 ++++++++++++++------------ 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/src/stages/buildReplacementHandlers.ts b/src/stages/buildReplacementHandlers.ts index 71bc8b61..9983fcb7 100644 --- a/src/stages/buildReplacementHandlers.ts +++ b/src/stages/buildReplacementHandlers.ts @@ -111,6 +111,10 @@ export const buildReplacementHandlers = ( return } rangeIndex++ + // No content to cut + if (isVoidElement(element)) { + return + } // A removed container still counts towards `depth`, so the ranges line up with the marker pass const state: ReplacementState | null = element.removed ? null : { operation: modification.fromMarker ? 'SCANNING' : 'REMOVING' } if (state) { @@ -135,7 +139,7 @@ export const buildReplacementHandlers = ( depth++ } } catch { - // A container with no end tag (`img`, or a self-closing ``) has no content to cut, and the runtime throws on it. + // `onEndTag` throws on a self-closing foreign container (``), which has no end tag or content to cut if (state) { states.pop() } diff --git a/test/stages/voidElements.spec.ts b/test/stages/voidElements.spec.ts index f4d5b242..d046ac3e 100644 --- a/test/stages/voidElements.spec.ts +++ b/test/stages/voidElements.spec.ts @@ -25,37 +25,30 @@ class EndTagSpyRewriter implements HtmlRewriterAdapter { readonly capabilities = this.inner.capabilities readonly endTagCalls: string[] = [] + // Methods are bound to the element, which a real rewriter element needs + private spyOn(element: RewriterElement): RewriterElement { + const onEndTag: RewriterElement['onEndTag'] = (callback) => { + this.endTagCalls.push(element.tagName.toLowerCase()) + element.onEndTag?.(callback) + } + return new Proxy(element, { + get: (target, prop) => { + if (prop === 'onEndTag') { + return onEndTag + } + const value = Reflect.get(target, prop) + return typeof value === 'function' ? value.bind(target) : value + }, + }) + } + create(): HtmlRewriterSession { const session = this.inner.create() const spy: HtmlRewriterSession = { on: (selector: string, handlers: ElementHandlers) => { session.on(selector, { ...handlers, - element: handlers.element - ? (element) => - handlers.element?.({ - get removed() { - return element.removed - }, - get tagName() { - return element.tagName - }, - getAttribute: (name) => element.getAttribute(name), - hasAttribute: (name) => element.hasAttribute(name), - setAttribute: (name, value) => element.setAttribute(name, value), - removeAttribute: (name) => element.removeAttribute(name), - before: (content, options) => element.before(content, options), - after: (content, options) => element.after(content, options), - prepend: (content, options) => element.prepend(content, options), - append: (content, options) => element.append(content, options), - replace: (content, options) => element.replace(content, options), - remove: () => element.remove(), - onEndTag: (callback) => { - this.endTagCalls.push(element.tagName.toLowerCase()) - element.onEndTag?.(callback) - }, - }) - : undefined, + element: handlers.element ? (element) => handlers.element?.(this.spyOn(element)) : undefined, }) return spy }, @@ -102,6 +95,17 @@ describe('range replacement with void elements', () => { expect(endTagCalls).toContain('p') }) + it('cuts past void elements written with an end tag, which the cut leaves behind', async () => { + const { body, endTagCalls } = await run( + '
S

kill

E
', + { fromMarker: 'span.start', toMarker: 'span.end' }, + ) + // The rewriter never hands a stray end tag to a handler, so cutting element by element can't remove it + expect(body).toBe('
SNEW
E
') + expect(endTagCalls).not.toContain('img') + expect(endTagCalls).not.toContain('br') + }) + it('handles a void element as the fromMarker without calling onEndTag on it', async () => { const { body, endTagCalls } = await run( '

keep

kill

E
',