From b652b983b8768991d5c0634d390af64e88caaec3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Nov 2025 20:29:27 +0000 Subject: [PATCH 1/2] Initial plan From a3c64a1edd1e484c92598d15ec35fc2c0479c991 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Nov 2025 20:43:12 +0000 Subject: [PATCH 2/2] Address review feedback on BL-15441 implementation - Remove duplicate SVG files from artwork directory - Add Enhance comment in BookThumbNailer.cs about unnecessary image processing - Refactor Book.cs comment and logic for better clarity on placeholder handling - Fix BloomServer.cs to send 404 response for placeholder requests - Update comment in BloomPubMaker.cs for clarity on placeholder handling - Update comment in CanvasElementManager.ts to include placeholder case - Mark CutImage string as obsolete in xlf file - Add translate="no" to Canvas xlf entry - Fix CSS to allow cropping non-placeholder background images - Fix bloomImages.ts to handle URLs with query parameters and rename parameter Co-authored-by: andrew-polk <5847219+andrew-polk@users.noreply.github.com> --- DistFiles/localization/en/Bloom.xlf | 5 +++-- artwork/canvas-placeholder.svg | 1 - artwork/placeholder.svg | 1 - src/BloomBrowserUI/bookEdit/css/editMode.less | 5 ++--- .../bookEdit/js/CanvasElementManager.ts | 2 +- src/BloomBrowserUI/bookEdit/js/bloomImages.ts | 11 ++++++++--- src/BloomExe/Book/Book.cs | 8 ++++---- src/BloomExe/BookThumbNailer.cs | 1 + src/BloomExe/Publish/BloomPub/BloomPubMaker.cs | 5 +++-- src/BloomExe/web/BloomServer.cs | 3 +++ 10 files changed, 25 insertions(+), 17 deletions(-) delete mode 100644 artwork/canvas-placeholder.svg delete mode 100644 artwork/placeholder.svg diff --git a/DistFiles/localization/en/Bloom.xlf b/DistFiles/localization/en/Bloom.xlf index cb5d22aec7d1..fead3e30289d 100644 --- a/DistFiles/localization/en/Bloom.xlf +++ b/DistFiles/localization/en/Bloom.xlf @@ -1361,7 +1361,7 @@ Image ID: EditTab.CustomPage.Image - + Canvas ID: EditTab.CustomPage.Canvas @@ -1877,9 +1877,10 @@ ID: EditTab.ImageMetadata.Corrupt.MoreInfo label of a button to click to display a website with more information - + Cut image ID: EditTab.Image.CutImage + Obsolete as of 6.3 Edit image credits, copyright, & license diff --git a/artwork/canvas-placeholder.svg b/artwork/canvas-placeholder.svg deleted file mode 100644 index 400bf7ba2055..000000000000 --- a/artwork/canvas-placeholder.svg +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file diff --git a/artwork/placeholder.svg b/artwork/placeholder.svg deleted file mode 100644 index c4a1e068aeeb..000000000000 --- a/artwork/placeholder.svg +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file diff --git a/src/BloomBrowserUI/bookEdit/css/editMode.less b/src/BloomBrowserUI/bookEdit/css/editMode.less index 2e10fe5db1c0..15afa0cf1ce8 100644 --- a/src/BloomBrowserUI/bookEdit/css/editMode.less +++ b/src/BloomBrowserUI/bookEdit/css/editMode.less @@ -1376,10 +1376,9 @@ canvas.moving { pointer-events: none; background-color: transparent; - // background images can't be manually resized + // background images can't be manually resized, but they can be cropped &.bloom-backgroundImage-control-frame { - .bloom-ui-canvas-element-resize-handle, - .bloom-ui-canvas-element-side-handle { + .bloom-ui-canvas-element-resize-handle { display: none; } } diff --git a/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts b/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts index ef68d125c631..40f51bdf2fba 100644 --- a/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts +++ b/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts @@ -2693,7 +2693,7 @@ export class CanvasElementManager { !useSizeOfNewImage && // not waiting for new dimensions imgOrVideo.classList.contains("bloom-imageLoadError")) // error occurred while trying to load ) { - // Image is in an error state; we probably won't ever get useful dimensions. Just leave + // Image is a placeholder or in an error state; we probably won't ever get useful dimensions. Just leave // the canvas element the shape it is. return; } diff --git a/src/BloomBrowserUI/bookEdit/js/bloomImages.ts b/src/BloomBrowserUI/bookEdit/js/bloomImages.ts index f6c05557aba2..0e23bd147f5a 100644 --- a/src/BloomBrowserUI/bookEdit/js/bloomImages.ts +++ b/src/BloomBrowserUI/bookEdit/js/bloomImages.ts @@ -36,12 +36,17 @@ export const kImageContainerSelector = `.${kImageContainerClass}`; // We don't use actual placeHolder.png files anymore, but we do continue to use // src="placeHolder.png" to mark placeholders export function isPlaceHolderImage( - filename: string | null | undefined, + url: string | null | undefined, ): boolean { - if (!filename) { + if (!url) { return false; } - return filename.toLowerCase().endsWith("placeholder.png"); + // Extract just the filename part to handle cases like "placeHolder.png?cachebust=12345" + // or paths like "book/placeHolder.png" + const urlLower = url.toLowerCase(); + const questionMarkIndex = urlLower.indexOf("?"); + const pathPart = questionMarkIndex >= 0 ? urlLower.substring(0, questionMarkIndex) : urlLower; + return pathPart.endsWith("placeholder.png"); } export function cleanupImages() { diff --git a/src/BloomExe/Book/Book.cs b/src/BloomExe/Book/Book.cs index ae1762e3764c..90528da0dac6 100644 --- a/src/BloomExe/Book/Book.cs +++ b/src/BloomExe/Book/Book.cs @@ -5190,10 +5190,10 @@ ref coverImageFileName ); // We no longer put placeHolder.png files in books (BL-15441) but we still need to detect when the placeholder // is called for, so here we return placeHolder.png instead of null. Callers of this method should handle this special case. - if ( - !ImageUtils.IsPlaceholderImageFilename(coverImagePath) - && !RobustFile.Exists(coverImagePath) - ) + if (ImageUtils.IsPlaceholderImageFilename(coverImagePath)) + return coverImagePath; + + if (!RobustFile.Exists(coverImagePath)) { // And the filename might be multiply-HTML encoded. while (coverImagePath.Contains("&")) diff --git a/src/BloomExe/BookThumbNailer.cs b/src/BloomExe/BookThumbNailer.cs index 6e2f81650984..4d6a9eff3c1e 100644 --- a/src/BloomExe/BookThumbNailer.cs +++ b/src/BloomExe/BookThumbNailer.cs @@ -243,6 +243,7 @@ internal static bool CreateThumbnailOfCoverImage( BloomFileLocator.BrowserRoot ); imageSrc = Path.Combine(bloomRoot, "images", "placeHolder.png"); + // Enhance: We are unnecessarily running some image checks/processing below even in the placeholder case. } if (!IsCoverImageSrcValid(imageSrc, options)) { diff --git a/src/BloomExe/Publish/BloomPub/BloomPubMaker.cs b/src/BloomExe/Publish/BloomPub/BloomPubMaker.cs index 1dc42f358377..30c738ee5cc5 100644 --- a/src/BloomExe/Publish/BloomPub/BloomPubMaker.cs +++ b/src/BloomExe/Publish/BloomPub/BloomPubMaker.cs @@ -727,8 +727,9 @@ private static void StripImgIfWeCannotFindFile(SafeXmlDocument dom, string bookF var imgElt in dom.SafeSelectNodes("//img[@src]").Cast().ToArray() ) { - // As of BL-15441, we don't use real files for place holders but we still mark them with src="placeHolder.png". - // Don't strip these - we want them to persist in template books, and for other books we are already removing them elsewhere. + // As of BL-15441, we don't use real files for place holders (except in templates) but we still mark them with src="placeHolder.png". + // Don't strip them here -- we want them to persist in template books. For other books we are already removing them + // elsewhere (see line 540 where we delete placeHolder.png from the book folder). string src = imgElt.GetAttribute("src"); if (ImageUtils.IsPlaceholderImageFilename(src)) continue; diff --git a/src/BloomExe/web/BloomServer.cs b/src/BloomExe/web/BloomServer.cs index ba41477783fc..b35500689f53 100644 --- a/src/BloomExe/web/BloomServer.cs +++ b/src/BloomExe/web/BloomServer.cs @@ -748,6 +748,9 @@ private bool ProcessImageFileRequest(IRequestInfo info) } else if (ImageUtils.IsPlaceholderImageFilename(imageFile)) { + // BL-15441: We no longer use real files for placeholders, but the HTML still references placeHolder.png. + // The placeholder is displayed via CSS. Return a 404 so the browser knows there's no file. + info.WriteError(404); return true; } // If the user does add a video or widget, these placeholder .svgs will get copied to the