Fix sign language tool for draggable videos (BL-16131)

src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts

@@ -81,7 +81,8 @@ import { get, postData, postJson, postString } from "../../utils/bloomApi";
 import AudioRecording from "../toolbox/talkingBook/audioRecording";
 import PlaceholderProvider from "./PlaceholderProvider";
 import { getExactClientSize } from "../../utils/elementUtils";
-import { copyContentToTarget, getTarget } from "bloom-player";
+import { getTarget } from "bloom-player";
+import { copyContentToTargetAndCleanup } from "./dragActivityRuntimeUtils";
 import { showRequiresSubscriptionDialogInEditView } from "../../react_components/requiresSubscription";
 import { FeatureStatus } from "../../react_components/featureStatus";
 import $ from "jquery";
@@ -2905,7 +2906,7 @@ export class CanvasElementManager {
             this.doAfterNewImageAdjusted();
             this.doAfterNewImageAdjusted = undefined;
         }
-        copyContentToTarget(canvasElement);
+        copyContentToTargetAndCleanup(canvasElement);
     }
 
     // When the image is changed in a canvas element (e.g., choose or paste image),

src/BloomBrowserUI/bookEdit/js/bloomEditing.ts

@@ -9,7 +9,10 @@ import {
     SetupResizableElement,
     SetupImagesInContainer,
 } from "./bloomImages";
-import { SetupVideoEditing } from "./bloomVideo";
+import {
+    removeTransientVideoTimestampParams,
+    SetupVideoEditing,
+} from "./bloomVideo";
 import { SetupWidgetEditing } from "./bloomWidgets";
 import { setupOrigami, cleanupOrigami } from "./origami";
 import theOneLocalizationManager from "../../lib/localizationManager/localizationManager";
@@ -1215,6 +1218,7 @@ function removeEditingDebris() {
     for (let i = 0; i < textLabels.length; i++) {
         textLabels[i].remove();
     }
+    removeTransientVideoTimestampParams(document.body);
     cleanupNiceScroll(); // don't leave the nicescroll debris around
 }
 

src/BloomBrowserUI/bookEdit/js/bloomVideo.spec.ts

@@ -0,0 +1,36 @@
+import { describe, expect, it } from "vitest";
+import {
+    removeTransientVideoTimestampParams,
+    stripTransientVideoTimestampParam,
+} from "./bloomVideo";
+
+describe("bloomVideo transient video timestamp cleanup", () => {
+    it("removes only the transient video timestamp param", () => {
+        const cleaned = stripTransientVideoTimestampParam(
+            "video.mp4?persist=true&bloomVideoTransientTimestamp=123#t=1,2",
+        );
+
+        expect(cleaned).toBe("video.mp4?persist=true#t=1,2");
+    });
+
+    it("leaves urls without the transient param unchanged", () => {
+        const cleaned = stripTransientVideoTimestampParam(
+            "video.mp4?persist=true#t=1,2",
+        );
+
+        expect(cleaned).toBe("video.mp4?persist=true#t=1,2");
+    });
+
+    it("cleans transient params from video sources in the page", () => {
+        document.body.innerHTML =
+            '<div><video src="movie.mp4?bloomVideoTransientTimestamp=7"></video><video><source src="clip.mp4?keep=1&bloomVideoTransientTimestamp=8#t=2,4"></source></video></div>';
+
+        removeTransientVideoTimestampParams(document.body);
+
+        const videos = Array.from(document.body.querySelectorAll("video"));
+        expect(videos[0].getAttribute("src")).toBe("movie.mp4");
+        expect(videos[1].querySelector("source")?.getAttribute("src")).toBe(
+            "clip.mp4?keep=1#t=2,4",
+        );
+    });
+});

src/BloomBrowserUI/bookEdit/js/bloomVideo.ts

@@ -18,45 +18,47 @@ import { getReplayIcon } from "../img/replayIcon";
 import { kCanvasElementSelector } from "../toolbox/canvas/canvasElementUtils";
 import $ from "jquery";
 
