From bd95d471498de2cf6ceaf500bfd1a9612c369401 Mon Sep 17 00:00:00 2001 From: Hatton Date: Mon, 7 Sep 2026 11:07:03 -0600 Subject: [PATCH] BL-16822 Inline images: teach the code that walks a text block's children Six places walk the children of a bloom-editable, or judge a block empty by its InnerText. A picture in the text is a non-editable island in there, and a block holding only a picture and the empty paragraph that has to follow it has no text at all, so each of them had something to get wrong. The production diffs are a few lines each; most of this commit is the tests that pin them. - TranslationGroupManager.FixDuplicateLanguageDivs discarded the div holding the picture and kept the genuinely empty one, because InnerText alone says the picture's div is the empty one. - BloomField's preventRemoval guard took its expected count once at page setup, so a picture added later was unprotected, and a picture the person deliberately deleted left the count permanently short and fired a browser undo on every keystroke afterwards. The count is now taken on each keydown and compared on its keyup, so what it guards is the keystroke. - The Talking Book tool recursed into the wrapper, reached the img, treated it as a leaf and wrote audio markup into it. It now stops at any contenteditable="false" island. - Source bubbles must not show the picture: a bubble is for reading another language's text, and the picture is the same in every language. The existing hasNoText pass already drops it; the test pins that, because the design leans on it. - The level-7 bloom-canvas migration must pass the wrapper by, which is why the wrapper has its own class rather than bloom-imageContainer. A real image container on the same page is still renamed, which proves the migration ran. - PublishModel.RemoveUnwantedLanguageData removes a div per unpublished language, and the image file survives only while something still refers to it. The prototype's copy is what keeps it alive, since "z" is always kept. Pinned, not changed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014DCBGajN5YyAYPBenEf1yy --- .../bookEdit/bloomField/BloomField.ts | 49 +- .../bookEdit/bloomField/bloomFieldSpec.ts | 184 ++++- .../bookEdit/bloomField/test.pug | 52 ++ .../sourceBubbles/SourceBubblesSpec.ts | 36 + .../toolbox/talkingBook/audioRecording.ts | 7 +- .../toolbox/talkingBook/audioRecordingSpec.ts | 30 + src/BloomExe/Book/TranslationGroupManager.cs | 45 +- src/BloomTests/Book/BookStorageTests.cs | 74 ++ .../Book/TranslationGroupManagerTests.cs | 644 +++++++++++++++++- src/BloomTests/Publish/PublishModelTests.cs | 64 +- 10 files changed, 1165 insertions(+), 20 deletions(-) diff --git a/src/BloomBrowserUI/bookEdit/bloomField/BloomField.ts b/src/BloomBrowserUI/bookEdit/bloomField/BloomField.ts index fe3e84ae2d17..af0eb8ccb6c6 100644 --- a/src/BloomBrowserUI/bookEdit/bloomField/BloomField.ts +++ b/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 diff --git a/src/BloomBrowserUI/bookEdit/bloomField/bloomFieldSpec.ts b/src/BloomBrowserUI/bookEdit/bloomField/bloomFieldSpec.ts index c5e60af32ded..668113e99a23 100644 --- a/src/BloomBrowserUI/bookEdit/bloomField/bloomFieldSpec.ts +++ b/src/BloomBrowserUI/bookEdit/bloomField/bloomFieldSpec.ts @@ -1,6 +1,6 @@ /// /// -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 = + '
'; + + it("EnsureParagraphsPresent puts the

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 + "

Some text

"; + 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 + "

Some text

"; + 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 + "

Some text

"; + 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 + "

Some text

"; + 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 = "

Some text

"; + 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 + "

Some text

"; + 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. diff --git a/src/BloomBrowserUI/bookEdit/bloomField/test.pug b/src/BloomBrowserUI/bookEdit/bloomField/test.pug index 987817cf9155..8ad2ce901fe0 100644 --- a/src/BloomBrowserUI/bookEdit/bloomField/test.pug +++ b/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 diff --git a/src/BloomBrowserUI/bookEdit/sourceBubbles/SourceBubblesSpec.ts b/src/BloomBrowserUI/bookEdit/sourceBubbles/SourceBubblesSpec.ts index dca0d587a5bd..632963c409b9 100644 --- a/src/BloomBrowserUI/bookEdit/sourceBubbles/SourceBubblesSpec.ts +++ b/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 = + "
"; + const testHtml = $( + [ + "
", + `
${inlineImage}

Spanish text

`, + `
${inlineImage}

English text

`, + `
${inlineImage}

