From 177eda93d095da27137a95b7535c2a687561859c Mon Sep 17 00:00:00 2001
From: Josh Knapp <jknapp85@gmail.com>
Date: Sun, 12 Jul 2026 14:35:27 -0700
Subject: [PATCH] script nits: slider hover/visibility pause + silent autoplay
 + iframe amp guard

ContentSlider: autoplay's setInterval now pauses on mouseenter and on
document visibilitychange (tab hidden), resumes on mouseleave/visible,
and is always clearable (timer var + clearInterval). The aria-live
region is "off" while autoplay is silently auto-rotating and only
flips to "polite" on manual next/prev/dot navigation, so screen readers
aren't spammed with an announcement every `interval` ms (F-export
review Minor). Updates the one existing test that hard-coded
aria-live="polite" as always-on to match this intentional behavior
change.

(Countdown's ticking-interval-stops-at-zero nit shipped in the previous
commit alongside its node-id migration, since both touched the same
inline script.)

Adds regression tests locking in that MapEmbed/VideoBlock iframe src
attributes (built by string concatenation with literal `&` query
params) are HTML-entity-encoded via the existing escapeAttr(safeUrl())
pipeline -- verified already correct, no source change needed there.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 .../components/media/MapEmbed.toHtml.test.ts  | 13 ++++
 .../media/VideoBlock.toHtml.test.ts           | 15 +++++
 .../sections/ContentSlider.toHtml.test.ts     | 61 ++++++++++++++++++-
 .../src/components/sections/ContentSlider.tsx | 46 +++++++++++---
 4 files changed, 124 insertions(+), 11 deletions(-)

diff --git a/craft/src/components/media/MapEmbed.toHtml.test.ts b/craft/src/components/media/MapEmbed.toHtml.test.ts
index f5b3814..4143836 100644
--- a/craft/src/components/media/MapEmbed.toHtml.test.ts
+++ b/craft/src/components/media/MapEmbed.toHtml.test.ts
@@ -14,3 +14,16 @@ describe('MapEmbed.toHtml iframe accessibility (F2.4)', () => {
     expect(html).toContain('title="Map of Golden Gate Bridge"');
   });
 });
+
+describe('MapEmbed.toHtml iframe src ampersand encoding (F-export review Minor)', () => {
+  test('the iframe src (built by string concatenation with literal &) emits &amp; in the attribute, not a raw &', () => {
+    const { html } = toHtml({ address: 'New York, NY', zoom: 14 }, '');
+    const srcMatch = html.match(/<iframe src="([^"]+)"/);
+    expect(srcMatch).toBeTruthy();
+    // The raw src is `...q=...&z=14&output=embed` -- concatenated with
+    // literal `&`s -- so the emitted attribute must HTML-encode them.
+    expect(srcMatch![1]).toContain('&amp;z=14');
+    expect(srcMatch![1]).toContain('&amp;output=embed');
+    expect(srcMatch![1]).not.toMatch(/&(?!amp;)/);
+  });
+});
diff --git a/craft/src/components/media/VideoBlock.toHtml.test.ts b/craft/src/components/media/VideoBlock.toHtml.test.ts
index f328554..573714c 100644
--- a/craft/src/components/media/VideoBlock.toHtml.test.ts
+++ b/craft/src/components/media/VideoBlock.toHtml.test.ts
@@ -71,3 +71,18 @@ describe('VideoBlock.toHtml iframe accessibility (F2.4)', () => {
     expect(html).toMatch(/<iframe[^>]*title="[^"]+"/);
   });
 });
+
+describe('VideoBlock.toHtml iframe src ampersand encoding (F-export review Minor)', () => {
+  test('embed params joined with literal & are HTML-entity-encoded in the emitted src attribute', () => {
+    // autoplay+muted+controls=false forces buildEmbedParams to concatenate
+    // multiple query params onto the URL with literal `&`s.
+    const { html } = toHtml(
+      { videoUrl: 'https://www.youtube.com/watch?v=dQw4w9WgXcQ', autoplay: true, muted: true, controls: false },
+      ''
+    );
+    const srcMatch = html.match(/<iframe src="([^"]+)"/);
+    expect(srcMatch).toBeTruthy();
+    expect(srcMatch![1]).toMatch(/&amp;/);
+    expect(srcMatch![1]).not.toMatch(/&(?!amp;)/);
+  });
+});
diff --git a/craft/src/components/sections/ContentSlider.toHtml.test.ts b/craft/src/components/sections/ContentSlider.toHtml.test.ts
index 39857f7..c66491b 100644
--- a/craft/src/components/sections/ContentSlider.toHtml.test.ts
+++ b/craft/src/components/sections/ContentSlider.toHtml.test.ts
@@ -23,9 +23,9 @@ describe('ContentSlider.toHtml accessibility (F1.1)', () => {
     expect(html).toMatch(/<button[^>]*aria-label="Go to slide 3"/);
   });
 
