BL-16822 Inline images: teach the code that walks a text block's children

src/BloomBrowserUI/bookEdit/bloomField/BloomField.ts

@@ -867,20 +867,41 @@ export default class BloomField {
     // inadvertently remove the embedded images. So we introduced the "bloom-preventRemoval" class, and this
     // tries to safeguard elements bearing that class.
     private static PreventRemovalOfSomeElements(field: HTMLElement) {
-        const numberThatShouldBeThere = $(field).find(
-            ".bloom-preventRemoval",
-        ).length;
-        if (numberThatShouldBeThere > 0) {
-            $(field).keyup((e) => {
-                if (
-                    $(field).find(".bloom-preventRemoval").length <
-                    numberThatShouldBeThere
-                ) {
-                    document.execCommand("undo");
-                    e.preventDefault();
-                }
-            });
-        }
+        // The count is taken on each keydown and compared on the matching keyup, so what this
+        // guards is the keystroke itself. Taking it once here instead would get two cases wrong,
+        // and inline images reach both: an image added AFTER page setup was never counted and so
+        // was unprotected, and an image the person deliberately deleted (from its menu) left the
+        // count permanently short, so every keystroke they typed afterwards fired a browser undo.
+        let countBeforeTheKeystroke = 0;
+        // Auto-repeat sends a whole run of keydowns before the single keyup that ends them, so
+        // only the first one of a run saw the field as it was before anything was deleted. Held
+        // Delete used to get an image past this guard for exactly that reason: keydown number
+        // two re-read the count AFTER the deletion, so the keyup had nothing to compare against
+        // and the image stayed deleted.
+        let aKeyIsDown = false;
+        const countPreventRemoval = () =>
+            $(field).find(".bloom-preventRemoval").length;
+        $(field).keydown(() => {
+            if (aKeyIsDown) return;
+            aKeyIsDown = true;
+            countBeforeTheKeystroke = countPreventRemoval();
+        });
+        $(field).keyup((e) => {
+            aKeyIsDown = false;
+            if (countPreventRemoval() < countBeforeTheKeystroke) {
+                document.execCommand("undo");
+                e.preventDefault();
+            }
+            countBeforeTheKeystroke = countPreventRemoval();
+        });
+        // A key held down while the focus leaves the field never delivers its keyup here, which
+        // would leave the flag set and the count stale -- the state this guard used to be in
+        // permanently. Losing the focus ends the run.
+        // (A native listener, not jQuery's focusout: jQuery 3 synthesizes focusin/focusout from
+        // focus/blur, which a dispatched focusout event does not go through.)
+        field.addEventListener("focusout", () => {
+            aKeyIsDown = false;
+        });
 
         //OK, now what if the above fails in some scenario? This adds a last-resort way of getting
         //bloom-editable back to the state it was in when the page was first created, by having

src/BloomBrowserUI/bookEdit/bloomField/bloomFieldSpec.ts

@@ -1,6 +1,6 @@
 ///<reference path="BloomField.ts" />
 ///<reference path="../../typings/bundledFromTSC.d.ts"/>
-import { describe, it, expect, beforeEach, afterAll } from "vitest";
+import { describe, it, expect, beforeEach, afterAll, vi } from "vitest";
 import { getTestRoot, removeTestRoot } from "../../utils/testHelper";
 import BloomField from "./BloomField";
 import $ from "jquery";
@@ -264,6 +264,188 @@ describe("BloomField", () => {
         expect($("div p").length).toBeGreaterThan(0);
     });
 
+    // Inline (Word-style) images use the same two protections that the old embedded-image
+    // templates did, so these tests pin down that BloomField still recognizes them by class
+    // when the class is on a .bloom-inlineImage wrapper. See inlineImages.ts.
+    describe("inline image protections", () => {
+        const inlineImageHtml =
+            '<div class="bloom-inlineImage bloom-inlineImageRight bloom-keepFirstInField bloom-preventRemoval" contenteditable="false"><img src="placeHolder.png" alt=""></div>';
+
+        it("EnsureParagraphsPresent puts the <p> after a bloom-keepFirstInField inline image", () => {
+            const editable = document.getElementById("simple")!;
+            editable.innerHTML = inlineImageHtml;
+            // Sanity check: no paragraph yet, and the image is the only child.
+            expect(editable.querySelectorAll("p").length).toBe(0);
+            expect(editable.children.length).toBe(1);
+
+            WireUp();
+
+            expect(editable.querySelectorAll("p").length).toBe(1);
+            // The image must stay first (that is what the class means) with the paragraph
+            // after it, since the text has to come after the float to wrap around it.
+            expect(
+                editable.firstElementChild!.classList.contains(
+                    "bloom-inlineImage",
+                ),
+            ).toBe(true);
+            expect(editable.lastElementChild!.tagName).toBe("P");
+        });
+
+        it("counts a bloom-preventRemoval inline image, so ctrl+a DEL is undone", () => {
+            const editable = document.getElementById("simple")!;
+            editable.innerHTML = inlineImageHtml + "<p>Some text</p>";
+            WireUp();
+            // jsdom has no execCommand, and we only want to know that BloomField asked for
+            // the undo.
+            const execCommand = vi.fn();
+            (document as any).execCommand = execCommand;
+
+            // Simulate the damage ctrl+a DEL does: the keydown, then the removal it caused,
+            // then the keyup. The guard compares the count across the keystroke, so the
+            // keydown is part of the gesture, not scaffolding.
+            editable.dispatchEvent(
+                new KeyboardEvent("keydown", { bubbles: true }),
+            );
+            editable.querySelector(".bloom-inlineImage")!.remove();
+            editable.dispatchEvent(
+                new KeyboardEvent("keyup", { bubbles: true }),
+            );
+
+            expect(execCommand).toHaveBeenCalledWith("undo");
+        });
+
+        // What made this a test: the count used to be taken once, at page setup, so a picture
+        // the person deleted from its own menu left it short for good and every keystroke they
+        // typed afterwards fired a browser undo -- taking back their typing, character by
+        // character. Comparing across the keystroke instead means a deletion nothing typed is
+        // simply the new state of the box.
+        it("does not undo the typing that follows a deliberate deletion of the image", () => {
+            const editable = document.getElementById("simple")!;
+            editable.innerHTML = inlineImageHtml + "<p>Some text</p>";
+            WireUp();
+            const execCommand = vi.fn();
+            (document as any).execCommand = execCommand;
+
+            // The menu's Delete: no keystroke involved.
+            editable.querySelector(".bloom-inlineImage")!.remove();
+
+            // And now the person types.
+            for (let i = 0; i < 3; i++) {
+                editable.dispatchEvent(
+                    new KeyboardEvent("keydown", { bubbles: true }),
+                );
+                editable.dispatchEvent(
+                    new KeyboardEvent("keyup", { bubbles: true }),
+                );
+            }
+
+            expect(execCommand).not.toHaveBeenCalled();
+        });
+
+        // Holding Delete rather than pressing it: the browser's auto-repeat sends a run of
+        // keydowns and one keyup at the end. Re-reading the count on every keydown meant the
+        // repeat that followed the deletion recorded the ALREADY-LOWER count, so the keyup had
+        // nothing to compare against and the picture stayed deleted.
+        it("protects the image when delete is held down rather than pressed", () => {
+            const editable = document.getElementById("simple")!;
+            editable.innerHTML = inlineImageHtml + "<p>Some text</p>";
+            WireUp();
+            const execCommand = vi.fn();
+            (document as any).execCommand = execCommand;
+
+            // First press: the deletion happens.
+            editable.dispatchEvent(
+                new KeyboardEvent("keydown", { bubbles: true, repeat: false }),
+            );
+            editable.querySelector(".bloom-inlineImage")!.remove();
+            // ...and the key is still down, so auto-repeat keeps sending keydowns.
+            for (let i = 0; i < 3; i++) {
+                editable.dispatchEvent(
+                    new KeyboardEvent("keydown", {
+                        bubbles: true,
+                        repeat: true,
+                    }),
+                );
+            }
+            // The one keyup, when they finally let go.
+            editable.dispatchEvent(
+                new KeyboardEvent("keyup", { bubbles: true }),
+            );
+
+            expect(execCommand).toHaveBeenCalledWith("undo");
+        });
+
+        // The flag that makes the above work has to be cleared when the field loses the focus,
+        // or a key held down as the focus moves away would leave it set and the count stale --
+        // and a stale count is what made every keystroke fire a browser undo.
+        it("recovers if the focus leaves while a key is held down", () => {
+            const editable = document.getElementById("simple")!;
+            editable.innerHTML = inlineImageHtml + "<p>Some text</p>";
+            WireUp();
+            const execCommand = vi.fn();
+            (document as any).execCommand = execCommand;
+
+            // A key goes down, and the focus leaves before its keyup arrives.
+            editable.dispatchEvent(
+                new KeyboardEvent("keydown", { bubbles: true }),
+            );
+            editable.dispatchEvent(
+                new FocusEvent("focusout", { bubbles: true }),
+            );
+            // The picture goes, from the menu this time: no keystroke to blame.
+            editable.querySelector(".bloom-inlineImage")!.remove();
+
+            // Now they type. The count must have been re-read, so this is just the new state.
+            editable.dispatchEvent(
+                new KeyboardEvent("keydown", { bubbles: true }),
+            );
+            editable.dispatchEvent(
+                new KeyboardEvent("keyup", { bubbles: true }),
+            );
+
+            expect(execCommand).not.toHaveBeenCalled();
+        });
+
+        // The other half: an image inserted after page setup was never counted, so ctrl+a DEL
+        // could take it out with nothing to put it back.
+        it("protects an inline image inserted after the field was wired up", () => {
+            const editable = document.getElementById("simple")!;
+            editable.innerHTML = "<p>Some text</p>";
+            WireUp();
+            const execCommand = vi.fn();
+            (document as any).execCommand = execCommand;
+            // Sanity check: nothing to protect when the field was wired up.
+            expect(
+                editable.querySelectorAll(".bloom-preventRemoval").length,
+            ).toBe(0);
+
+            editable.insertAdjacentHTML("afterbegin", inlineImageHtml);
+            editable.dispatchEvent(
+                new KeyboardEvent("keydown", { bubbles: true }),
+            );
+            editable.querySelector(".bloom-inlineImage")!.remove();
+            editable.dispatchEvent(
+                new KeyboardEvent("keyup", { bubbles: true }),
+            );
+
+            expect(execCommand).toHaveBeenCalledWith("undo");
+        });
+
+        it("does not undo on a keyup that left the inline image alone", () => {
+            const editable = document.getElementById("simple")!;
+            editable.innerHTML = inlineImageHtml + "<p>Some text</p>";
+            WireUp();
+            const execCommand = vi.fn();
+            (document as any).execCommand = execCommand;
+
+            editable.dispatchEvent(
+                new KeyboardEvent("keyup", { bubbles: true }),
+            );
+
+            expect(execCommand).not.toHaveBeenCalled();
+        });
+    });
+
     // Content converted from other formats can arrive with a real heading as the first thing
     // in the box. Prepending an empty paragraph above it would show the reader a blank first
     // line, and saving the page would make that permanent.

src/BloomBrowserUI/bookEdit/bloomField/test.pug

@@ -75,6 +75,45 @@ html
 				display: block;
 				clear: both;
 			}
+
+			/* Inline (Word-style) images. A real book gets these from
+			   content/bookLayout/inlineImages.less; this hand-test page has no basePage.css,
+			   so the load-bearing rules are repeated here. */
+			.bloom-editable:has(> .bloom-inlineImage) {
+				/* contain the float, so the image can't hang out of the field */
+				display: flow-root;
+			}
+			.bloom-inlineImage img {
+				display: block;
+				width: 100%;
+				aspect-ratio: var(--inline-image-aspect-ratio, auto);
+			}
+			.bloom-inlineImageLeft,
+			.bloom-inlineImageMiddle {
+				/* the offset is transparent padding, and the wrap shape excludes it, so text
+				   flows through it at full width */
+				padding-top: var(--inline-image-offset, 0px);
+				shape-outside: inset(var(--inline-image-offset, 0px) 0 0 0);
+				clear: both;
+			}
+			.bloom-inlineImageLeft {
+				float: left;
+				width: var(--inline-image-width, 40%);
+				margin: 0 1em 0.5em 0;
+			}
+			.bloom-inlineImageMiddle {
+				float: left;
+				width: 100%;
+			}
+			.bloom-inlineImageMiddle img {
+				width: var(--inline-image-width, 40%);
+				margin: 0 auto;
+			}
+			#inlineImageLeft,
+			#inlineImageMiddle {
+				height: 220px;
+				width: 400px;
+			}
 		script(type='text/javascript').
 			$(document).ready(function () {
 				$(".bloom-editable").each(function () {
@@ -103,6 +142,19 @@ html
 							.caption(contenteditable='true', lang='teo')
 								| Caption
 					//- p.bloom-cloneToOtherLanguages.bloom-preventRemoval
+				h4 Fields with an inline (Word-style) image
+				ul
+					li The text should wrap around the image, and the first lines should run at full width above it (that is the vertical offset).
+					li You should not be able to delete the image, and it should not move up or down as you type.
+					li In the second field the image is a full-width band: text above and below it, none beside it.
+				+field#inlineImageLeft
+					.bloom-inlineImage.bloom-inlineImageLeft.bloom-keepFirstInField.bloom-preventRemoval(contenteditable='false', style='--inline-image-width: 40%; --inline-image-offset: 40px; --inline-image-aspect-ratio: 4 / 3')
+						img(src='../../images/experiment.png', alt='')
+					p Docked left with a 40px offset. These first words should start at the very left edge of the field, above the beaker, and only the later lines should be pushed over to the right of it.
+				+field#inlineImageMiddle
+					.bloom-inlineImage.bloom-inlineImageMiddle.bloom-keepFirstInField.bloom-preventRemoval(contenteditable='false', style='--inline-image-width: 50%; --inline-image-offset: 30px; --inline-image-aspect-ratio: 4 / 3')
+						img(src='../../images/experiment.png', alt='')
+					p A middle band. This first line belongs above the picture, and everything after the band starts again below it at full width.
 				+field#brAtStart
 					br
 					| brAtStart There was a br at the start here

src/BloomBrowserUI/bookEdit/sourceBubbles/SourceBubblesSpec.ts

@@ -196,6 +196,42 @@ describe("SourceBubbles", () => {
         ]);
     });
 
+    // Inline (Word-style) images live inside each bloom-editable, so they are inside the
+    // clone that becomes the source bubble too. They must not show up there: a source bubble
+    // is for reading another language's text, and the picture is the same in every language
+    // anyway. Nothing in this file does that on purpose -- the existing hasNoText pass drops
+    // the text-less wrapper div, taking the img with it -- so this test pins that down,
+    // because the whole v1 design leans on it. See INLINE-IMAGES-PLAN.md.
+    it("MakeSourceTextDivForGroup drops inline images from the bubble", () => {
+        const inlineImage =
+            "<div class='bloom-inlineImage bloom-inlineImageRight bloom-keepFirstInField bloom-preventRemoval' contenteditable='false'><img src='flower.jpg'/></div>";
+        const testHtml = $(
+            [
+                "<div id='testTarget' class='bloom-translationGroup'>",
+                `   <div class='bloom-editable' lang='es'>${inlineImage}<p>Spanish text</p></div>`,
+                `   <div class='bloom-editable bloom-content1 bloom-visibility-code-on' lang='en'>${inlineImage}<p>English text</p></div>`,
+                `   <div class='bloom-editable' lang='tpi'>${inlineImage}<p>Tok Pisin text</p></div>`,
+                "</div>",
+            ].join("\n"),
+        );
+        $("body").append(testHtml);
+        // Sanity check: the images really are in the group we are about to clone.
+        expect($("#testTarget img").length).toBe(3);
+
+        const result = BloomSourceBubbles.MakeSourceTextDivForGroup(
+            $("body").find("#testTarget")[0],
+        );
+
+        // The bubble still has the source languages...
+        expect(result.find("div.source-text").length).toBe(2);
+        expect(result.find("div.source-text[lang=es]").text().trim()).toBe(
+            "Spanish text",
+        );
+        // ...but no trace of the image.
+        expect(result.find("img").length).toBe(0);
+        expect(result.find(".bloom-inlineImage").length).toBe(0);
+    });
+
     it("Run CreateDropdownIfNecessary with pre-defined settings", () => {
         const testHtml = $(
             [

src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.ts

@@ -3811,7 +3811,12 @@ export default class AudioRecording implements IAudioRecorder {
                         name != "u" && // ckeditor underline
                         name != "sup" && // ckeditor superscript
                         name != "a" && // Allow users to manually insert hyperlinks 4.5, and support 4.6 hyperlinks
-                        !$(child).hasClass("bloom-ui") // don't process transient UI elements (e.g. the format button)
+                        !$(child).hasClass("bloom-ui") && // don't process transient UI elements (e.g. the format button)
+                        // Don't process non-editable islands embedded in the text, such as an
+                        // inline image (bloom-inlineImage). They hold no recordable text, and
+                        // recursing into one ends at the img element, which would then be
+                        // treated as a leaf and have audio markup written into it.
+                        child.getAttribute("contenteditable") !== "false"
                     ) {
                         processedChild = true;
                         updateFuncs.push(