Merge pull request #185 from ator-dev/paint-methods-and-flow-tracker-…

…enhance

Enhance Paint, fire FlowTracker listeners less

src/modules/highlight/engines/paint.mts

@@ -74,9 +74,12 @@ class PaintEngine implements AbstractTreeCacheEngine, HighlightingStyleObserver
 		this.#termTokens = termTokens;
 		this.#flowTracker = new FlowTracker(this.terms, termPatterns);
 		this.#flowTracker.setNewSpanOwnerListener(flowOwner => {
+			if (this.#method.highlightables) {
+				flowOwner = this.#method.highlightables.findHighlightableAncestor(flowOwner);
+			}
 			this.observeVisibilityChangesFor(flowOwner);
 			if (!this.#elementHighlightingIdMap.has(flowOwner)) {
-				const id = highlightingId.next().value;
+				const id = highlightingIds.next().value;
 				this.#elementHighlightingIdMap.set(flowOwner, id);
 				// NOTE: Some webpages may remove unknown attributes. It is possible to check and re-apply it from cache.
 				// TODO make sure there is cleanup once the highlighting ID becomes invalid (e.g. when the cache is removed).
@@ -89,6 +92,9 @@ class PaintEngine implements AbstractTreeCacheEngine, HighlightingStyleObserver
 			}
 		});
 		this.#flowTracker.setNonSpanOwnerListener(flowOwner => {
+			if (this.#method.highlightables) {
+				flowOwner = this.#method.highlightables.findHighlightableAncestor(flowOwner);
+			}
 			// TODO this is done for consistency with the past behaviour; but is it right/necessary?
 			this.#elementHighlightingIdMap.delete(flowOwner);
 			this.#elementStyleRuleMap.delete(flowOwner);
@@ -161,7 +167,7 @@ class PaintEngine implements AbstractTreeCacheEngine, HighlightingStyleObserver
 				visibilityObserver.disconnect();
 			};
 		}