-export function SetupVideoEditing(container) {
+const kBloomVideoTransientTimestampParam = "bloomVideoTransientTimestamp";
+
+export function SetupVideoEditing(container: HTMLElement) {
     $(container)
         .find(".bloom-videoContainer")
         .each((index, vc) => {
             SetupVideoContainer(vc);
         });
-    Array.from(container.getElementsByTagName("video")).forEach(
-        (videoElement: HTMLVideoElement) => {
-            videoElement.removeAttribute("controls");
-            // I don't think we need to do this in normal operation, but it's useful when
-            // debugging, and just might prevent a problem in normal operation.
-            videoElement.parentElement?.classList.remove("playing");
-            videoElement.parentElement?.classList.remove("paused");
-            videoElement.addEventListener("click", handleVideoClick);
-            const playButton = wrapVideoIcon(
-                videoElement,
-                // Alternatively, we could import the Material UI icon, make this file a TSX, and use
-                // ReactDom.render to render the icon into the div. But just creating the SVG
-                // ourselves (as these methods do) seems more natural to me. We would not be using
-                // React for anything except to make use of an image which unfortunately is only
-                // available by default as a component.
-                getPlayIcon("#ffffff", videoElement),
-                "bloom-videoPlayIcon",
-            );
-            playButton.addEventListener("click", handlePlayClick);
-            const pauseButton = wrapVideoIcon(
-                videoElement,
-                getPauseIcon("#ffffff", videoElement),
-                "bloom-videoPauseIcon",
-            );
-            pauseButton.addEventListener("click", handlePauseClick);
-            const replayButton = wrapVideoIcon(
-                videoElement,
-                getReplayIcon("#ffffff", videoElement),
-                "bloom-videoReplayIcon",
-            );
-            replayButton.addEventListener("click", handleReplayClick);
-        },
-    );
+    Array.from<HTMLVideoElement>(
+        container.getElementsByTagName("video"),
+    ).forEach((videoElement: HTMLVideoElement) => {
+        videoElement.removeAttribute("controls");
+        // I don't think we need to do this in normal operation, but it's useful when
+        // debugging, and just might prevent a problem in normal operation.
+        videoElement.parentElement?.classList.remove("playing");
+        videoElement.parentElement?.classList.remove("paused");
+        videoElement.addEventListener("click", handleVideoClick);
+        const playButton = wrapVideoIcon(
+            videoElement,
+            // Alternatively, we could import the Material UI icon, make this file a TSX, and use
+            // ReactDom.render to render the icon into the div. But just creating the SVG
+            // ourselves (as these methods do) seems more natural to me. We would not be using
+            // React for anything except to make use of an image which unfortunately is only
+            // available by default as a component.
+            getPlayIcon("#ffffff", videoElement),
+            "bloom-videoPlayIcon",
+        );
+        playButton.addEventListener("click", handlePlayClick);
+        const pauseButton = wrapVideoIcon(
+            videoElement,
+            getPauseIcon("#ffffff", videoElement),
+            "bloom-videoPauseIcon",
+        );
+        pauseButton.addEventListener("click", handlePauseClick);
+        const replayButton = wrapVideoIcon(
+            videoElement,
+            getReplayIcon("#ffffff", videoElement),
+            "bloom-videoReplayIcon",
+        );
+        replayButton.addEventListener("click", handleReplayClick);
+    });
 }
 
 function SetupVideoContainer(videoContainerDiv: Element) {
@@ -92,7 +94,7 @@ function videoPlayingEventHandler(e: Event) {
     currentVideoElement = video;
     let end: number = getVideoEndSeconds(video);
     const untrimmedEndPoint: number = 0.0;
-    if (end == untrimmedEndPoint) {
+    if (end === untrimmedEndPoint) {
         // We can't just set the endpoint to equal the duration of the video here, because
         // we will be testing that the current playback time is greater than the endpoint.
         // Since we test for the end of the video every 1/10th of a second, set the endpoint
@@ -292,13 +294,59 @@ export function updateVideoInContainer(container: Element, url: string): void {
             video.appendChild(source);
         }
         if (source) {
-            source.setAttribute("src", url);
+            source.setAttribute("src", addTransientVideoTimestampParam(url));
+            video.load();
             // Transparent background videos allow the placeholder to show.  See BL-13918.
             container.classList.remove("bloom-noVideoSelected");
         }
     }
 }
 
+function addTransientVideoTimestampParam(url: string): string {
+    const hashIndex = url.indexOf("#");
+    const hash = hashIndex >= 0 ? url.substring(hashIndex) : "";
+    const beforeHash = hashIndex >= 0 ? url.substring(0, hashIndex) : url;
+    const queryIndex = beforeHash.indexOf("?");
+    const baseUrl =
+        queryIndex >= 0 ? beforeHash.substring(0, queryIndex) : beforeHash;
+    const query = queryIndex >= 0 ? beforeHash.substring(queryIndex + 1) : "";
+    const searchParams = new URLSearchParams(query);
+    searchParams.set(kBloomVideoTransientTimestampParam, Date.now().toString());
+    const search = searchParams.toString();
+
+    return `${baseUrl}${search ? `?${search}` : ""}${hash}`;
+}
+
+export function stripTransientVideoTimestampParam(url: string): string {
+    const hashIndex = url.indexOf("#");
+    const hash = hashIndex >= 0 ? url.substring(hashIndex) : "";
+    const beforeHash = hashIndex >= 0 ? url.substring(0, hashIndex) : url;
+    const queryIndex = beforeHash.indexOf("?");
+    if (queryIndex < 0) {
+        return url;
+    }
+
+    const baseUrl = beforeHash.substring(0, queryIndex);
+    const query = beforeHash.substring(queryIndex + 1);
+    const searchParams = new URLSearchParams(query);
+    searchParams.delete(kBloomVideoTransientTimestampParam);
+    const search = searchParams.toString();
+
+    return `${baseUrl}${search ? `?${search}` : ""}${hash}`;
+}
+
+export function removeTransientVideoTimestampParams(root: ParentNode): void {
+    for (const element of Array.from(
+        root.querySelectorAll<HTMLElement>("video[src], video source[src]"),
+    )) {
+        const src = element.getAttribute("src");
+        if (!src) {
+            continue;
+        }
+        element.setAttribute("src", stripTransientVideoTimestampParam(src));
+    }
+}
+
 // configure one of the icons we display over videos. We put a div around it and apply
 // various classes and append it to the parent of the video.
 function wrapVideoIcon(

src/BloomBrowserUI/bookEdit/js/dragActivityRuntimeUtils.ts

@@ -0,0 +1,17 @@
+import { copyContentToTarget, getTarget } from "bloom-player";
+
+// Keep UI-only selection state out of target shadow content.
+export function copyContentToTargetAndCleanup(
+    draggableElement: HTMLElement,
+): void {
+    copyContentToTarget(draggableElement);
+
+    const target = getTarget(draggableElement);
+    if (!target) {
+        return;
+    }
+
+    target.querySelectorAll(".bloom-selected").forEach((el) => {
+        el.classList.remove("bloom-selected");
+    });
+}

src/BloomBrowserUI/bookEdit/js/videoUtils.ts

@@ -9,6 +9,39 @@ import {
 
 export const kVideoContainerClass = "bloom-videoContainer";
 
+function resolveSelectableVideoContainer(
+    videoContainer: Element | undefined | null,
+    body: HTMLElement | null,
+): Element | undefined {
+    if (!videoContainer) {
+        return undefined;
+    }
+
+    const containingTarget = videoContainer.closest("[data-target-of]");
+    if (!containingTarget) {
+        return videoContainer;
+    }
+
+    const draggableId = containingTarget.getAttribute("data-target-of");
+    if (!draggableId || !body) {
+        return undefined;
+    }
+
+    // Keep this local to avoid importing CanvasElementManager (which imports this file).
+    // We can have copied target content that still contains data-draggable-id, so make
+    // sure we resolve to a real draggable (not anything inside [data-target-of]).
+    const draggable = Array.from(
+        body.querySelectorAll(`[data-draggable-id='${draggableId}']`),
+    ).find((candidate) => !candidate.closest("[data-target-of]")) as
+        | HTMLElement
+        | undefined;
+    if (!draggable) {
+        return undefined;
+    }
+
+    return draggable.querySelector(`.${kVideoContainerClass}`) ?? undefined;
+}
+
 // Set the attribute which makes a canvas element active for the sign language tool.
 // Make sure nothing else has it.
 // If it's in a canvas element, make that canvas element active. If not, make sure no canvas element is active.
@@ -17,14 +50,19 @@ export function selectVideoContainer(
     videoContainer: Element | undefined | null,
     notifyCanvasElementManager = true,
 ) {
-    const body = getPageIframeBody();
+    const body =
+        getPageIframeBody() ?? videoContainer?.ownerDocument?.body ?? null;
+    const selectableVideoContainer = resolveSelectableVideoContainer(
+        videoContainer,
+        body,
+    );
     if (body) {
-        Array.from(body.getElementsByClassName("bloom-selected"))
-            .filter((e) => e !== videoContainer)
-            .forEach((e) => e.classList.remove("bloom-selected"));
+        Array.from(body.getElementsByClassName("bloom-selected")).forEach((e) =>
+            e.classList.remove("bloom-selected"),
+        );
     }
-    videoContainer?.classList.add("bloom-selected");
-    const canvasElement = videoContainer?.closest(
+    selectableVideoContainer?.classList.add("bloom-selected");
+    const canvasElement = selectableVideoContainer?.closest(
         kCanvasElementSelector,
     ) as HTMLElement;
     // If it's in a canvas element, make that canvas element active. If not, make sure no canvas element is active.
@@ -40,9 +78,9 @@ export function selectVideoContainer(
 // since the yellow box is hidden in canvas elements, and we would like the same one (that is our active
 // canvas element) to be selected in the sign langauge tool if we switch back.)
 export function deselectVideoContainers() {
-    const videoContainers: HTMLElement[] = Array.from(
-        document.getElementsByClassName(kVideoContainerClass) as any,
-    );
+    const videoContainers = Array.from(
+        document.getElementsByClassName(kVideoContainerClass),
+    ) as HTMLElement[];
     videoContainers
         .filter((x) => !x.closest(kCanvasElementSelector))
         .forEach((container) => {

src/BloomBrowserUI/bookEdit/toolbox/ToolboxRoot.tsx

@@ -638,7 +638,12 @@ export const ToolboxRoot: React.FunctionComponent = () => {
         window.toolboxReactAdapter = {
             isEnabled: () => true,
             setActiveToolByToolId: (toolId: string) => {
-                setExpandedSectionId(normalizeToolId(toolId));
+                const normalizedToolId = normalizeToolId(toolId);
+                setExpandedSectionId(normalizedToolId);
+                const callbackToolId = toToolboxToolId(normalizedToolId);
+                activeToolChangedCallbacks.current.forEach((callback) => {
+                    callback(callbackToolId);
+                });
             },
             getActiveToolId: () => {
                 if (!expandedSectionId) {

src/BloomBrowserUI/bookEdit/toolbox/games/GamePromptDialog.tsx

@@ -10,11 +10,11 @@ interface IGamePromptDialogProps {
 import { useRef } from "react";
 import {
     adjustDraggablesForLanguage,
-    copyContentToTarget,
     getTarget,
     shuffle,
     isTheTextInDraggablesTheSame,
 } from "bloom-player";
+import { copyContentToTargetAndCleanup } from "../../js/dragActivityRuntimeUtils";
 import { setGeneratedDraggableId } from "../canvas/CanvasElementItem";
 import { adjustTarget, makeTargetForDraggable } from "../games/GameTool";
 import * as ReactDOM from "react-dom";
@@ -403,7 +403,7 @@ const initializeDialog = (prompt: HTMLElement, tg: HTMLElement | null) => {
                 "bloom-unused-in-lang",
                 i >= letters.length,
             );
-            copyContentToTarget(draggables[i]);
+            copyContentToTargetAndCleanup(draggables[i]);
         }
         const shuffledDraggables = draggables.slice();
         shuffledDraggables.splice(letters.length); // don't want any invisible ones taking up space