MD-3032 Fix replaceRange when range components share elements, and cut nested containers by their own ranges

.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.

.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.

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>): void
+    onEndTag?(callback: (endTag?: RewriterEndTag) => void | Promise<void>): 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 {

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'

src/index.ts

@@ -15,6 +15,7 @@ export type {
     Resource,
     ResourceProvider,
     RewriterElement,
+    RewriterEndTag,
     RewriterText,
     UnresolvedConfigContext,
     UnresolvedConfigHandler,

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,15 +44,16 @@ 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
     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',
@@ -53,7 +62,7 @@ export const buildReplacementHandlers = (
             })
             return
         }
-        return state
+        return frame.state ?? undefined
     }
 
     const onChildElement = (element: RewriterElement) => {
@@ -63,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') {
@@ -73,15 +82,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'
                 }
             }
@@ -98,53 +103,42 @@ 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)
-            }
 
-            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 (`<path/>`), 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)
                 }
             }
+            const index = open.length
+            // A self-closing foreign container (`<path/>`) has no end tag or content to cut. A container's end also
+            // closes containers left open in it.
+            if (
+                onEnd(element, () => {
+                    open.length = index
+                })
+            ) {
+                open.push({ state, range: ranges[rangeIndex] })
+            }
         },
 
         text: (text: RewriterText) => {
@@ -157,10 +151,22 @@ export const buildReplacementHandlers = (
 
     const selector = subSurface.metadata.cssSelector
     if (!selector) {
-        return []
+        return null
+    }
+    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 [
-        { selector, handler: parentHandler },
-        { selector: combineSelectors(selector, ' ', '*'), handler: { element: onChildElement } },
-    ]
+    return low < islands.length && (islands[low] as number) <= to
 }