From fa05177f728b3fa988ed683847f598aeaea4eac6 Mon Sep 17 00:00:00 2001 From: Ben Kennedy Date: Fri, 6 Nov 2026 22:28:32 +0000 Subject: [PATCH 1/4] MD-3032 Fix element-by-element range cuts when range components share elements Only the last end tag callback registered on an element runs. When the containers or markers of two range components overlapped, one component's callback replaced the other's, so a container's depth never went back to 0 and every later container of that component was skipped by the marker pass while the replacement pass still counted it. The two passes then disagreed on which markers belonged to which range. One handler on every range container and descendant now starts each element, visits it for each range whose container it is in (in place of a `${container} *` handler per component), and lets the range handlers share one end tag callback for it. It is registered with the first range component, so components ahead of the ranges keep seeing elements before a range removes them. A component registered between two range components now sees the later range's cut first, as it does when the wrapper cuts the page. --- .changeset/range-components-share-end-tags.md | 5 + src/stages/buildReplacementHandlers.ts | 75 +++++---- src/stages/findMarkerPositions.ts | 147 +++++++++++------- src/stages/rangeElements.ts | 65 ++++++++ src/stages/surfaceComponents.ts | 79 ++++++++-- test/stages/surfaceComponents.spec.ts | 38 ++++- 6 files changed, 295 insertions(+), 114 deletions(-) create mode 100644 .changeset/range-components-share-end-tags.md create mode 100644 src/stages/rangeElements.ts diff --git a/.changeset/range-components-share-end-tags.md b/.changeset/range-components-share-end-tags.md new file mode 100644 index 00000000..b3780de1 --- /dev/null +++ b/.changeset/range-components-share-end-tags.md @@ -0,0 +1,5 @@ +--- +"@monetizationos/proxy": patch +--- + +Fix range replacement cut element by element when the containers or markers of several range components share elements. Only the last end tag callback registered on an element runs, so one range's callback dropped another's, and every later container of that range was cut from the wrong markers. Range handlers now share one end tag callback per element. diff --git a/src/stages/buildReplacementHandlers.ts b/src/stages/buildReplacementHandlers.ts index 2f783ac6..6d260c30 100644 --- a/src/stages/buildReplacementHandlers.ts +++ b/src/stages/buildReplacementHandlers.ts @@ -3,8 +3,8 @@ import type { HtmlStageContext } from '../context' import type { SubSurfaceBehaviorApi } from '../types' import { renderElement } from './elements' import type { ComponentRange } from './findMarkerPositions' +import type { RangeElements, RangeVisitor } from './rangeElements' import { isVoidElement } from './voidElements' -import { combineSelectors } from './wrappedRange' type ReplacementState = { // `IN_START`: a start marker nested in the first one doesn't start the range again @@ -13,20 +13,28 @@ type ReplacementState = { export type ReplacementHandler = { selector: string; handler: ElementHandlers } +export type ReplacementHandlers = { + container: ReplacementHandler + /** Visits each element in a container, from the shared handler */ + visitor: RangeVisitor +} + /** * Requires a prior pass by `findMarkerPositions` to populate `ranges`. Depends on * `element.onEndTag` to detect element boundaries — on adapters without `capabilities.onEndTag`, - * the caller should skip this step. + * the caller should skip this step. The `visitor` runs from a `createRangeElements` handler, + * which also provides `onEnd`. */ export const buildReplacementHandlers = ( ctx: HtmlStageContext, subSurface: SubSurfaceBehaviorApi, ranges: ComponentRange[] | undefined, -): ReplacementHandler[] => { + onEnd: RangeElements['onEnd'], +): ReplacementHandlers | null => { const { logger } = ctx const modification = subSurface.content?.replaceRange if (!modification || !ranges) { - return [] + return null } if (modification.fromMarker && modification.toMarker && modification.fromMarker === modification.toMarker) { @@ -36,7 +44,7 @@ export const buildReplacementHandlers = ( message: 'Range replacement skipped: fromMarker and toMarker are identical.', context: { marker: modification.fromMarker, selector: subSurface.metadata.cssSelector }, }) - return [] + return null } let elementCounter = 0 @@ -73,15 +81,11 @@ export const buildReplacementHandlers = ( element.after(content, options) } state.operation = 'IN_START' - if (element.onEndTag && !isVoidElement(element)) { - try { - element.onEndTag(() => { - state.operation = 'REMOVING' - }) - } catch { + if ( + !onEnd(element, () => { state.operation = 'REMOVING' - } - } else { + }) + ) { state.operation = 'REMOVING' } } @@ -122,29 +126,25 @@ export const buildReplacementHandlers = ( states.push(state) } - 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(() => { - depth-- - if (state) { - states.pop() - } - }) - depth++ - } - } catch { - // `onEndTag` throws on a self-closing foreign container (``), which has no end tag or content to cut - if (state) { - states.pop() + if (state?.operation === 'REMOVING') { + for (const prepend of (modification.replaceWith ?? []).slice().reverse()) { + const [content, options] = renderElement(ctx, prepend) + element.prepend(content, options) } } + if ( + onEnd(element, () => { + depth-- + if (state) { + states.pop() + } + }) + ) { + depth++ + } else if (state) { + // A self-closing foreign container (``) has no end tag or content to cut + states.pop() + } }, text: (text: RewriterText) => { @@ -157,10 +157,7 @@ export const buildReplacementHandlers = ( const selector = subSurface.metadata.cssSelector if (!selector) { - return [] + return null } - return [ - { selector, handler: parentHandler }, - { selector: combineSelectors(selector, ' ', '*'), handler: { element: onChildElement } }, - ] + return { container: { selector, handler: parentHandler }, visitor: { open: () => depth > 0, child: onChildElement } } } diff --git a/src/stages/findMarkerPositions.ts b/src/stages/findMarkerPositions.ts index eed61b41..8ea78017 100644 --- a/src/stages/findMarkerPositions.ts +++ b/src/stages/findMarkerPositions.ts @@ -1,7 +1,7 @@ import type { ElementHandlers, HtmlRewriterAdapter, RewriterElement } from '../adapters/HtmlRewriterAdapter' import type { HtmlStageContext } from '../context' import type { SurfaceDecisionResponse } from '../types' -import { isVoidElement } from './voidElements' +import { createRangeElements, type RangeElements, type RangeVisitor } from './rangeElements' import { combineSelectors } from './wrappedRange' export type SurfaceMarkers = { @@ -21,6 +21,7 @@ export type Marker = { } type Handler = { selector: string; componentKey: string; handler: ElementHandlers } +type ComponentHandlers = { componentKey: string; container: string; visitor: RangeVisitor; handlers: Handler[] } /** * For any modifications with start or end markers, find the positions of those markers. @@ -45,7 +46,8 @@ export const findMarkerPositions = async ( Object.entries(surfaceDecisions.componentBehaviors).map(([key]) => [key, []]), ) - const handlers: Handler[] = Object.entries(surfaceDecisions.componentBehaviors).flatMap( + let elements: RangeElements | null = null + const components: ComponentHandlers[] = Object.entries(surfaceDecisions.componentBehaviors).flatMap( ([componentKey, componentBehavior]) => { const modification = componentBehavior.content?.replaceRange if (!componentBehavior.metadata?.cssSelector || !modification) { @@ -68,46 +70,13 @@ export const findMarkerPositions = async ( } 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 - } - } - }, - }, - }, - { - selector: combineSelectors(componentBehavior.metadata.cssSelector, ' ', '*'), - componentKey, - handler: { - element(element: RewriterElement) { - if (!currentRange) { - logInvalidState('child-element', componentKey) - return - } - ++elementCounter - const marker: Marker = { - endMarker: false, - startMarker: false, - endTag: null, - nextEndMarker: null, - } - currentRange.markers[elementCounter] = marker - if (element.onEndTag && !isVoidElement(element)) { - try { - element.onEndTag(() => { - marker.endTag = elementCounter - }) - } catch { - marker.endTag = elementCounter - } - } else { - marker.endTag = elementCounter + // With no end tag, nothing can be nested in it + if ( + elements?.onEnd(element, () => { + depth-- + }) + ) { + depth++ } }, }, @@ -157,29 +126,93 @@ export const findMarkerPositions = async ( } : null, ] - return entries.filter((e): e is Handler => !!e) + const visitor: RangeVisitor = { + open: () => depth > 0, + child(element: RewriterElement) { + if (!currentRange) { + logInvalidState('child-element', componentKey) + return + } + ++elementCounter + const marker: Marker = { + endMarker: false, + startMarker: false, + endTag: null, + nextEndMarker: null, + } + currentRange.markers[elementCounter] = marker + if ( + !elements?.onEnd(element, () => { + marker.endTag = elementCounter + }) + ) { + marker.endTag = elementCounter + } + }, + } + return [ + { + componentKey, + container: componentBehavior.metadata.cssSelector, + visitor, + handlers: entries.filter((e): e is Handler => !!e), + }, + ] }, ) - if (!handlers.length || !response.body) { + if (!components.length || !response.body) { return { markers, componentsWithInvalidSelectors: [] } } const componentsWithInvalidSelectors: string[] = [] - let session = rewriter.create() - for (const { selector, handler, componentKey } of handlers) { - try { - session = session.on(selector, handler) - } catch (error) { - ctx.logger.log({ - level: 'error', - code: 'marker-pass-selector-failed', - message: 'Failed to register marker-pass handler for component selector; skipping range replacement for this component.', - context: { selector, componentKey }, - error, - }) - componentsWithInvalidSelectors.push(componentKey) + const logFailure = (selector: string, componentKey: string, error: unknown) => { + ctx.logger.log({ + level: 'error', + code: 'marker-pass-selector-failed', + message: 'Failed to register marker-pass handler for component selector; skipping range replacement for this component.', + context: { selector, componentKey }, + error, + }) + componentsWithInvalidSelectors.push(componentKey) + } + const register = (included: ComponentHandlers[]) => { + elements = createRangeElements( + included.map(({ container }) => container), + included.map(({ visitor }) => visitor), + ) + let session = rewriter.create().on(elements.handler.selector, elements.handler.handler) + for (const { handlers } of included) { + for (const { selector, handler } of handlers) { + session = session.on(selector, handler) + } + } + return session + } + // A component whose handlers don't all register is dropped, so the shared handler only covers valid containers. + // `${container} *` is part of the shared handler's selector, so it is checked here too. + const isValid = ({ componentKey, container, handlers }: ComponentHandlers) => { + let probe = rewriter.create() + for (const selector of [container, combineSelectors(container, ' ', '*'), ...handlers.map((h) => h.selector)]) { + try { + probe = probe.on(selector, {}) + } catch (error) { + logFailure(selector, componentKey, error) + return false + } + } + return true + } + + let session: ReturnType + try { + session = register(components) + } catch { + const valid = components.filter(isValid) + if (!valid.length) { + return { markers, componentsWithInvalidSelectors } } + session = register(valid) } await session.transform(response.clone()).text() diff --git a/src/stages/rangeElements.ts b/src/stages/rangeElements.ts new file mode 100644 index 00000000..42a287b1 --- /dev/null +++ b/src/stages/rangeElements.ts @@ -0,0 +1,65 @@ +import type { ElementHandlers, RewriterElement } from '../adapters/HtmlRewriterAdapter' +import { isVoidElement } from './voidElements' +import { combineSelectors } from './wrappedRange' + +export type RangeVisitor = { + /** A container is open, so the element is in it */ + open: () => boolean + child: (element: RewriterElement) => void +} + +export type RangeElements = { + /** Registered ahead of the range handlers, on every container and everything in one. */ + handler: { selector: string; handler: ElementHandlers } + /** Runs `callback` at the element's end tag; false when it has none. */ + onEnd: (element: RewriterElement, callback: () => void) => boolean +} + +/** + * Only the last end tag callback registered on an element runs, so range handlers from several components + * share one per element. One handler on every container and descendant starts each element and visits it for + * the components whose container it is in, in place of a `${container} *` handler per component. + */ +export const createRangeElements = (containers: string[], visitors: RangeVisitor[]): RangeElements => { + let current: (() => void)[] | null = null + let started = false + return { + handler: { + selector: containers.flatMap((container) => [container, combineSelectors(container, ' ', '*')]).join(', '), + handler: { + element(element: RewriterElement) { + current = null + started = false + for (const visitor of visitors) { + if (visitor.open()) { + visitor.child(element) + } + } + }, + }, + }, + onEnd(element, callback) { + if (started) { + current?.push(callback) + return current !== null + } + started = true + if (!element.onEndTag || isVoidElement(element)) { + return false + } + const callbacks = [callback] + try { + element.onEndTag(() => { + for (const run of callbacks) { + run() + } + }) + } catch { + // Self-closing, so no end tag + return false + } + current = callbacks + return true + }, + } +} diff --git a/src/stages/surfaceComponents.ts b/src/stages/surfaceComponents.ts index cfdad5e9..44969860 100644 --- a/src/stages/surfaceComponents.ts +++ b/src/stages/surfaceComponents.ts @@ -1,9 +1,10 @@ import type { HtmlRewriterAdapter } from '../adapters/HtmlRewriterAdapter' import type { ComponentStageContext } from '../context' import type { SurfaceDecisionResponse } from '../types' -import { buildReplacementHandlers } from './buildReplacementHandlers' +import { buildReplacementHandlers, type ReplacementHandlers } from './buildReplacementHandlers' import { ContentElementHandler } from './ContentElementHandler' import { findMarkerPositions } from './findMarkerPositions' +import { createRangeElements, type RangeElements } from './rangeElements' import { planRangeWrapper, type RangeWrapper } from './wrappedRange' export default async function handleSurfaceComponents( @@ -74,31 +75,28 @@ export async function applyComponents( // Ahead of every other component, so what they insert lands outside the wrapper registerWrapper(wrapper?.handlers ?? []) - Object.entries(surfaceDecisions.componentBehaviors).forEach(([componentKey, componentBehavior]) => { - if (!componentBehavior.metadata.cssSelector || !componentBehavior.content) { - return + let elements: RangeElements | null = null + const onEnd: RangeElements['onEnd'] = (element, callback) => elements?.onEnd(element, callback) ?? false + const components = Object.entries(surfaceDecisions.componentBehaviors).flatMap(([componentKey, componentBehavior]) => { + const selector = componentBehavior.metadata.cssSelector + if (!selector || !componentBehavior.content) { + return [] } - if (componentBehavior.metadata.cssSelector.includes(':last-child')) { + if (selector.includes(':last-child')) { logger.log({ level: 'warn', code: 'component-selector-unsupported', message: 'Ignoring component with unsupported CSS selector.', - context: { componentKey, selector: componentBehavior.metadata.cssSelector }, + context: { componentKey, selector }, }) - return + return [] } - const handlers: { selector: string; handler: Parameters[1] }[] = [ - { - selector: componentBehavior.metadata.cssSelector, - handler: new ContentElementHandler(ctx, componentBehavior.content), - }, - ] - + let range: ReplacementHandlers | null = null const cutByWrapper = wrapper?.componentKeys.includes(componentKey) if (supportsRangeReplacement && !cutByWrapper && !componentsWithInvalidSelectors.includes(componentKey)) { - handlers.push(...buildReplacementHandlers(ctx, componentBehavior, markers[componentKey])) + range = buildReplacementHandlers(ctx, componentBehavior, markers[componentKey], onEnd) } else if (!supportsRangeReplacement && componentBehavior.content.replaceRange) { logger.log({ level: 'warn', @@ -107,6 +105,53 @@ export async function applyComponents( context: { componentKey }, }) } + return [{ componentKey, selector, content: componentBehavior.content, range }] + }) + + const ranged = components.flatMap(({ range }) => (range ? [range] : [])) + if (ranged.length) { + elements = createRangeElements( + ranged.map(({ container }) => container.selector), + ranged.map(({ visitor }) => visitor), + ) + } + // Registered with the first range component, so it starts each element before any range handler runs and + // components ahead of the ranges still see an element before a range removes it + let elementsRegistered = false + const registerElements = () => { + if (!elements || elementsRegistered) { + return + } + elementsRegistered = true + try { + session = session.on(elements.handler.selector, elements.handler.handler) + doRewrite = true + } catch (error) { + // The marker pass registered these selectors, so this is not expected + logger.log({ + level: 'error', + code: 'component-transform-failed', + message: 'Failed to register range transforms; skipping range replacement.', + context: { selector: elements.handler.selector }, + error, + }) + elements = null + } + } + + for (const { componentKey, selector: componentSelector, content, range } of components) { + if (range) { + registerElements() + } + const handlers: { selector: string; handler: Parameters[1] }[] = [ + { + selector: componentSelector, + handler: new ContentElementHandler(ctx, content), + }, + ] + if (range && elements) { + handlers.push(range.container) + } for (const { selector, handler } of handlers) { try { @@ -117,12 +162,12 @@ export async function applyComponents( level: 'error', code: 'component-transform-failed', message: 'Failed to register component transform; skipping this handler.', - context: { componentKey, selector: componentBehavior.metadata.cssSelector }, + context: { componentKey, selector: componentSelector }, error, }) } } - }) + } if (config.injectScriptUrl) { const scriptUrl = config.injectScriptUrl diff --git a/test/stages/surfaceComponents.spec.ts b/test/stages/surfaceComponents.spec.ts index b7b79220..2e5e1f6d 100644 --- a/test/stages/surfaceComponents.spec.ts +++ b/test/stages/surfaceComponents.spec.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest' import { normalizeMOSConfig } from '../../src/config' import type { PipelineContext } from '../../src/context' import type { MOSProxyLogEvent, MOSProxyLogger } from '../../src/logger' -import handleSurfaceComponents from '../../src/stages/surfaceComponents' +import handleSurfaceComponents, { applyComponents } from '../../src/stages/surfaceComponents' import type { MOSConfigInput, SubSurfaceBehaviorApi, SurfaceDecisionResponse } from '../../src/types' import { LolHtmlRewriter } from '../fakes/LolHtmlRewriter' @@ -404,6 +404,42 @@ describe('handleSurfaceComponents', () => { }) }) + describe('several range components cut element by element', () => { + type Range = { fromMarker?: string; toMarker?: string } + const range = (cssSelector: string, marker: Range, content: string): SubSurfaceBehaviorApi => ({ + metadata: { cssSelector }, + content: { replaceRange: { ...marker, replaceWith: [{ type: 'html', content }] } }, + }) + const cut = async (body: string, ranges: Record) => + (await applyComponents(baseCtx, html(body), decisions(ranges), new LolHtmlRewriter(), null)).text() + const a = (marker: Range) => range('.a', marker, 'A') + const b = (marker: Range) => range('.b', marker, 'B') + + it('cuts a container in another range container, and the ones after it', async () => { + const container = '

x

y

' + const body = await cut(`
${container}

t

${container}
${container}`, { + a: a({ toMarker: 'hr' }), + b: b({ toMarker: 'h3' }), + }) + const kept = '
A

y

' + expect(body).toBe(`
B

t

${kept}
${kept}`) + }) + + it('cuts every container that is also the container of another range', async () => { + const container = '

x

y

t

z

' + const body = await cut(container + container, { a: a({ toMarker: 'hr' }), b: range('div', { fromMarker: 'h3' }, 'B') }) + const both = '
A

y

t

B
' + expect(body).toBe(both + both) + }) + + it('cuts after a start marker that is the container of another range, in either order', async () => { + const page = '

x

y

t

z

w

' + const expected = '

x

B

t

z

A
' + expect(await cut(page, { a: a({ fromMarker: '.s' }), b: b({ toMarker: 'h3' }) })).toBe(expected) + expect(await cut(page, { b: b({ toMarker: 'h3' }), a: a({ fromMarker: '.s' }) })).toBe(expected) + }) + }) + 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 aeb0fe9900b76d6f8fbb386e12a859f8f9464a99 Mon Sep 17 00:00:00 2001 From: Ben Kennedy Date: Fri, 6 Nov 2026 22:28:32 +0000 Subject: [PATCH 2/4] MD-3032 Run the Cloudflare proxy's truncation cases on both range paths --- test/stages/proxyTruncation.spec.ts | 451 ++++++++++++++++++++++++++++ 1 file changed, 451 insertions(+) create mode 100644 test/stages/proxyTruncation.spec.ts diff --git a/test/stages/proxyTruncation.spec.ts b/test/stages/proxyTruncation.spec.ts new file mode 100644 index 00000000..0ef2d9ae --- /dev/null +++ b/test/stages/proxyTruncation.spec.ts @@ -0,0 +1,451 @@ +import { describe, expect, it } from 'vitest' +import { normalizeMOSConfig } from '../../src/config' +import type { PipelineContext } from '../../src/context' +import handleSurfaceComponents, { applyComponents } from '../../src/stages/surfaceComponents' +import type { SurfaceDecisionResponse, WebContentSurfaceBehavior } from '../../src/types' +import { LolHtmlRewriter } from '../fakes/LolHtmlRewriter' + +// Ported from the Cloudflare proxy's truncation tests, so a release keeps the proxies' suites passing + +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() {} }, +} + +const decisions = (componentBehaviors: SurfaceDecisionResponse['componentBehaviors']): SurfaceDecisionResponse => ({ + status: 'success', + identity: { identifier: 'id', isAuthenticated: false, authType: 'anonymous', jwtClaims: {} }, + features: {}, + customer: { hasProducts: false }, + surfaceBehavior: {}, + componentsSkipped: false, + componentBehaviors, +}) + +/** The page cut as the proxy cuts it, and cut element by element, which is where the proxy falls back to. */ +const bothPaths = async (page: string, decision: SurfaceDecisionResponse) => { + const response = () => new Response(page, { headers: { 'Content-Type': 'text/html; charset=utf-8' } }) + const proxy = await (await handleSurfaceComponents(ctx, response(), decision, new LolHtmlRewriter())).text() + const elementByElement = await (await applyComponents(ctx, response(), decision, new LolHtmlRewriter(), null)).text() + return { proxy, elementByElement } +} + +describe('range replacement on proxy pages', () => { + it.each([ + { + name: 'Empty modification set', + cssSelector: 'p', + content: { + replaceRange: null, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

', + expected: `

First text

SubTitle

`, + }, + { + name: 'Replace entire contents', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }] }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

', + expected: `

REPLACEMENT

`, + }, + { + name: 'Replace entire with multiple values', + cssSelector: 'p', + content: { + replaceRange: { + replaceWith: [ + { type: 'text', content: 'REPLACEMENT1' }, + { type: 'html', content: '

REPLACEMENT2

' }, + ], + }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

', + expected: `

REPLACEMENT1

REPLACEMENT2

`, + }, + { + name: 'Replace everything after the second paragraph', + cssSelector: 'div', + content: { + replaceRange: { fromMarker: 'p:nth-child(2)', replaceWith: [{ type: 'text', content: 'REPLACEMENT' }] }, + } satisfies WebContentSurfaceBehavior, + original: '

First

Outside

Second

Outside

Third

', + expected: `

First

Outside

Second

REPLACEMENT
`, + }, + { + name: 'Remove entire contents', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [] }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

', + expected: `

`, + }, + { + name: 'Replace entire contents multiple', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }] }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle


Second text

SubTitle

', + expected: `

REPLACEMENT


REPLACEMENT

`, + }, + { + name: 'Replace up to tag', + cssSelector: 'p', + content: { + replaceRange: { + replaceWith: [ + { type: 'text', content: 'REPLACEMENT1' }, + { type: 'html', content: '

REPLACEMENT2

' }, + ], + toMarker: 'h2', + }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

Second text.

', + expected: `

REPLACEMENT1

REPLACEMENT2

SubTitle

Second text.

`, + }, + { + name: 'Replace up to self-closing tag', + cssSelector: 'p', + content: { + replaceRange: { + replaceWith: [ + { type: 'text', content: 'REPLACEMENT1' }, + { type: 'html', content: '

REPLACEMENT2

' }, + ], + toMarker: 'br', + }, + } satisfies WebContentSurfaceBehavior, + original: '

First text
Second text.

', + expected: `

REPLACEMENT1

REPLACEMENT2


Second text.

`, + }, + { + name: 'Replace up to tag with multiple values', + cssSelector: 'p', + content: { + replaceRange: { + replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], + toMarker: 'h2', + }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

Second text.

', + expected: `

REPLACEMENT

SubTitle

Second text.

`, + }, + { + name: 'Replace up to malformed tag', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], toMarker: 'h2' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle Second text.

', + expected: `

REPLACEMENT

SubTitle Second text.

`, + }, + { + name: 'Replace up to tag inside container', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], toMarker: 'h2' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

Second text.

', + expected: `

REPLACEMENT

SubTitle

Second text.

`, + }, + { + name: 'Replace up to tag', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], toMarker: 'h2' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

Subtitle

Subtitle

SubTitle

Second text.

', + expected: `

REPLACEMENT

SubTitle

Second text.

`, + }, + { + name: 'Replace up to tag inside container', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], toMarker: 'h2' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

Subtitle

Subtitle

SubTitle

Second text.

', + expected: `

REPLACEMENT

SubTitle

Second text.

`, + }, + { + name: 'Replace up to tag that does not exist replaces all content', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], toMarker: 'h5' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

Second text.

', + expected: `

REPLACEMENT

`, + }, + { + name: 'Replace up to tag multiple', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], toMarker: 'h2' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

Second text.


Third text

SubTitle

Fourth text.

', + expected: `

REPLACEMENT

SubTitle

Second text.


REPLACEMENT

SubTitle

Fourth text.

`, + }, + { + name: 'Replace from tag', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], fromMarker: 'h2' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

Second text.

', + expected: `

First text

SubTitle

REPLACEMENT

`, + }, + { + name: 'Replace from tag multiple', + cssSelector: 'p', + content: { + replaceRange: { + replaceWith: [ + { type: 'text', content: 'REPLACEMENT1' }, + { type: 'text', content: 'REPLACEMENT2' }, + ], + fromMarker: 'h2', + }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

Second text.

', + expected: `

First text

SubTitle

REPLACEMENT1REPLACEMENT2

`, + }, + { + name: 'Replace from self-closing tag', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], fromMarker: 'br' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text
Second text.

', + expected: `

First text
REPLACEMENT

`, + }, + { + name: 'Replace from tag that does not exist replaces no content', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], fromMarker: 'h5' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

Second text.

', + expected: '

First text

SubTitle

Second text.

', + }, + { + name: 'Replace from tag multiple', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], fromMarker: 'h2' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

Second text.


Third text

SubTitle

Fourth text.

', + expected: `

First text

SubTitle

REPLACEMENT


Third text

SubTitle

REPLACEMENT

`, + }, + { + name: 'Replace between tags', + cssSelector: 'p', + content: { + replaceRange: { + replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], + fromMarker: 'h2', + toMarker: 'h3', + }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

Second text.

SubTitle 2

Third text.

', + expected: `

First text

SubTitle

REPLACEMENT

SubTitle 2

Third text.

`, + }, + { + name: 'Replace between tags in container', + cssSelector: 'p', + content: { + replaceRange: { + replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], + fromMarker: 'h2', + toMarker: 'h3', + }, + } satisfies WebContentSurfaceBehavior, + original: '

Prefix

First text

SubTitle

Second text.

SubTitle 2

Third text.

', + expected: `

Prefix

First text

SubTitle

REPLACEMENT

SubTitle 2

Third text.

`, + }, + { + name: 'Replace between tags nested', + cssSelector: 'p', + content: { + replaceRange: { + replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], + fromMarker: 'h2', + toMarker: 'h3', + }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

Second text.

SubTitle 2

Third text.

', + expected: `

First text

SubTitle

REPLACEMENT

SubTitle 2

Third text.

`, + }, + { + name: 'Replace between tags nested multiple', + cssSelector: 'p', + content: { + replaceRange: { + replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], + fromMarker: 'h2', + toMarker: 'h3', + }, + } satisfies WebContentSurfaceBehavior, + original: + '

First text

SubTitle

Second text.

SubTitle 2

Third text.

First text

SubTitle

Second text.

SubTitle 2

Third text.

', + expected: `

First text

SubTitle

REPLACEMENT

SubTitle 2

Third text.

First text

SubTitle

REPLACEMENT

SubTitle 2

Third text.

`, + }, + { + name: 'Replace from nested tag', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], fromMarker: 'h2' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

Second text.

SubTitle 2

Third text.

', + expected: `

First text

SubTitle

REPLACEMENT

`, + }, + { + name: 'Replace to nested tag', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], toMarker: 'h2' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

Second text.

SubTitle 2

Third text.

', + expected: `

REPLACEMENT

SubTitle

Second text.

SubTitle 2

Third text.

`, + }, + { + name: 'Replace between tags that do not exist', + cssSelector: 'p', + content: { + replaceRange: { + replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], + fromMarker: 'h4', + toMarker: 'h5', + }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

SubTitle

Second text.

SubTitle 2

Third text.

', + expected: '

First text

SubTitle

Second text.

SubTitle 2

Third text.

', + }, + { + name: 'Ignores :last-child selector by skipping range replacement', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], toMarker: 'h4:last-child' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

Subtitle

Subtitle

SubTitle

Second text.

', + expected: `

First text

Subtitle

Subtitle

SubTitle

Second text.

`, + }, + { + name: 'Replaces up to wildcard', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], toMarker: '*' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

Subtitle

Subtitle

SubTitle

Second text.

', + expected: `

REPLACEMENT

Subtitle

Subtitle

SubTitle

Second text.

`, + }, + { + name: 'Replaces from wildcard by replacing after the first element', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], fromMarker: '*' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

Subtitle

Subtitle

SubTitle

Second text.

', + expected: '

First text

Subtitle

REPLACEMENT

', + }, + { + name: 'Handles from and to the same selector by ignoring the replacement', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], fromMarker: 'h2', toMarker: 'h2' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

Subtitle

Subtitle

SubTitle

Second text.

', + expected: '

First text

Subtitle

Subtitle

SubTitle

Second text.

', + }, + { + name: 'Handles from and to wildcard by ignoring the replacement', + cssSelector: 'p', + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], fromMarker: '*', toMarker: '*' }, + } satisfies WebContentSurfaceBehavior, + original: '

First text

Subtitle

Subtitle

SubTitle

Second text.

', + expected: '

First text

Subtitle

Subtitle

SubTitle

Second text.

', + }, + ])('truncates HTML component content - $name', async ({ content, original, expected, cssSelector }) => { + const page = (body: string) => `${body}` + const decision = decisions({ test: { metadata: { cssSelector }, content: content as WebContentSurfaceBehavior } }) + expect(await bothPaths(page(original), decision)).toEqual({ proxy: page(expected), elementByElement: page(expected) }) + }) + + it('performs multiple truncations', async () => { + const page = ` + + + +

First text

SubTitle

+

First text

Second Text

SubTitle

Third Text

+

Some other text

+

First text

SubTitle

Second text

+

First text

SubTitle

Second text

+

First text

SubTitle

First text

SubTitle

+

First text

SubTitle

Second text

SubTitle

Second text

text

SubTitle

text

+

Prefix

First text

SubTitle

Second text

+

First text

SubTitle

Second text

+ +` + const decision = decisions({ + c1: { + metadata: { cssSelector: '.c1' }, + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], toMarker: 'h2' }, + }, + }, + c2: { + metadata: { cssSelector: '.c2' }, + content: { + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], toMarker: 'h3' }, + }, + }, + p: { + metadata: { cssSelector: 'p' }, + content: { + remove: false, + }, + }, + c3: { + metadata: { cssSelector: '.c3' }, + content: { + prepend: [{ type: 'text', content: 'PREPEND' }], + replaceRange: { replaceWith: [{ type: 'text', content: 'REPLACEMENT' }], fromMarker: 'h3' }, + append: [{ type: 'text', content: 'APPEND' }], + after: [{ type: 'text', content: 'AFTER' }], + }, + }, + body: { + metadata: { cssSelector: 'body' }, + content: { + append: [{ type: 'text', content: 'APPEND' }], + }, + }, + }) + const expected = ` + + + +

REPLACEMENT

SubTitle

+

REPLACEMENT

SubTitle

Third Text

+

Some other text

+

REPLACEMENT

SubTitle

Second text

+

REPLACEMENT

SubTitle

Second text

+

REPLACEMENT

SubTitle

REPLACEMENT

SubTitle

+

REPLACEMENT

SubTitle

Second text

REPLACEMENT

SubTitle

text

+

REPLACEMENT

SubTitle

Second text

+

PREPENDFirst text

SubTitle

REPLACEMENTAPPEND

AFTER +APPEND +` + expect(await bothPaths(page, decision)).toEqual({ proxy: expected, elementByElement: expected }) + }) +}) From 4da4d138b0bfde74e238d0d8a42eba02b7c2b45f Mon Sep 17 00:00:00 2001 From: Ben Kennedy Date: Fri, 6 Nov 2026 20:34:28 +0000 Subject: [PATCH 3/4] MD-3032 Cut nested containers by their own ranges A container nested in another container of the same range is now cut by its own range. The outer container's cut goes around it, keeps what holds it, and ignores the markers inside it, which belong to the innermost container. The Cloudflare proxy's nested `.c2` truncation case is back to its original output. Element by element, both passes keep a stack of open containers in place of the depth counter, each element goes to the innermost container's range, and a range records where its nested containers start so its cut keeps them and the elements holding them. A void container has nothing to cut, so the container it is in still cuts it. The wrapper keeps the same stack per range. A nested container that is a direct child of the outer container's cut closes the cut before it and reopens it after, without a second replacement. A marker is the innermost container's, so a marker directly in a nested container no longer sends the page element by element; a marker on a nested container is the next container out's. A nested container deeper in the cut, in raw text or MathML, on a page with a nested start marker, or closed by another element's end tag still goes element by element. The last needs the end tag's name, so `onEndTag` callbacks now receive it as `RewriterEndTag`; native rewriters already pass it. --- .../range-nested-containers-own-ranges.md | 5 + src/adapters/HtmlRewriterAdapter.ts | 7 +- src/adapters/index.ts | 1 + src/index.ts | 1 + src/stages/buildReplacementHandlers.ts | 57 +++--- src/stages/findMarkerPositions.ts | 60 +++--- src/stages/wrappedRange.ts | 180 +++++++++++------- test/fakes/LolHtmlRewriter.ts | 4 +- test/stages/proxyTruncation.spec.ts | 2 +- test/stages/surfaceComponents.spec.ts | 14 +- test/stages/wrappedRange.spec.ts | 91 +++++++-- 11 files changed, 276 insertions(+), 146 deletions(-) create mode 100644 .changeset/range-nested-containers-own-ranges.md diff --git a/.changeset/range-nested-containers-own-ranges.md b/.changeset/range-nested-containers-own-ranges.md new file mode 100644 index 00000000..e88e0a25 --- /dev/null +++ b/.changeset/range-nested-containers-own-ranges.md @@ -0,0 +1,5 @@ +--- +"@monetizationos/proxy": minor +--- + +A range's container nested in another of its containers is now cut by its own range. The outer container's cut goes around it and ignores the markers inside it, and keeps what holds it. Previously only the outermost container was cut. A nested container deeper than a direct child of the outer container's cut, or on a page whose start marker needs a nested cut, is cut element by element. diff --git a/src/adapters/HtmlRewriterAdapter.ts b/src/adapters/HtmlRewriterAdapter.ts index ef2b9d01..56ba3d52 100644 --- a/src/adapters/HtmlRewriterAdapter.ts +++ b/src/adapters/HtmlRewriterAdapter.ts @@ -56,7 +56,12 @@ export interface RewriterElement { replace(content: string, options?: ContentOptions): void remove(): void removeAndKeepContent(): void - onEndTag?(callback: () => void | Promise): void + onEndTag?(callback: (endTag?: RewriterEndTag) => void | Promise): void +} + +/** The end tag that closed an element. Its name differs from the element's when another element's end tag closed it. */ +export interface RewriterEndTag { + readonly name: string } export interface RewriterText { diff --git a/src/adapters/index.ts b/src/adapters/index.ts index c2652213..7e93b5b4 100644 --- a/src/adapters/index.ts +++ b/src/adapters/index.ts @@ -15,6 +15,7 @@ export type { HtmlRewriterCapabilities, HtmlRewriterSession, RewriterElement, + RewriterEndTag, RewriterText, } from './HtmlRewriterAdapter' export type { Identity, IdentityProvider, PersistIdentityArgs, ResolveIdentityArgs } from './IdentityProvider' diff --git a/src/index.ts b/src/index.ts index 697ea80f..123d1e1e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -15,6 +15,7 @@ export type { Resource, ResourceProvider, RewriterElement, + RewriterEndTag, RewriterText, UnresolvedConfigContext, UnresolvedConfigHandler, diff --git a/src/stages/buildReplacementHandlers.ts b/src/stages/buildReplacementHandlers.ts index 6d260c30..2fc956db 100644 --- a/src/stages/buildReplacementHandlers.ts +++ b/src/stages/buildReplacementHandlers.ts @@ -49,10 +49,11 @@ export const buildReplacementHandlers = ( let elementCounter = 0 let rangeIndex = -1 - const states: ReplacementState[] = [] + // Open containers, innermost last; each element is in the range of the innermost one. A removed container has no state. + const open: { state: ReplacementState | null; range: ComponentRange | undefined }[] = [] const getCurrentState = () => { - const state = states[states.length - 1] - if (!state) { + const frame = open.at(-1) + if (!frame) { logger.log({ level: 'warn', code: 'replacement-state-missing', @@ -61,7 +62,7 @@ export const buildReplacementHandlers = ( }) return } - return state + return frame.state ?? undefined } const onChildElement = (element: RewriterElement) => { @@ -71,7 +72,7 @@ export const buildReplacementHandlers = ( } const state = getCurrentState() - const range = ranges[rangeIndex] + const range = open.at(-1)?.range const marker = range?.markers[elementCounter] if (state?.operation === 'SCANNING') { @@ -102,29 +103,25 @@ export const buildReplacementHandlers = ( return } + // A nested container, or an element holding one, is kept for the nested container's own range + if (range && marker?.endTag && holdsIsland(range.islands, elementCounter, marker.endTag)) { + return + } + element.remove() return } } - // 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 (depth > 0) { - 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 + // A removed container still opens a range, 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) - } if (state?.operation === 'REMOVING') { for (const prepend of (modification.replaceWith ?? []).slice().reverse()) { @@ -132,18 +129,15 @@ export const buildReplacementHandlers = ( element.prepend(content, options) } } + const index = open.length + // A self-closing foreign container (``) has no end tag or content to cut. A container's end also + // closes containers left open in it. if ( onEnd(element, () => { - depth-- - if (state) { - states.pop() - } + open.length = index }) ) { - depth++ - } else if (state) { - // A self-closing foreign container (``) has no end tag or content to cut - states.pop() + open.push({ state, range: ranges[rangeIndex] }) } }, @@ -159,5 +153,20 @@ export const buildReplacementHandlers = ( if (!selector) { return null } - return { container: { selector, handler: parentHandler }, visitor: { open: () => depth > 0, child: onChildElement } } + return { container: { selector, handler: parentHandler }, visitor: { open: () => open.length > 0, child: onChildElement } } +} + +/** Whether element `from` is a nested container or holds one: one starts from it to `to`, its last element. */ +const holdsIsland = (islands: number[], from: number, to: number) => { + let low = 0 + let high = islands.length + while (low < high) { + const mid = (low + high) >> 1 + if ((islands[mid] as number) < from) { + low = mid + 1 + } else { + high = mid + } + } + return low < islands.length && (islands[low] as number) <= to } diff --git a/src/stages/findMarkerPositions.ts b/src/stages/findMarkerPositions.ts index 8ea78017..f4777969 100644 --- a/src/stages/findMarkerPositions.ts +++ b/src/stages/findMarkerPositions.ts @@ -10,7 +10,12 @@ export type SurfaceMarkers = { } export type ComponentRange = { + /** By element number; an element in a nested container is in that container's range instead */ markers: Marker[] + /** The element number before the container's first element */ + start: number + /** Element numbers of the containers nested in this one, in order. The range cuts around them. */ + islands: number[] } export type Marker = { @@ -55,9 +60,10 @@ export const findMarkerPositions = async ( } const { toMarker, fromMarker } = modification let elementCounter = 0 - let currentRange: ComponentRange | null = null - // Open containers; a container inside another one is part of the outer one's range - let depth = 0 + // Open containers, innermost last. Each element is in the range of the innermost one. + const open: ComponentRange[] = [] + // The current element and the range it is in + let current: { range: ComponentRange; marker: Marker } | null = null const entries: (Handler | null)[] = [ { @@ -65,18 +71,19 @@ export const findMarkerPositions = async ( componentKey, handler: { element: (element: RewriterElement) => { - if (depth > 0) { - return - } - currentRange = { markers: [] } - markers[componentKey]?.push(currentRange) - // With no end tag, nothing can be nested in it + const range: ComponentRange = { markers: [], start: elementCounter, islands: [] } + markers[componentKey]?.push(range) + const index = open.length + // With no end tag there is nothing to cut in it, so a container it is in cuts it as any element. + // Its end also closes containers left open in it. if ( elements?.onEnd(element, () => { - depth-- + open.length = index }) ) { - depth++ + // Nested in another container, which cuts around it + open.at(-1)?.islands.push(elementCounter) + open.push(range) } }, }, @@ -87,21 +94,21 @@ export const findMarkerPositions = async ( componentKey, handler: { element() { - if (!currentRange) { + if (!current) { logInvalidState('to-marker', componentKey) return } - const marker = currentRange.markers[elementCounter] - if (marker) { - marker.endMarker = true - } - for (let i = elementCounter; i >= 1; i--) { - const m = currentRange.markers[i] - if (m && !m.nextEndMarker) { - m.nextEndMarker = elementCounter - } else { + const { range, marker } = current + marker.endMarker = true + for (let i = elementCounter; i > range.start; i--) { + // Missing where the element is in a nested container's range + const m = range.markers[i] + if (m?.nextEndMarker) { break } + if (m) { + m.nextEndMarker = elementCounter + } } }, }, @@ -113,22 +120,20 @@ export const findMarkerPositions = async ( componentKey, handler: { element() { - if (!currentRange) { + if (!current) { logInvalidState('from-marker', componentKey) return } - const marker = currentRange.markers[elementCounter] - if (marker) { - marker.startMarker = true - } + current.marker.startMarker = true }, }, } : null, ] const visitor: RangeVisitor = { - open: () => depth > 0, + open: () => open.length > 0, child(element: RewriterElement) { + const currentRange = open.at(-1) if (!currentRange) { logInvalidState('child-element', componentKey) return @@ -141,6 +146,7 @@ export const findMarkerPositions = async ( nextEndMarker: null, } currentRange.markers[elementCounter] = marker + current = { range: currentRange, marker } if ( !elements?.onEnd(element, () => { marker.endTag = elementCounter diff --git a/src/stages/wrappedRange.ts b/src/stages/wrappedRange.ts index 31520add..2eb7fa2b 100644 --- a/src/stages/wrappedRange.ts +++ b/src/stages/wrappedRange.ts @@ -1,4 +1,4 @@ -import type { ElementHandlers, HtmlRewriterAdapter, RewriterElement } from '../adapters/HtmlRewriterAdapter' +import type { ElementHandlers, HtmlRewriterAdapter, RewriterElement, RewriterEndTag } from '../adapters/HtmlRewriterAdapter' import type { HtmlStageContext } from '../context' import type { SurfaceDecisionResponse } from '../types' import { renderElement } from './elements' @@ -67,19 +67,27 @@ export type RangeWrapper = { splice: (page: Uint8Array, rewriter: HtmlRewriterAdapter, headers: Headers) => Promise | null> } -type RangeState = { - depth: number +type ContainerState = { // `inStart`: a start marker nested in the first one doesn't start the range again phase: 'scanning' | 'inStart' | 'open' | 'closed' - // Set by a `>` handler for the check on the same element, which runs after it - directStart: boolean - directEnd: boolean // The start marker was nested, so the range is cut by `splice` nested: boolean // The container isn't HTML, so a fragment of it can't be parsed on its own foreign: boolean } +type RangeState = { + // Open containers, innermost last. A marker is the innermost one's, and a container nested in another is cut by + // its own range while the outer one cuts around it. + open: ContainerState[] + // Set by a `>` handler for the check on the same element, which runs after it + directStart: boolean + directEnd: boolean + directContainer: boolean + // The current element opened a container, so a marker on it is the next one out's + opened: boolean +} + // Starts every sentinel; pages hardly ever hold it const MARK = 1 const COMMENT_OPEN = 4 @@ -88,7 +96,8 @@ const COMMENT_CLOSE = 3 /** * Cuts each range by splitting its container into wrapper elements in one pass and removing the one that holds the * range in the next, so the cut costs one callback instead of one per element. The split only balances when both - * markers are direct children of the outermost container. A nested start marker of the only range instead gets + * markers are direct children of their container. A container nested in another one's cut must be a direct child of + * it, so the cut can close before it and reopen after. A nested start marker of the only range instead gets * sentinels around it and its replacement, and `splice` cuts the bytes from there to the end marker or the container's * end, putting back the end tags of the elements the start marker sits in. Any other nested marker makes the page * unsupported. What other components insert inside the cut is removed with it, and a marker removed by another @@ -122,6 +131,8 @@ export const planRangeWrapper = (ctx: HtmlStageContext, decisions: SurfaceDecisi let unsupported = false let wrapped = 0 let nested = 0 + // Containers nested in another of their range's, which `splice` doesn't expect + let nestedContainers = 0 const sentinel = (index: number, kind: 'o' | 'c' | 's' | 'r' | 'e') => `${String.fromCharCode(MARK)}${nonce}-${index}-${kind}` const comment = (index: number, kind: 's' | 'r' | 'e') => `` // Ranges whose container handler has run for the current element, cleared once its marker handlers have run @@ -135,13 +146,13 @@ export const planRangeWrapper = (ctx: HtmlStageContext, decisions: SurfaceDecisi } // Only the last end tag callback registered on an element runs, so one registered on `math` or `svg` keeps its count - const onEnd = (element: RewriterElement, callback: () => void) => { + const onEnd = (element: RewriterElement, callback: (endTag?: RewriterEndTag) => void) => { const tag = foreignTag(element) element.onEndTag?.( tag - ? () => { + ? (endTag) => { foreign[tag]-- - callback() + callback(endTag) } : callback, ) @@ -173,65 +184,89 @@ export const planRangeWrapper = (ctx: HtmlStageContext, decisions: SurfaceDecisi const cut = `<${tag}>` const keep = `<${tag} data-keep>` const close = `` + // Reopens a cut after a nested container, without a second replacement + const more = `<${tag} data-more>` const replacement = (replaceRange.replaceWith ?? []).map((item) => renderElement(ctx, item)) - const state: RangeState = { depth: 0, phase: 'closed', directStart: false, directEnd: false, nested: false, foreign: false } + const state: RangeState = { open: [], directStart: false, directEnd: false, directContainer: false, opened: false } + const owner = () => state.open.at(state.opened ? -2 : -1) - handlers.push({ - selector: container, - handler: { - element(element: RewriterElement) { - // The same element holding two ranges would need two end tag callbacks - unsupported ||= atContainer++ > 0 - if (isVoidElement(element)) { - return - } - if (!element.onEndTag) { - unsupported = true - return - } - if (state.depth > 0) { - // Only the outermost container is cut; counting this one marks the markers in it as nested + handlers.push( + { + selector: combineSelectors(container, ' > ', container), + handler: { + element() { + state.directContainer = true + }, + }, + }, + { + selector: container, + handler: { + element(element: RewriterElement) { + // The same element holding two ranges would need two end tag callbacks + unsupported ||= atContainer++ > 0 + const direct = state.directContainer + state.directContainer = false + // With no end tag there is nothing to cut in it, so a container it is in cuts it as any element + if (isVoidElement(element)) { + return + } + if (!element.onEndTag) { + unsupported = true + return + } + const outer = state.open.at(-1) + const raw = RAW_TEXT_ELEMENTS.has(element.tagName.toLowerCase()) || foreign.math > 0 + if (outer && (raw || outer.nested || nested > 0)) { + unsupported = true + return + } + if (!outer && raw) { + return + } + const index = state.open.length + const tagName = element.tagName.toLowerCase() try { - onEnd(element, () => { - state.depth-- + onEnd(element, (endTag) => { + // A nested container closed by another element's end tag: the rewriter writes only one of + // the containers' closing wrapper tags there, so they don't balance + unsupported ||= index > 0 && endTag?.name.toLowerCase() !== tagName + state.open.length = index }) - state.depth++ } catch { - // No end tag, so nothing is nested in it + // Self-closing foreign element (``): nothing to cut in it, and no end tag to close a cut at + unsupported ||= !outer + return } - return - } - if (RAW_TEXT_ELEMENTS.has(element.tagName.toLowerCase()) || foreign.math > 0) { - return - } - try { - onEnd(element, () => { - state.phase = 'closed' - // Not a decrement: an inner container left open never runs its own callback - state.depth = 0 - }) - } catch { - // Self-closing foreign element (``), so there is no end tag to close the cut at - unsupported = true - return - } - state.depth++ - state.phase = fromMarker ? 'scanning' : 'open' - state.nested = false - state.foreign = foreign.svg > 0 - // Closed by the container's own end, so what other components append lands outside it. Sentinels - // in the tags let `splice` find the container; pass 2 removes the tags with them. - element.prepend(fromMarker ? `<${tag} data-keep ${sentinel(index, 'o')}>` : cut, html) - element.append(fromMarker ? `` : close, html) - wrapped++ + if (outer) { + nestedContainers++ + if (outer.phase === 'open') { + // Deeper in the cut, the cut can't close around it + if (!direct) { + unsupported = true + return + } + element.before(close, html) + element.after(more, html) + } + } + state.open.push({ phase: fromMarker ? 'scanning' : 'open', nested: false, foreign: foreign.svg > 0 }) + state.opened = true + // Closed by the container's own end, so what other components append lands outside it. Sentinels + // in the tags let `splice` find the container; pass 2 removes the tags with them. + element.prepend(fromMarker ? `<${tag} data-keep ${sentinel(index, 'o')}>` : cut, html) + element.append(fromMarker ? `` : close, html) + wrapped++ + }, }, }, - }) + ) cleanups.push({ selector: container, handler: { element() { atContainer = 0 + state.opened = false }, }, }) @@ -242,7 +277,8 @@ export const planRangeWrapper = (ctx: HtmlStageContext, decisions: SurfaceDecisi selector: combineSelectors(container, ' > ', toMarker), handler: { element() { - state.directEnd = state.depth === 1 + // Its parent is the innermost open container + state.directEnd = true }, }, }, @@ -252,16 +288,17 @@ export const planRangeWrapper = (ctx: HtmlStageContext, decisions: SurfaceDecisi element(element: RewriterElement) { const direct = state.directEnd state.directEnd = false + const range = owner() // Before the start marker, or inside it - if (state.phase !== 'open') { + if (range?.phase !== 'open') { return } if (!direct || atContainer) { unsupported = true return } - element.before(state.nested ? comment(index, 'e') : close + keep, html) - state.phase = 'closed' + element.before(range.nested ? comment(index, 'e') : close + keep, html) + range.phase = 'closed' }, }, }, @@ -273,7 +310,7 @@ export const planRangeWrapper = (ctx: HtmlStageContext, decisions: SurfaceDecisi selector: combineSelectors(container, ' > ', fromMarker), handler: { element() { - state.directStart = state.depth === 1 + state.directStart = true }, }, }, @@ -283,11 +320,12 @@ export const planRangeWrapper = (ctx: HtmlStageContext, decisions: SurfaceDecisi element(element: RewriterElement) { const direct = state.directStart state.directStart = false - if (state.phase !== 'scanning') { + const range = owner() + if (range?.phase !== 'scanning') { return } - // Another range's wrapper tags would come apart in the splice - if (atContainer || (!direct && (ranges.length > 1 || state.foreign || foreign.math > 0))) { + // Another range's wrapper tags, or a nested container's, would come apart in the splice + if (atContainer || (!direct && (ranges.length > 1 || nestedContainers > 0 || range.foreign || foreign.math > 0))) { unsupported = true return } @@ -300,21 +338,21 @@ export const planRangeWrapper = (ctx: HtmlStageContext, decisions: SurfaceDecisi for (const [content, options] of replacement.slice().reverse()) { element.after(content, options) } - state.nested = true + range.nested = true nested++ } - state.phase = 'inStart' + range.phase = 'inStart' if (!element.onEndTag || isVoidElement(element)) { - state.phase = 'open' + range.phase = 'open' return } try { onEnd(element, () => { - state.phase = 'open' + range.phase = 'open' }) } catch { // Self-closing foreign element, so nothing is inside it - state.phase = 'open' + range.phase = 'open' } }, }, @@ -330,6 +368,10 @@ export const planRangeWrapper = (ctx: HtmlStageContext, decisions: SurfaceDecisi element.removeAndKeepContent() return } + if (element.hasAttribute('data-more')) { + element.remove() + return + } for (const [content, options] of replacement) { element.before(content, options) } diff --git a/test/fakes/LolHtmlRewriter.ts b/test/fakes/LolHtmlRewriter.ts index 1b2c07b5..4b9a8615 100644 --- a/test/fakes/LolHtmlRewriter.ts +++ b/test/fakes/LolHtmlRewriter.ts @@ -134,8 +134,8 @@ const adaptElement = (element: WasmElement): RewriterElement => ({ element.removeAndKeepContent() }, onEndTag: (callback) => { - element.onEndTag(async () => { - await callback() + element.onEndTag(async (endTag) => { + await callback({ name: endTag.name }) }) }, }) diff --git a/test/stages/proxyTruncation.spec.ts b/test/stages/proxyTruncation.spec.ts index 0ef2d9ae..29cce8de 100644 --- a/test/stages/proxyTruncation.spec.ts +++ b/test/stages/proxyTruncation.spec.ts @@ -442,7 +442,7 @@ describe('range replacement on proxy pages', () => {

REPLACEMENT

SubTitle

Second text

REPLACEMENT

SubTitle

REPLACEMENT

SubTitle

REPLACEMENT

SubTitle

Second text

REPLACEMENT

SubTitle

text

-

REPLACEMENT

SubTitle

Second text

+

REPLACEMENT

REPLACEMENT

SubTitle

Second text

PREPENDFirst text

SubTitle

REPLACEMENTAPPEND

AFTER APPEND ` diff --git a/test/stages/surfaceComponents.spec.ts b/test/stages/surfaceComponents.spec.ts index 2e5e1f6d..c8785787 100644 --- a/test/stages/surfaceComponents.spec.ts +++ b/test/stages/surfaceComponents.spec.ts @@ -360,19 +360,19 @@ describe('handleSurfaceComponents', () => { expect(body).toBe('

a

NEW
') }) - it('applies the range to the outermost of nested containers', async () => { + it('cuts a nested container by its own range and the outer one around it', async () => { const body = await replaceRange( '

a

b

c

d

e

f

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

a

NEW

e

NEW
', + '

a

NEW

c

NEW

e

NEW
', ) }) - it('replaces the whole outermost container when nested containers have no markers', async () => { + it('keeps a nested container in the cut, cut by its own range', async () => { const body = await replaceRange('

a

b

c

', {}) - expect(body).toBe('
NEW
') + expect(body).toBe('
NEW
NEW
') }) it('still cuts a later container after one removed by another component', async () => { @@ -394,13 +394,15 @@ describe('handleSurfaceComponents', () => { expect(body).toBe('

a

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

a

b

c

d

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

a

NEW
') + expect(body).toBe( + '

a

NEW

c

NEW
', + ) }) }) diff --git a/test/stages/wrappedRange.spec.ts b/test/stages/wrappedRange.spec.ts index 83460ec9..4c374062 100644 --- a/test/stages/wrappedRange.spec.ts +++ b/test/stages/wrappedRange.spec.ts @@ -333,23 +333,92 @@ describe('range replacement by wrapper', () => { }) describe('nested containers', () => { - it('cut only the outermost', async () => { + it('cut each by its own markers', async () => { const body = '

a

b

c

d

' const { body: out, path } = await bothWays(body, '.c', { fromMarker: 'p.s' }) expect(out).toBe('

a

b

c

NEW
') expect(path).toBe('wrapper') }) - it('cut only the outermost, holding a start marker in the math', async () => { + it('cut around one in the math', async () => { const body = '
x

b

' const { body: out, path } = await bothWays(body, '.c', { fromMarker: '.s' }) expect(out).toBe('
xNEW
') + expect(path).toBe('fallback') + }) + + it('cut element by element when an inner one is left without its end tag', async () => { + for (const [body, range, expected] of [ + [ + '
  • x
  • y
', + {}, + '
    NEW
  • NEW
    NEW
', + ], + [ + '

x

after

', + { fromMarker: 'p.s' }, + '

x

after

', + ], + [ + '

x

a

b
', + { fromMarker: 'p.s' }, + '

x

a

NEW
', + ], + [ + '
  • x
  • y
  • z
', + { fromMarker: '.s' }, + '
  • x
  • y
    • NEW
', + ], + ] as const) { + const { body: out, path } = await bothWays(body, '.c', range) + expect(out).toBe(expected) + expect(path).toBe('fallback') + } + }) + + it('cut around a direct child in the cut, which keeps its own replacement', async () => { + const { body, path } = await bothWays('
x
y
z
w
', '.c', { toMarker: 'hr' }) + expect(body).toBe('
NEW
NEW
z
') + expect(path).toBe('wrapper') + }) + + it('cut by the markers directly in the nested one', async () => { + const { body, path } = await bothWays('

a

c

', '.c', { + toMarker: 'hr.e', + }) + expect(body).toBe('
NEW
NEW

c

') + expect(path).toBe('wrapper') + }) + + it('cut a nested one holding the start marker, and leave the outer one', async () => { + const body = '

a

b

c

d

' + const { body: out, path } = await bothWays(body, '.c', { fromMarker: 'p.s' }) + expect(out).toBe('

a

b

NEW

d

') + expect(path).toBe('wrapper') + }) + + it('cut a nested one deeper in what the outer one keeps', async () => { + const body = + '

y

s

z

a

b

' + const { body: out, path } = await bothWays(body, '.c', { fromMarker: 'p.s' }) + expect(out).toBe( + '

y

s

NEW

a

NEW
', + ) expect(path).toBe('wrapper') }) - it('still cut a later container after an inner one left without its end tag', async () => { - const body = '
  • x
  • y
' - expect(await wrapped(body, '.c', {})).toBe('
    NEW
    NEW
') + it('end the outer cut at a nested one that is its end marker', async () => { + const { body } = await bothWays('

a

b
c
d
', 'div', { + fromMarker: '.s', + toMarker: '.e', + }) + expect(body).toBe('

a

NEW
c
d
') + }) + + it('keep what holds a nested one deeper in the cut, element by element', async () => { + const { body, path } = await bothWays('
x
s
y
t
w
', '.c', {}) + expect(body).toBe('
NEW
NEW
') + expect(path).toBe('fallback') }) }) @@ -429,10 +498,6 @@ describe('range replacement by wrapper', () => { }) }) - it('when the end marker is a direct child of a nested container', async () => { - await expectsFallback('

a

c

', '.c', { toMarker: 'hr.e' }) - }) - it('when the start marker is nested and the decision has another range', async () => { const others = { second: { selector: 'footer', content: { replaceRange: {} } } } await expectsFallback(nested, '.c', { fromMarker: 'p.s' }, { others }) @@ -510,12 +575,6 @@ describe('range replacement by wrapper', () => { ) }) - it('cuts in a nested container holding the start marker', async () => { - const body = '

a

b

c

d

' - const result = await spliced(body, '.c', { fromMarker: 'p.s' }) - expect(result.body).toBe('

a

b

NEW
') - }) - it('closes an ancestor whose end tag the page left out', async () => { const body = '

a x y

z

after

' const { body: out, path } = await run(body, '.c', { fromMarker: '.s' }) @@ -588,7 +647,7 @@ describe('range replacement by wrapper', () => { // In one of its own kind, the parent's end tag would close it and leave the parent open if (i === n - 1 && depth > 0 && open !== parent && rnd() < 0.1) { unclosed = true - out += `<${open}${rnd() < 0.4 ? ` class="${pick(['e', 'x'])}"` : ''}>${tree(depth - 1, open)}` + out += `<${open}${rnd() < 0.4 ? ` class="${pick(['e', 'x', 'c'])}"` : ''}>${tree(depth - 1, open)}` } else if (r < 0.2) out += pick(['text ', 'a b ', '& ']) else if (r < 0.35) out += `<${pick(['img', 'br', 'hr', 'input'])}${rnd() < 0.4 ? ` class="${pick(['s', 'e', 'x'])}"` : ''}>` else if (r < 0.4) out += `` From caf383884179c3f7af7a9ba7513cb7db7bb2a8ff Mon Sep 17 00:00:00 2001 From: Ben Kennedy Date: Fri, 6 Nov 2026 22:55:56 +0000 Subject: [PATCH 4/4] MD-3032 Run the Cloudflare proxy's HTML transform cases --- test/stages/proxyHtmlTransform.spec.ts | 174 +++++++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 test/stages/proxyHtmlTransform.spec.ts diff --git a/test/stages/proxyHtmlTransform.spec.ts b/test/stages/proxyHtmlTransform.spec.ts new file mode 100644 index 00000000..c65d0aca --- /dev/null +++ b/test/stages/proxyHtmlTransform.spec.ts @@ -0,0 +1,174 @@ +import { describe, expect, it } from 'vitest' +import { normalizeMOSConfig } from '../../src/config' +import type { PipelineContext } from '../../src/context' +import handleSurfaceComponents from '../../src/stages/surfaceComponents' +import type { SurfaceDecisionResponse, WebContentSurfaceBehavior } from '../../src/types' +import { LolHtmlRewriter } from '../fakes/LolHtmlRewriter' + +// Ported from the Cloudflare proxy's HTML transform tests, so a release keeps the proxies' suites passing + +const componentsTag = `` + +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', + injectScriptUrl: 'https://example.com/web-components-latest.js', + }), + logger: { log() {} }, +} + +const decisions = (componentBehaviors: SurfaceDecisionResponse['componentBehaviors']): SurfaceDecisionResponse => ({ + status: 'success', + identity: { identifier: 'id', isAuthenticated: false, authType: 'anonymous', jwtClaims: {} }, + features: {}, + customer: { hasProducts: false }, + surfaceBehavior: {}, + componentsSkipped: false, + componentBehaviors, +}) + +describe('component content on proxy pages', () => { + it.each([ + { + name: 'before', + content: { before: [{ type: 'html', content: 'BEFORE' } as const] }, + expected: `${componentsTag}BEFORE

Test

`, + }, + { + name: 'before multiple', + content: { + before: [ + { type: 'html', content: '

1

' } as const, + { type: 'html', content: '

2

' } as const, + { type: 'html', content: '

3

' } as const, + ], + }, + expected: `${componentsTag}

1

2

3

Test

`, + }, + { + name: 'after', + content: { after: [{ type: 'html', content: 'AFTER' } as const] }, + expected: `${componentsTag}

Test

AFTER`, + }, + { + name: 'after multiple', + content: { + after: [ + { type: 'html', content: '

1

' } as const, + { type: 'html', content: '

2

' } as const, + { type: 'html', content: '

3

' } as const, + ], + }, + expected: `${componentsTag}

Test

1

2

3

`, + }, + { + name: 'prepend', + content: { prepend: [{ type: 'html', content: '

PREPEND

' } as const] }, + expected: `${componentsTag}

PREPEND

Test

`, + }, + { + name: 'prepend multiple', + content: { + prepend: [ + { type: 'html', content: '

1

' } as const, + { type: 'html', content: '

2

' } as const, + { type: 'html', content: '

3

' } as const, + ], + }, + expected: `${componentsTag}

1

2

3

Test

`, + }, + { + name: 'append', + content: { append: [{ type: 'html', content: '

APPEND

' } as const] }, + expected: `${componentsTag}

Test

APPEND

`, + }, + { + name: 'append multiple', + content: { + append: [ + { type: 'html', content: '

1

' } as const, + { type: 'html', content: '

2

' } as const, + { type: 'html', content: '

3

' } as const, + ], + }, + expected: `${componentsTag}

Test

1

2

3

`, + }, + { + name: 'remove', + content: { remove: true }, + expected: `${componentsTag}`, + }, + { + name: 'before + after + remove', + content: { + before: [{ type: 'html', content: 'BEFORE' } as const], + after: [{ type: 'text', content: 'AFTER' } as const], + remove: true, + }, + expected: `${componentsTag}BEFOREAFTER`, + }, + { + name: 'append + after', + content: { + after: [{ type: 'html', content: '

AFTER

' } as const], + append: [{ type: 'html', content: '

APPEND

' } as const], + }, + expected: `${componentsTag}

Test

APPEND

AFTER

`, + }, + { + name: 'append + prepend + remove -> removes element and ignores append', + content: { + remove: true, + append: [{ type: 'html', content: '

APPEND

' } as const], + prepend: [{ type: 'html', content: '

PREPEND

' } as const], + }, + expected: `${componentsTag}`, + }, + { + name: 'ignore custom', + content: { + before: [{ type: 'custom', content: 'UNKNOWN' } as const], + }, + expected: `${componentsTag}

Test

`, + }, + { + name: 'MOS element', + content: { + before: [ + { + type: 'element', + schema: 'mos:test@1.0', + props: { + prop1: 'value1', + prop2: true, + }, + } as const, + ], + }, + expected: `${componentsTag}

Test

`, + }, + { + name: ':last-child selector is ignored', + content: { before: [{ type: 'html', content: 'BEFORE' } as const] }, + expected: `

Test

`, + cssSelector: 'h1:last-child', + }, + { + name: 'junk CSS selector is ignored', + content: { before: [{ type: 'html', content: 'BEFORE' } as const] }, + expected: `

Test

`, + cssSelector: '&&&invalid###', + }, + ])('rewrites HTML component content - $name', async ({ content, expected, ...rest }) => { + const cssSelector = 'cssSelector' in rest ? rest.cssSelector : 'h1' + const page = new Response('

Test

', { headers: { 'Content-Type': 'text/html; charset=utf-8' } }) + const decision = decisions({ test: { metadata: { cssSelector }, content: content as WebContentSurfaceBehavior } }) + const result = await handleSurfaceComponents(ctx, page, decision, new LolHtmlRewriter()) + expect(await result.text()).toBe(expected) + }) +})