-		const highlightingId = (function* () {
+		const highlightingIds = (function* () {
 			let i = 0;
 			while (true) {
 				yield i++;

src/modules/highlight/engines/paint/methods/element.mts

@@ -77,12 +77,13 @@ class ElementMethod implements AbstractMethod {
 				this.#elementDrawContainerMap.delete(element);
 			}
 			for (const element of newlyStyledElements) {
-				this.#elementDrawContainerMap.get(element)?.remove();
-				this.#elementDrawContainerMap.delete(element);
 				const container = this.getDrawElementContainer(element);
+				this.#elementDrawContainerMap.get(element)?.remove();
 				if (container) {
 					this.#elementDrawContainerMap.set(element, container);
 					this.#drawContainersParent.appendChild(container);
+				} else {
+					this.#elementDrawContainerMap.delete(element);
 				}
 			}
 		});

src/modules/highlight/engines/paint/methods/paint.mts

@@ -12,7 +12,7 @@ import { StyleManager } from "/dist/modules/style-manager.mjs";
 import { HTMLStylesheet } from "/dist/modules/stylesheets/html.mjs";
 import { EleID, EleClass } from "/dist/modules/common.mjs";
 
-type TermSelectorStyles = Record<string, {
+type TermTokenStyles = Record<string, {
 	hue: number
 	cycle: number
 }>
@@ -49,7 +49,7 @@ class PaintMethod implements AbstractMethod {
 	endHighlighting () {}
 
 	getTermsCSS (terms: ReadonlyArray<MatchTerm>, hues: ReadonlyArray<number>) {
-		const styles: TermSelectorStyles = {};
+		const styles: TermTokenStyles = {};
 		for (let i = 0; i < terms.length; i++) {
 			styles[this.#termTokens.get(terms[i])] = {
 				hue: hues[i % hues.length],
@@ -69,7 +69,8 @@ class PaintMethod implements AbstractMethod {
 		--markmysearch-styles: unset;
 		--markmysearch-boxes: unset;
 	}
-}`
+}
+`
 		);
 	}
 
@@ -100,4 +101,7 @@ class PaintMethod implements AbstractMethod {
 	}
 }
 
-export { type TermSelectorStyles, PaintMethod };
+export {
+	type TermTokenStyles,
+	PaintMethod,
+};

src/modules/highlight/models/tree-cache/flow-tracker.mts

@@ -167,10 +167,12 @@ class FlowTracker implements AbstractFlowTracker {
 			const flows = this.#elementFlowsMap.get(element);
 			if (flows) {
 				this.#elementFlowsMap.delete(element);
-				if (this.#spansRemovedListener)
+				if (this.#spansRemovedListener) {
 					this.#spansRemovedListener(element, flows.flatMap(flow => flow.spans));
-				if (this.#nonSpanOwnerListener)
+				}
+				if (this.#nonSpanOwnerListener) {
 					this.#nonSpanOwnerListener(element);
+				}
 			}
 		// eslint-disable-next-line no-cond-assign
 		} while (element = walker.nextNode());
@@ -196,25 +198,28 @@ class FlowTracker implements AbstractFlowTracker {
 				});
 				if (flowsNew.length > 0) {
 					this.#elementFlowsMap.set(element, flowsNew);
-					if (spansRemoved.length > 0) {
-						if (this.#spansRemovedListener)
-							this.#spansRemovedListener(element, spansRemoved);
+					if (this.#spansRemovedListener && spansRemoved.length > 0) {
+						this.#spansRemovedListener(element, spansRemoved);
 					}
 				} else {
 					this.#elementFlowsMap.delete(element);
-					if (this.#spansRemovedListener)
+					if (this.#spansRemovedListener) {
 						this.#spansRemovedListener(element, spansRemoved);
-					if (this.#nonSpanOwnerListener)
+					}
+					if (this.#nonSpanOwnerListener) {
 						this.#nonSpanOwnerListener(element);
+					}
 				}
 			}
 		} else {
 			if (this.#spansRemovedListener || this.#nonSpanOwnerListener) {
 				for (const [ element, flows ] of this.#elementFlowsMap) {
-					if (this.#spansRemovedListener)
+					if (this.#spansRemovedListener) {
 						this.#spansRemovedListener(element, flows.flatMap(flow => flow.spans));
-					if (this.#nonSpanOwnerListener)
+					}
+					if (this.#nonSpanOwnerListener) {
 						this.#nonSpanOwnerListener(element);
+					}
 				}
 			}
 			this.#elementFlowsMap.clear();
@@ -284,12 +289,21 @@ class FlowTracker implements AbstractFlowTracker {
 			this.#elementFlowsMap.set(ancestor, flows);
 		}
 		flows.push({ text, spans: spansCreated });
-		if (flows.length === 1) {
-			if (this.#newSpanOwnerListener)
+		if (this.#newSpanOwnerListener) {
+			if (
+				// If: the new flow contains spans
+				spansCreated.length > 0
+				// AND the total number of spans equals the number of spans in the new flow
+				&& (flows.reduce((previous, current) => previous + current.spans.length, 0)
+					=== spansCreated.length)
+				// So: the element just became a span owner
+			) {
 				this.#newSpanOwnerListener(ancestor);
+			}
 		}
-		if (this.#spansCreatedListener)
+		if (this.#spansCreatedListener && spansCreated.length > 0) {
 			this.#spansCreatedListener(ancestor, spansCreated);
+		}
 	}
 
 	getAncestorCommon (nodeA_ancestor: HTMLElement, nodeB: Node): HTMLElement | null {

src/paint.ts

@@ -5,7 +5,7 @@
  */
 
 import type { Box } from "/dist/modules/highlight/engines/paint.mjs";
-import type { TermSelectorStyles } from "/dist/modules/highlight/engines/paint/methods/paint.mjs";
+import type { TermTokenStyles } from "/dist/modules/highlight/engines/paint/methods/paint.mjs";
 
 registerPaint("markmysearch-highlights", class {
 	static get inputProperties () {
@@ -16,27 +16,38 @@ registerPaint("markmysearch-highlights", class {
 	}
 
 	paint (
-		ctx: PaintRenderingContext2D,
-		geom: PaintSize,
+		context: PaintRenderingContext2D,
+		size: PaintSize,
 		properties: StylePropertyMapReadOnly,
 	) {
-		const selectorStyles = JSON.parse(properties.get("--markmysearch-styles")?.toString() ?? "{}") as TermSelectorStyles;
-		const boxes = JSON.parse(properties.get("--markmysearch-boxes")?.toString() ?? "[]") as Array<Box>;
+		// Using `<string | undefined> || <string>` instead of `<string | undefined> ?? <string>`
+		// means that the empty string (a falsy value) will also shortcut to the right-hand-side.
+		// This is important because properties.get() could return a CSSUnparsedValue which
+		// evaluates to the empty string, which is not valid JSON. This happens regularly during
+		// normal operation, and after investigation seems to be a quirk of CSS.
+		const termTokenStyles = JSON.parse(
+			// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
+			properties.get("--markmysearch-styles")?.toString() || "{}"
+		) as TermTokenStyles;
+		const boxes = JSON.parse(
+			// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
+			properties.get("--markmysearch-boxes")?.toString() || "[]"
+		) as Array<Box>;
 		for (const box of boxes) {
-			const style = selectorStyles[box.token];
+			const style = termTokenStyles[box.token];
 			if (!style) {
-				return;
+				continue;
 			}
-			ctx.strokeStyle = `hsl(${style.hue} 100% 10% / 0.4)`;
-			ctx.strokeRect(box.x, box.y, box.width, box.height);
+			context.strokeStyle = `hsl(${style.hue} 100% 10% / 0.4)`;
+			context.strokeRect(box.x, box.y, box.width, box.height);
 			const height = box.height / Math.floor((style.cycle + 3) / 2);
 			if (style.cycle === 0) {
 				// Special case: 1st cycle (only one with a single block of color) must begin with the lightness closer to 50%.
 				style.cycle = -1;
 			}
 			for (let i = 0; i <= (style.cycle + 1) / 2; i++) {
-				ctx.fillStyle = `hsl(${style.hue} 100% ${(i % 2 == style.cycle % 2) ? 98 : 60}% / 0.4)`;
-				ctx.fillRect(box.x, box.y + height*i, box.width, height);
+				context.fillStyle = `hsl(${style.hue} 100% ${(i % 2 == style.cycle % 2) ? 98 : 60}% / 0.4)`;
+				context.fillRect(box.x, box.y + height*i, box.width, height);
 			}
 		}
 	}