-  test('slides are wrapped in an aria-live="polite" region', () => {
+  test('slides are wrapped in an aria-live region', () => {
     const { html } = toHtml({ slides }, '');
-    expect(html).toContain('aria-live="polite"');
+    expect(html).toMatch(/aria-live="(polite|off)"/);
   });
 
   test('decorative chevron icons in arrows are aria-hidden', () => {
@@ -34,3 +34,60 @@ describe('ContentSlider.toHtml accessibility (F1.1)', () => {
     expect(html).toMatch(/<i class="fa fa-chevron-right" aria-hidden="true"><\/i>/);
   });
 });
+
+describe('ContentSlider.toHtml deterministic + unique scope ids (thread node id)', () => {
+  test('same node id -> identical output across calls (deterministic, no Math.random)', () => {
+    const { html: html1 } = toHtml({ slides }, '', 'node-cs1');
+    const { html: html2 } = toHtml({ slides }, '', 'node-cs1');
+    expect(html1).toBe(html2);
+  });
+
+  test('different node ids -> different, non-colliding scope ids (identical slides, no collision)', () => {
+    const { html: html1 } = toHtml({ slides }, '', 'node-cs1');
+    const { html: html2 } = toHtml({ slides }, '', 'node-cs2');
+    const id1 = html1.match(/<section id="([^"]+)"/)![1];
+    const id2 = html2.match(/<section id="([^"]+)"/)![1];
+    expect(id1).not.toBe(id2);
+  });
+
+  test('no nodeId (legacy 2-arg call): still deterministic across repeated calls, not random', () => {
+    const { html: html1 } = toHtml({ slides }, '');
+    const { html: html2 } = toHtml({ slides }, '');
+    expect(html1).toBe(html2);
+  });
+});
+
+describe('ContentSlider.toHtml autoplay silences aria-live and is pausable (F-export review Minor)', () => {
+  test('aria-live is "off" while autoplay is running, to avoid announcing every auto-rotation', () => {
+    const { html } = toHtml({ slides, autoplay: true }, '');
+    expect(html).toContain('aria-live="off"');
+  });
+
+  test('aria-live stays "polite" when autoplay is disabled', () => {
+    const { html } = toHtml({ slides, autoplay: false }, '');
+    expect(html).toContain('aria-live="polite"');
+  });
+
+  test('manual navigation (next/prev/dot) marks the live region "polite"', () => {
+    const { html } = toHtml({ slides, autoplay: true }, '');
+    expect(html).toMatch(/setAttribute\(["']aria-live["'],\s*["']polite["']\)/);
+  });
+
+  test('autoplay pauses on hover and resumes on mouse leave', () => {
+    const { html } = toHtml({ slides, autoplay: true }, '');
+    expect(html).toMatch(/addEventListener\(["']mouseenter["']/);
+    expect(html).toMatch(/addEventListener\(["']mouseleave["']/);
+  });
+
+  test('autoplay pauses when the tab is hidden (visibilitychange) and the interval is clearable', () => {
+    const { html } = toHtml({ slides, autoplay: true }, '');
+    expect(html).toMatch(/visibilitychange/);
+    expect(html).toMatch(/clearInterval\(/);
+  });
+
+  test('no autoplay: no setInterval/hover/visibility wiring at all', () => {
+    const { html } = toHtml({ slides, autoplay: false }, '');
+    expect(html).not.toMatch(/setInterval\(/);
+    expect(html).not.toMatch(/visibilitychange/);
+  });
+});
diff --git a/craft/src/components/sections/ContentSlider.tsx b/craft/src/components/sections/ContentSlider.tsx
index 1395ada..7b0b623 100644
--- a/craft/src/components/sections/ContentSlider.tsx
+++ b/craft/src/components/sections/ContentSlider.tsx
@@ -1,7 +1,7 @@
 import React, { CSSProperties, useState, useEffect, useRef, useCallback } from 'react';
 import { useNode, UserComponent } from '@craftjs/core';
 import { cssPropsToString } from '../../utils/style-helpers';
-import { escapeHtml, escapeAttr, safeUrl } from '../../utils/escape';
+import { escapeHtml, escapeAttr, safeUrl, scopeId } from '../../utils/escape';
 
 interface Slide {
   type: 'image' | 'content';
@@ -238,7 +238,7 @@ ContentSlider.craft = {
 
 /* ---------- HTML export ---------- */
 
-(ContentSlider as any).toHtml = (props: ContentSliderProps, _childrenHtml: string) => {
+(ContentSlider as any).toHtml = (props: ContentSliderProps, _childrenHtml: string, nodeId?: string) => {
   const {
     slides = defaultSlides,
     autoplay = true,
@@ -250,7 +250,11 @@ ContentSlider.craft = {
   } = props;
 
   const items = slides.length > 0 ? slides : defaultSlides;
-  const uid = 'cs_' + Math.random().toString(36).slice(2, 8);
+  // Deterministic AND unique id, scoped on the Craft node id, for this
+  // slider's slide/dot element ids and inline-script globals -- so two
+  // ContentSlider instances (e.g. both left at default slides) don't
+  // collide and end up driving each other's rotation.
+  const uid = scopeId(nodeId, JSON.stringify(items) + interval, 'cs');
 
   const sectionStyle = cssPropsToString({
     position: 'relative',
@@ -299,9 +303,18 @@ ContentSlider.craft = {
   </div>`
     : '';
 
+  // Autoplay ticks call show() directly (internal), while manual nav goes
+  // through the exposed window[...] functions -- that split lets us mark
+  // the live region "polite" only on manual navigation, and keep it "off"
+  // while autoplay is silently auto-rotating, so screen readers aren't
+  // spammed with an announcement every `interval` ms (F-export review
+  // Minor).
+  const autoplayActive = autoplay && items.length > 1;
+  const liveAttr = autoplayActive ? 'off' : 'polite';
+
   return {
-    html: `<section${sectionStyle ? ` style="${sectionStyle}"` : ''}>
-  <div aria-live="polite">
+    html: `<section id="${uid}"${sectionStyle ? ` style="${sectionStyle}"` : ''}>
+  <div id="${uid}_live" aria-live="${liveAttr}">
   ${slidesHtml}
   </div>
   ${arrowsHtml}
@@ -309,6 +322,8 @@ ContentSlider.craft = {
   <script>
   (function(){
     var current=0, total=${items.length}, uid="${uid}";
+    var liveRegion=document.getElementById(uid+"_live");
+    function markManualNav(){ if(liveRegion){ liveRegion.setAttribute("aria-live","polite"); } }
     function show(idx){
       document.getElementById(uid+"_s"+current).style.opacity="0";
       ${showDots ? `document.getElementById(uid+"_d"+current).style.backgroundColor="rgba(255,255,255,0.5)";` : ''}
@@ -316,10 +331,23 @@ ContentSlider.craft = {
       document.getElementById(uid+"_s"+current).style.opacity="1";
       ${showDots ? `document.getElementById(uid+"_d"+current).style.backgroundColor="#ffffff";` : ''}
     }
-    window["${uid}_go"]=show;
-    window["${uid}_next"]=function(){show(current+1);};
-    window["${uid}_prev"]=function(){show(current-1);};
-    ${autoplay && items.length > 1 ? `setInterval(function(){show(current+1);},${interval});` : ''}
+    window["${uid}_go"]=function(idx){ markManualNav(); show(idx); };
+    window["${uid}_next"]=function(){ markManualNav(); show(current+1); };
+    window["${uid}_prev"]=function(){ markManualNav(); show(current-1); };
+    ${autoplayActive ? `
+    var timer=null;
+    function start(){ if(!timer && document.visibilityState!=="hidden"){ timer=setInterval(function(){show(current+1);},${interval}); } }
+    function stop(){ if(timer){ clearInterval(timer); timer=null; } }
+    var root=document.getElementById(uid);
+    if(root){
+      root.addEventListener("mouseenter", stop);
+      root.addEventListener("mouseleave", start);
+    }
+    document.addEventListener("visibilitychange", function(){
+      if(document.visibilityState==="hidden"){ stop(); } else { start(); }
+    });
+    start();
+    ` : ''}
   })();
   </script>
 </section>`,