Tok Pisin text

`, + "
", + ].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 = $( [ diff --git a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.ts b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.ts index 1199dff5aaac..4bd7b3bf5271 100644 --- a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.ts +++ b/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( diff --git a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecordingSpec.ts b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecordingSpec.ts index 29ff89db35a9..76c0bc1783df 100644 --- a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecordingSpec.ts +++ b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecordingSpec.ts @@ -682,6 +682,36 @@ describe("audio recording tests", () => { ); }); + // An inline (Word-style) image is a contenteditable=false island inside the + // bloom-editable. It holds no recordable text, and recursing into it bottoms out at + // the img, which would then be treated as a leaf and get audio markup written into + // it. See inlineImages.ts. + it("skips a contenteditable=false island such as an inline image", () => { + const islandHtml = + '
'; + const div = $( + `
${islandHtml}

This is a sentence. This is another.

`, + ); + const recording = new AudioRecording(); + recording.makeAudioSentenceElementsTest( + div, + RecordingMode.Sentence, + ); + + // The paragraph got its sentence spans as usual... + const spans = div.find("p span.audio-sentence"); + expect(spans.length).toBe(2); + // ...and the island came through untouched: no spans, no id, no audio class. + const island = div.find(".bloom-inlineImage"); + expect(island.length).toBe(1); + expect(island.find("span").length).toBe(0); + expect(island.attr("class")).toBe( + "bloom-inlineImage bloom-inlineImageRight", + ); + expect(island.attr("id")).toBeUndefined(); + expect(island.html()).toBe(''); + }); + it("flattens nested audio spans", () => { const p = $( '

This is the first. This is the second. This is the third.

', diff --git a/src/BloomExe/Book/TranslationGroupManager.cs b/src/BloomExe/Book/TranslationGroupManager.cs index 3a605510e8dd..d67ae8dfc9c5 100644 --- a/src/BloomExe/Book/TranslationGroupManager.cs +++ b/src/BloomExe/Book/TranslationGroupManager.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -857,7 +857,7 @@ internal static void FixDuplicateLanguageDivs(SafeXmlElement groupElement, strin foreach (var div in list) { var innerText = div.InnerText.Trim(); - if (String.IsNullOrEmpty(innerText)) + if (String.IsNullOrEmpty(innerText) && !HasInlineImage(div)) { Logger.WriteEvent( $"An empty duplicate div for {langTag} has been removed from a translation group." @@ -882,13 +882,54 @@ internal static void FixDuplicateLanguageDivs(SafeXmlElement groupElement, strin ); first.AppendChild(newline); foreach (SafeXmlNode node in list[i].ChildNodes) + { + // The duplicate normally holds a COPY of a picture the survivor + // already has -- that is what duplicating a block produces -- and + // appending it would leave the same picture in the block twice, which + // the reader sees twice. Sameness is by id; a wrapper without one + // cannot be matched to anything, so it is kept. + if (IsInlineImageAlreadyPresent(first, node)) + continue; first.AppendChild(node); + } groupElement.RemoveChild(list[i]); } } } } + /// + /// Whether this node is an inline (Word-style) image wrapper that the given editable + /// already holds a copy of. The per-language copies of one picture share a + /// data-bloom-inline-image-id (see inlineImages.ts), and so do the copies that + /// duplicating a block makes, which is what identifies the repeat. + /// + private static bool IsInlineImageAlreadyPresent(SafeXmlNode editable, SafeXmlNode node) + { + var element = node as SafeXmlElement; + if (element == null || !element.HasClass("bloom-inlineImage")) + return false; + var id = element.GetAttribute("data-bloom-inline-image-id"); + if (String.IsNullOrEmpty(id)) + return false; + return editable + .SafeSelectNodes(".//div[@data-bloom-inline-image-id='" + id + "']") + .Length > 0; + } + + /// + /// Whether this editable holds an inline (Word-style) image: a .bloom-inlineImage wrapper + /// (see inlineImages.ts). Such a block can be entirely without text -- a picture and the + /// empty paragraph that has to follow it -- so InnerText alone says it is empty, and + /// anything that discards an "empty" block would discard the picture with it. + /// + private static bool HasInlineImage(SafeXmlNode div) + { + return div.SafeSelectNodes( + ".//div[contains(concat(' ', @class, ' '), ' bloom-inlineImage ')]" + ).Length > 0; + } + /// /// Shift any translationGroup level style setting to child editable divs that lack a style. /// This is motivated by the fact HTML/CSS underlining cannot be turned off in child nodes. diff --git a/src/BloomTests/Book/BookStorageTests.cs b/src/BloomTests/Book/BookStorageTests.cs index 3d8d99247f4b..8533e02617c1 100644 --- a/src/BloomTests/Book/BookStorageTests.cs +++ b/src/BloomTests/Book/BookStorageTests.cs @@ -2172,6 +2172,80 @@ public void PerformNecessaryMaintenanceOnBook_UpdatesToBloomCanvas() ); } + /// + /// The inline image wrapper deliberately uses its own class (bloom-inlineImage) rather than + /// bloom-imageContainer, so that the level-7 bloom-canvas migration passes it by. This locks + /// that in: a real bloom-imageContainer on the same page IS renamed (proving the migration + /// ran), while the wrapper and its img come through untouched. + /// + [Test] + public void MigrateToLevel7BloomCanvas_LeavesInlineImageAlone() + { + var storage = GetInitialStorageWithCustomHtml( + @" + + + + + +
+
+ +
+
+
+
+ +
+

Text that wraps around the image.

+
+
+
+" + ); + + // Sanity check the starting state: one imageContainer, one inline image wrapper, no bloom-canvas. + Assert.That( + storage.Dom.SelectSingleNode("//*[@id='shouldBeRenamed']").GetAttribute("class"), + Is.EqualTo("bloom-imageContainer") + ); + AssertThatXmlIn + .Dom(storage.Dom.RawDom) + .HasNoMatchForXpath("//div[contains(@class,'bloom-canvas')]"); + + //SUT + storage.MigrateToLevel7BloomCanvas(); + + //Verification + Assert.That(storage.Dom.GetMetaValue("maintenanceLevel", "0"), Is.EqualTo("7")); + // The migration really did run: the genuine image container is now a bloom-canvas. + var migrated = storage.Dom.SelectSingleNode("//*[@id='shouldBeRenamed']"); + Assert.That(migrated.GetAttribute("class"), Does.Contain("bloom-canvas")); + Assert.That(migrated.GetAttribute("class"), Does.Not.Contain("bloom-imageContainer")); + + // The inline image wrapper is exactly as it was. + var wrapper = storage.Dom.SelectSingleNode("//*[@id='inlineWrapper']"); + var classes = wrapper.GetAttribute("class"); + Assert.That(classes, Does.Contain("bloom-inlineImage")); + Assert.That(classes, Does.Contain("bloom-inlineImageRight")); + Assert.That(classes, Does.Contain("bloom-keepFirstInField")); + Assert.That(classes, Does.Contain("bloom-preventRemoval")); + Assert.That( + classes, + Does.Not.Contain("bloom-canvas"), + "the migration must not turn the inline image wrapper into a bloom-canvas" + ); + Assert.That(wrapper.GetAttribute("contenteditable"), Is.EqualTo("false")); + Assert.That(wrapper.GetAttribute("style"), Does.Contain("width: 40%")); + AssertThatXmlIn + .Dom(storage.Dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[@id='inlineWrapper']/img[@src='flower.jpg']", + 1 + ); + } + [Test] public void PerformNecessaryMaintenanceOnBook_EnsuresImgAtStartOfImageContainer() { diff --git a/src/BloomTests/Book/TranslationGroupManagerTests.cs b/src/BloomTests/Book/TranslationGroupManagerTests.cs index eb7af9caa9ed..62f25925e15e 100644 --- a/src/BloomTests/Book/TranslationGroupManagerTests.cs +++ b/src/BloomTests/Book/TranslationGroupManagerTests.cs @@ -1,4 +1,4 @@ -using System.Collections.Generic; +using System.Collections.Generic; using System.Linq; using System.Xml; using Bloom.Book; @@ -1651,6 +1651,95 @@ public void FixDuplicateLanguageDivs_HandlesEmptyFirstDiv() ); } + // A block whose only content is an inline (Word-style) image has no text in it, so + // judging "empty" by InnerText alone deleted the picture along with the div. The person + // who put the picture there and typed nothing beside it would find it simply gone. + [Test] + public void FixDuplicateLanguageDivs_KeepsADivWhoseOnlyContentIsAnInlineImage() + { + var contents = """ +
+
+

+
+
+
+ """; + var dom = SafeXmlDocument.Create(); + dom.LoadXml(contents); + // Sanity check: the div with the picture really does have no text of its own, which + // is what made it look empty. + var withThePicture = (SafeXmlElement) + dom.SafeSelectNodes("//div[@data-languagetipcontent='First']")[0]; + Assert.That(withThePicture.InnerText.Trim(), Is.Empty); + + TranslationGroupManager.FixDuplicateLanguageDivs( + (SafeXmlElement) + dom.SafeSelectNodes("//div[contains(@class,'bloom-translationGroup')]")[0], + "xyz" + ); + + AssertThatXmlIn + .Dom(dom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[contains(@class,'bloom-editable') and @lang='xyz']", + 1 + ); + AssertThatXmlIn + .Dom(dom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[contains(@class,'bloom-editable') and @lang='xyz']//img[@src='bird.png']", + 1 + ); + } + + // Duplicating a block gives both copies the same picture, and neither is empty, so the + // merge branch runs. Appending the second block's children wholesale put a second copy of + // the one picture into the survivor, and the reader saw it twice. + [Test] + public void FixDuplicateLanguageDivs_MergeDoesNotRepeatTheSameInlineImage() + { + var contents = """ +
+
+

First Xyz text

+

Second Xyz text

+
+
+ """; + var dom = SafeXmlDocument.Create(); + dom.LoadXml(contents); + // Sanity check: both blocks have text, so neither can be dropped as empty, and each + // holds a copy of the same picture. + Assert.That( + dom.SafeSelectNodes("//div[@lang='xyz']//img[@src='bird.png']").Length, + Is.EqualTo(2) + ); + + TranslationGroupManager.FixDuplicateLanguageDivs( + (SafeXmlElement) + dom.SafeSelectNodes("//div[contains(@class,'bloom-translationGroup')]")[0], + "xyz" + ); + + AssertThatXmlIn + .Dom(dom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[contains(@class,'bloom-editable') and @lang='xyz']", + 1 + ); + AssertThatXmlIn + .Dom(dom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[contains(@class,'bloom-editable') and @lang='xyz']//img[@src='bird.png']", + 1 + ); + // Both blocks' text is still kept: it is only the repeated picture that is dropped. + var merged = dom.SafeSelectNodes("//div[@lang='xyz']")[0].InnerText; + Assert.That(merged, Does.Contain("First Xyz text")); + Assert.That(merged, Does.Contain("Second Xyz text")); + } + [Test] public void FixDuplicateLanguageDivs_HandlesNonemptyDivs() { @@ -2301,5 +2390,558 @@ public void MakeElementWithLanguageForOneGroup_EnsuresStyleClassExists() 3 ); } + + /// + /// An inline image (the Word-style image wrapper that lives INSIDE a bloom-editable) must + /// survive the creation of a new language block: the whole wrapper, its classes, its + /// contenteditable='false', its inline style (dock offset/width/aspect-ratio) and its img + /// are part of the "structure" that gets cloned, while the prototype's text is stripped. + /// + [Test] + public void PrepareElementsInPageOrDocument_PrototypeEditableHasInlineImage_WrapperClonedIntact() + { + const string contents = + @"
+
+
+
+ +
+

Do not copy me.

+
+
+
"; + var dom = new HtmlDom(contents); + var bookData = new BookData(dom, _collectionSettings, null); + + // Sanity check: exactly one inline image wrapper, in the English block, before we start. + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[contains(@class,'bloom-inlineImage')]", + 1 + ); + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath("//div[@lang='fr']", 0); + + TranslationGroupManager.PrepareElementsInPageOrDocument( + (SafeXmlElement)dom.SafeSelectNodes("//div[contains(@class,'bloom-page')]")[0], + bookData + ); + + // A block was made for each of xyz (L1), fr (L2) and es (L3), and each got its own copy + // of the wrapper (4 blocks in all, counting the original English one). + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[contains(@class,'bloom-inlineImage')]", + 4 + ); + + var frWrapper = dom.SelectSingleNode( + "//div[@lang='fr']/div[contains(@class,'bloom-inlineImage')]" + ); + Assert.That( + frWrapper, + Is.Not.Null, + "the new French block should have gotten a wrapper" + ); + var frWrapperClasses = frWrapper.GetAttribute("class"); + Assert.That(frWrapperClasses, Does.Contain("bloom-inlineImage")); + Assert.That(frWrapperClasses, Does.Contain("bloom-inlineImageRight")); + Assert.That(frWrapperClasses, Does.Contain("bloom-keepFirstInField")); + Assert.That(frWrapperClasses, Does.Contain("bloom-preventRemoval")); + Assert.That(frWrapper.GetAttribute("contenteditable"), Is.EqualTo("false")); + var frWrapperStyle = frWrapper.GetAttribute("style"); + Assert.That(frWrapperStyle, Does.Contain("--inline-image-offset: 120px")); + Assert.That(frWrapperStyle, Does.Contain("width: 40%")); + Assert.That(frWrapperStyle, Does.Contain("aspect-ratio: 800 / 600")); + + // The wrapper is still the first child of the new editable (bloom-keepFirstInField's slot). + var frEditable = dom.SelectSingleNode("//div[@lang='fr']"); + var firstChildElement = + frEditable.ChildNodes.FirstOrDefault(x => x is SafeXmlElement) as SafeXmlElement; + Assert.That( + firstChildElement.GetAttribute("class"), + Does.Contain("bloom-inlineImage"), + "the wrapper should still be the first child element of the new editable" + ); + + // The img and its metadata came along... + var frImg = dom.SelectSingleNode( + "//div[@lang='fr']/div[contains(@class,'bloom-inlineImage')]/img" + ); + Assert.That(frImg, Is.Not.Null); + Assert.That(frImg.GetAttribute("src"), Is.EqualTo("flower.jpg")); + Assert.That(frImg.GetAttribute("data-copyright"), Is.EqualTo("Copyright Me")); + Assert.That(frImg.GetAttribute("data-license"), Is.EqualTo("cc-by")); + + // ...but the prototype's text did not. + AssertThatXmlIn.Dom(dom.RawDom).HasNoMatchForXpath("//div[@lang='fr']//p"); + Assert.That(frEditable.InnerText.Trim(), Is.Empty); + + // And the original English block was left with both its wrapper and its text. + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[@lang='en']/div[contains(@class,'bloom-inlineImage')]/img[@src='flower.jpg']", + 1 + ); + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[@lang='en']/p[contains(text(),'Do not copy me')]", + 1 + ); + } + + /// + /// PrepareElementsOnPageOneLanguage deletes language-less DIRECT children of a + /// translationGroup. An inline image wrapper has no lang, but it is nested inside a + /// bloom-editable rather than being a direct child of the group, so it must survive. + /// This checks both directions: the wrapper stays, a stray language-less direct child goes. + /// + [Test] + public void PrepareElementsInPageOrDocument_LangLessDivsRemoved_InlineImageWrapperSurvives() + { + const string contents = + @"
+
+
+
+ +
+
+
I have no lang and am a direct child, so I should be deleted.
+
+
"; + var dom = new HtmlDom(contents); + var bookData = new BookData(dom, _collectionSettings, null); + + // Sanity check: both the wrapper and the stray div are there to begin with. + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath("//div[contains(@class,'strayDiv')]", 1); + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[contains(@class,'bloom-inlineImage')]", + 1 + ); + + TranslationGroupManager.PrepareElementsInPageOrDocument( + (SafeXmlElement)dom.SafeSelectNodes("//div[contains(@class,'bloom-page')]")[0], + bookData + ); + + // The stray language-less direct child of the group is gone... + AssertThatXmlIn + .Dom(dom.RawDom) + .HasNoMatchForXpath("//div[contains(@class,'strayDiv')]"); + // ...while every editable (en plus the three new ones) still has its wrapper, complete + // with the img and the contenteditable='false' that makes it a non-editable island. + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[contains(@class,'bloom-editable')]/div[contains(@class,'bloom-inlineImage') and @contenteditable='false']/img[@src='flower.jpg']", + 4 + ); + // The wrapper must NOT have been given a lang by the "editables without a lang" sweep; + // that sweep only matches contenteditable='true' or bloom-editable. + AssertThatXmlIn + .Dom(dom.RawDom) + .HasNoMatchForXpath("//div[contains(@class,'bloom-inlineImage') and @lang]"); + } + + /// + /// UpdateContentLanguageClasses walks every div under a translationGroup, not just the + /// editables, so it sees the inline image wrapper too. It must not turn the wrapper into + /// something that looks like a visible language block: no bloom-visibility-code-on, no + /// bloom-contentN, and its own classes intact. + /// + [Test] + public void UpdateContentLanguageClasses_InlineImageWrapper_GetsNoVisibilityOrContentClasses() + { + const string contents = + @"
+
+
+
+ +
+

Some vernacular text.

+
+
+
+ +
+
+
+
"; + var dom = new HtmlDom(contents); + var bookData = new BookData(dom, _collectionSettings, null); + + // Sanity check: the only pre-existing generated class is the stale + // bloom-visibility-code-on we planted on the French wrapper, which proves below that + // the sweep really does reach inside the editables (and strips such classes off the + // wrapper, so the wrapper must never depend on one). + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[contains(@class,'bloom-inlineImage') and contains(@class,'bloom-visibility-code-on')]", + 1 + ); + AssertThatXmlIn + .Dom(dom.RawDom) + .HasNoMatchForXpath( + "//div[contains(@class,'bloom-editable') and contains(@class,'bloom-visibility-code-on')]" + ); + + var pageDiv = (SafeXmlElement) + dom.RawDom.SafeSelectNodes("//div[contains(@class,'bloom-page')]")[0]; + TranslationGroupManager.UpdateContentLanguageClasses( + pageDiv, + bookData, + LegacyAppearanceSettings, + "xyz", + "fr", + null + ); + + // Sanity check that the pass actually ran: the vernacular editable was turned on. + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[@lang='xyz' and contains(@class,'bloom-visibility-code-on') and contains(@class,'bloom-content1')]", + 1 + ); + + foreach ( + SafeXmlElement wrapper in dom.SafeSelectNodes( + "//div[contains(@class,'bloom-inlineImage')]" + ) + ) + { + var classes = wrapper.GetAttribute("class"); + Assert.That( + classes, + Does.Not.Contain("bloom-visibility-code"), + "the wrapper is not a language block and must not be marked visible/invisible" + ); + Assert.That( + classes, + Does.Not.Contain("bloom-content"), + "the wrapper must not gain bloom-contentN/bloom-contentNationalN classes" + ); + // Its own classes are all still there. + Assert.That(classes, Does.Contain("bloom-inlineImage")); + Assert.That(classes, Does.Contain("bloom-inlineImageRight")); + Assert.That(classes, Does.Contain("bloom-keepFirstInField")); + Assert.That(classes, Does.Contain("bloom-preventRemoval")); + Assert.That(wrapper.GetAttribute("contenteditable"), Is.EqualTo("false")); + Assert.That(wrapper.GetAttribute("style"), Does.Contain("width: 40%")); + } + } + + /// + /// Returns the data-bloom-inline-image-id of each inline image wrapper directly inside the + /// editable for the given language, in document order. + /// + private static string[] GetInlineImageIdsInOrder(HtmlDom dom, string langTag) + { + return dom.SafeSelectNodes( + $"//div[@lang='{langTag}']/div[contains(@class,'bloom-inlineImage')]" + ) + .Cast() + .Select(w => w.GetAttribute("data-bloom-inline-image-id")) + .ToArray(); + } + + /// + /// Multiple inline images per translation group are matched up across the sibling language + /// editables by data-bloom-inline-image-id, so cloning a prototype editable to make a new + /// language block must carry that attribute through unchanged on every wrapper, and must + /// keep the wrappers in the same order (the ids are the only thing tying a wrapper to its + /// counterparts, and the order is what the reader sees). + /// + [Test] + public void PrepareElementsInPageOrDocument_EditableHasTwoInlineImages_IdsAndOrderPreservedInClones() + { + const string contents = + @"
+
+
+
+ +
+

Do not copy me.

+
+ +
+

Do not copy me either.

+
+
+
"; + var dom = new HtmlDom(contents); + var bookData = new BookData(dom, _collectionSettings, null); + + // Sanity check the starting state: two wrappers, in the order abc123 then def456, + // and only in English so far. + Assert.That( + GetInlineImageIdsInOrder(dom, "en"), + Is.EqualTo(new[] { "abc123", "def456" }) + ); + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath("//div[@data-bloom-inline-image-id]", 2); + + TranslationGroupManager.PrepareElementsInPageOrDocument( + (SafeXmlElement)dom.SafeSelectNodes("//div[contains(@class,'bloom-page')]")[0], + bookData + ); + + // Every language block (en plus the new xyz, fr and es) has both wrappers, with the + // same ids in the same order. + foreach (var lang in new[] { "en", "xyz", "fr", "es" }) + { + Assert.That( + GetInlineImageIdsInOrder(dom, lang), + Is.EqualTo(new[] { "abc123", "def456" }), + $"the {lang} block should have both wrappers, in order, with their ids intact" + ); + } + + // Nothing renamed or de-duplicated the shared id values: each appears once per language. + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[@data-bloom-inline-image-id='abc123']", + 4 + ); + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[@data-bloom-inline-image-id='def456']", + 4 + ); + + // Each clone kept the rest of its wrapper too, matched to the right id. + var frFirst = dom.SelectSingleNode( + "//div[@lang='fr']/div[@data-bloom-inline-image-id='abc123']" + ); + Assert.That(frFirst.GetAttribute("class"), Does.Contain("bloom-inlineImageRight")); + Assert.That(frFirst.GetAttribute("style"), Does.Contain("width: 40%")); + Assert.That(frFirst.GetAttribute("contenteditable"), Is.EqualTo("false")); + Assert.That( + frFirst.SelectSingleNode("img").GetAttribute("src"), + Is.EqualTo("flower.jpg") + ); + var frSecond = dom.SelectSingleNode( + "//div[@lang='fr']/div[@data-bloom-inline-image-id='def456']" + ); + Assert.That(frSecond.GetAttribute("class"), Does.Contain("bloom-inlineImageLeft")); + Assert.That(frSecond.GetAttribute("style"), Does.Contain("width: 25%")); + Assert.That( + frSecond.SelectSingleNode("img").GetAttribute("src"), + Is.EqualTo("tree.jpg") + ); + + // The prototype's text still did not come along. + AssertThatXmlIn.Dom(dom.RawDom).HasNoMatchForXpath("//div[@lang='fr']//p"); + Assert.That(dom.SelectSingleNode("//div[@lang='fr']").InnerText.Trim(), Is.Empty); + } + + /// + /// The pass that deletes language-less direct children of a translation group must leave + /// multiple nested inline image wrappers alone, ids and all, and must not stamp a lang onto + /// them. + /// + [Test] + public void PrepareElementsInPageOrDocument_LangLessDivsRemoved_MultipleInlineImageIdsSurvive() + { + const string contents = + @"
+
+
+
+ +
+
+ +
+
+
I have no lang and am a direct child, so I should be deleted.
+
+
"; + var dom = new HtmlDom(contents); + var bookData = new BookData(dom, _collectionSettings, null); + + // Sanity check: the stray div and both wrappers are all there to begin with. + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath("//div[contains(@class,'strayDiv')]", 1); + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath("//div[@data-bloom-inline-image-id]", 2); + + TranslationGroupManager.PrepareElementsInPageOrDocument( + (SafeXmlElement)dom.SafeSelectNodes("//div[contains(@class,'bloom-page')]")[0], + bookData + ); + + // The stray language-less direct child of the group is gone... + AssertThatXmlIn + .Dom(dom.RawDom) + .HasNoMatchForXpath("//div[contains(@class,'strayDiv')]"); + // ...but the nested wrappers are not: 2 per language block, 8 in all, each still + // carrying its id, its contenteditable='false' and its img. + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath("//div[@data-bloom-inline-image-id]", 8); + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[contains(@class,'bloom-editable')]/div[@data-bloom-inline-image-id='abc123' and @contenteditable='false']/img[@src='flower.jpg']", + 4 + ); + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[contains(@class,'bloom-editable')]/div[@data-bloom-inline-image-id='def456' and @contenteditable='false']/img[@src='tree.jpg']", + 4 + ); + // The "editables without a lang" sweep must not have given the wrappers a lang; if it + // had, they would start collecting bloom-content* classes from UpdateContentLanguageClasses. + AssertThatXmlIn + .Dom(dom.RawDom) + .HasNoMatchForXpath("//div[@data-bloom-inline-image-id and @lang]"); + } + + /// + /// UpdateContentLanguageClasses walks every div under a translation group, so it sees the + /// inline image wrappers. With several wrappers per editable and the same id repeated across + /// sibling language editables, it must leave every data-bloom-inline-image-id exactly as it + /// found it, and add no visibility/content classes to the wrappers. + /// + [Test] + public void UpdateContentLanguageClasses_MultipleInlineImages_IdsUntouchedAcrossSiblings() + { + const string contents = + @"
+
+
+
+ +
+

Some vernacular text.

+
+ +
+
+
+
+ +
+
+ +
+
+
+
"; + var dom = new HtmlDom(contents); + var bookData = new BookData(dom, _collectionSettings, null); + + // Sanity check: the two sibling editables already share both id values, which is the + // whole point of the scheme, and nothing has a generated visibility class yet. + Assert.That( + GetInlineImageIdsInOrder(dom, "xyz"), + Is.EqualTo(new[] { "abc123", "def456" }) + ); + Assert.That( + GetInlineImageIdsInOrder(dom, "fr"), + Is.EqualTo(new[] { "abc123", "def456" }) + ); + AssertThatXmlIn + .Dom(dom.RawDom) + .HasNoMatchForXpath("//div[contains(@class,'bloom-visibility-code-on')]"); + + var pageDiv = (SafeXmlElement) + dom.RawDom.SafeSelectNodes("//div[contains(@class,'bloom-page')]")[0]; + TranslationGroupManager.UpdateContentLanguageClasses( + pageDiv, + bookData, + LegacyAppearanceSettings, + "xyz", + "fr", + null + ); + + // Sanity check that the pass actually ran: the vernacular editable was turned on. + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[@lang='xyz' and contains(@class,'bloom-visibility-code-on') and contains(@class,'bloom-content1')]", + 1 + ); + + // Both editables still have both wrappers, in order, with their shared ids intact. + Assert.That( + GetInlineImageIdsInOrder(dom, "xyz"), + Is.EqualTo(new[] { "abc123", "def456" }) + ); + Assert.That( + GetInlineImageIdsInOrder(dom, "fr"), + Is.EqualTo(new[] { "abc123", "def456" }) + ); + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[@data-bloom-inline-image-id='abc123']", + 2 + ); + AssertThatXmlIn + .Dom(dom.RawDom) + .HasSpecifiedNumberOfMatchesForXpath( + "//div[@data-bloom-inline-image-id='def456']", + 2 + ); + + foreach ( + SafeXmlElement wrapper in dom.SafeSelectNodes( + "//div[contains(@class,'bloom-inlineImage')]" + ) + ) + { + var classes = wrapper.GetAttribute("class"); + Assert.That( + classes, + Does.Not.Contain("bloom-visibility-code"), + "a wrapper is not a language block and must not be marked visible/invisible" + ); + Assert.That( + classes, + Does.Not.Contain("bloom-content"), + "a wrapper must not gain bloom-contentN/bloom-contentNationalN classes" + ); + Assert.That(wrapper.GetAttribute("contenteditable"), Is.EqualTo("false")); + Assert.That( + wrapper.GetAttribute("data-bloom-inline-image-id"), + Is.Not.Empty, + "the sweep must not have cleared the id that ties this wrapper to its counterparts" + ); + } + } } } diff --git a/src/BloomTests/Publish/PublishModelTests.cs b/src/BloomTests/Publish/PublishModelTests.cs index ec911beded1a..65aa6690ea26 100644 --- a/src/BloomTests/Publish/PublishModelTests.cs +++ b/src/BloomTests/Publish/PublishModelTests.cs @@ -1,4 +1,4 @@ -using System.Collections.Generic; +using System.Collections.Generic; using System.Linq; using Bloom.Book; using Bloom.Publish; @@ -716,6 +716,68 @@ Just Text assertThatDom.HasNoMatchForXpath("//div[@lang and @testRemoves='true']"); } + [Test] + public void RemoveUnwantedLanguageData_KeepsAnInlineImageThatOnlyExcludedLanguagesHold() + { + // An inline (Word-style) image lives once per editable in the translation group, + // including the hidden lang="z" prototype (see inlineImages.ts, + // syncInlineImagesFromEditable). Publishing one language deletes every other language's + // editable, and the image file itself is kept only while something in the DOM still + // refers to it -- CleanupUnusedImageFiles keeps what + // BookStorage.GetImagePathsRelativeToBook finds, which is ".//img", the wrapper's img + // included. So if every div holding a copy were removed, the file would be deleted from + // the published book and the remaining language would show a broken picture. + // + // What saves it is that "z" is in contentLanguages here, so the prototype's copy stays + // and keeps the reference alive. That is load-bearing and easy to break by tightening + // this method, which is why it is pinned here. + var html = """ + + + +
+
+
+
+
+

Ako si Robin

+
+
+
+

+
+
+
+
+ + + """; + var dom = new HtmlDom(html); + var assertThatDom = AssertThatXmlIn.Dom(dom.RawDom); + // Sanity check: both copies are there to start with, so the test can tell what the + // method took away. + assertThatDom.HasSpecifiedNumberOfMatchesForXpath("//img[@src='bird.png']", 2); + + // SUT: publish a language that no editable on this page holds. + PublishModel.RemoveUnwantedLanguageData( + dom, + new[] { "en" }, + false, + new HashSet() + ); + + // The "tl" editable goes, as it should. + assertThatDom.HasSpecifiedNumberOfMatchesForXpath( + "//div[@lang='tl' and contains(@class, 'bloom-editable')]", + 0 + ); + // But the picture is still referred to, so the file survives CleanupUnusedImageFiles. + assertThatDom.HasSpecifiedNumberOfMatchesForXpath( + "//div[@lang='z']//img[@src='bird.png']", + 1 + ); + } + [Test] public void RemoveUnwantedLanguageData_BloomPage_PreservesPageLabel() {