Skip to content

Keep the cover image's Opaque choice through the Edit tab and the thumbnail (BL-16819) #8339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
andrew-polk merged 1 commit into from
Sep 10, 2026
Merged

Keep the cover image's Opaque choice through the Edit tab and the thumbnail (BL-16819) #8339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/BloomExe/Book/BookData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2544,15 +2544,23 @@ internal bool UpdateImageFromDataSet(DataSet data, SafeXmlElement node, string k
/// from the image (so, for the transparency classes, Auto is restored too). Without this,
/// the user's choice would be lost every time the xmatter is regenerated from the template.
/// See BL-16819.
/// A null savedAttributes means the data set carries no attribute information for this
/// image at all, not that the image has no classes, so the image is left alone (as
/// MergeAttrsIntoElement does). This is the normal state of the member _dataset once a page
/// has been saved: UpdateSingleTextVariableInDataDiv recreates its entry with the new value
/// but without the attribute list, and UpdateDomFromDataset() pushes that data set to the
/// pages (e.g. from Book.SetMultilingualContentLanguages every time the Edit tab is entered).
/// </summary>
private static void RestoreImgClassesFromDataDiv(
SafeXmlElement img,
List<Tuple<string, XmlString>> savedAttributes
)
{
if (savedAttributes == null)
return;
var savedClasses =
savedAttributes
?.Find(a => a.Item1 == "class")
.Find(a => a.Item1 == "class")
?.Item2.Unencoded.Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries)
?? new string[0];
foreach (var className in _imgClassesToRestoreFromDataDiv)
Expand Down
51 changes: 44 additions & 7 deletions src/BloomExe/BookThumbNailer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Bloom.Publish;
using Bloom.SafeXml;
using SIL.IO;
using SIL.Reporting;
using SIL.Windows.Forms.ImageToolbox;

namespace Bloom
Expand Down Expand Up @@ -261,20 +262,56 @@ internal static bool CreateThumbnailOfCoverImage(
var destFilePath = Path.Combine(book.StoragePageFolder, options.FileName);
// Writing a transparent image to a file, then reading it in again appears to be the only
// way to get the thumbnail image to draw with the book's cover color background reliably.
// It is always a .png, whatever the cover image is: only a PNG can carry the alpha
// channel, and ImageUtils.MakeTransparentBackground needs a .png destination. The name
// is random rather than the cover image's own, so thumbnails being made at the same time
// (two books whose covers are both "cover.jpg", say) cannot overwrite or delete each
// other's file.
transparentImageFile = Path.Combine(
Path.GetTempPath(),
"Bloom",
"Transparent",
Path.GetFileName(imageSrc)
Path.GetRandomFileName() + ".png"
);
Directory.CreateDirectory(Path.GetDirectoryName(transparentImageFile));

if (
RuntimeImageProcessor.MakePngBackgroundTransparentIfDesirable(
imageSrc,
transparentImageFile
)
)
// Honor the user's Transparency choice for the cover image (the bloom-opaque and
// bloom-transparent classes; Auto is neither) the same way the browser does when it
// displays the cover: Opaque never gets a transparent background, Transparent always
// does, and Auto gets one if the image looks like line art. The thumbnail is always
// drawn on the cover color, so the page counts as one that needs transparent images.
// See BL-16819.
var transparencyMode =
coverImgElt == null
? ImageTransparencyMode.Auto
: HtmlDom.GetImageTransparencyMode(coverImgElt, pageNeedsTransparent: true);
// A problem image should still get a thumbnail, just without the transparency.
var madeTransparent = false;
try
{
switch (transparencyMode)
{
case ImageTransparencyMode.Force:
madeTransparent = ImageUtils.MakeTransparentBackground(
imageSrc,
transparentImageFile
);
break;
case ImageTransparencyMode.Auto:
madeTransparent = ImageUtils.MakeTransparentBackgroundIfNeeded(
imageSrc,
transparentImageFile
);
break;
}
}
catch (Exception e)
{
Logger.WriteEvent(
"Could not make the cover image transparent for the thumbnail: " + e.Message
);
}
if (madeTransparent)
imageSrc = transparentImageFile;
using (var coverImage = PalasoImage.FromFileRobustly(imageSrc))
{
Expand Down
31 changes: 23 additions & 8 deletions src/BloomExe/ImageProcessing/ImageUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3641,31 +3641,46 @@ SafeXmlElement imgElement
}
}

/// <summary>
/// Apply the transparency algorithm to a copy of the image at destinationPath (which must be
/// a .png path) if the image looks like line art on a white background; otherwise do nothing
/// and return false. Handles a JPEG source the same way the display pipeline does: the
/// line-art check accepts JPEGs, and the copy is re-saved as PNG so it can carry the alpha.
/// </summary>
internal static bool MakeTransparentBackgroundIfNeeded(
string sourcePath,
string destinationPath
)
{
using (var imageInfo = PalasoImage.FromFileRobustly(sourcePath))
{
if (ShouldMakeBackgroundTransparent(imageInfo))
{
RobustFile.Copy(sourcePath, destinationPath, true);
ApplyBloomTransparencyToFile(destinationPath);
return true;
}
if (!ShouldMakeBackgroundTransparent(imageInfo))
return false;
}
return false;
return MakeTransparentBackground(sourcePath, destinationPath);
}

/// <summary>
/// Like <see cref="MakeTransparentBackgroundIfNeeded"/> but always applies the
/// transparency algorithm, bypassing the line-art detection check.
/// Used when an image has the <c>bloom-transparent</c> class (<c>transparent=force</c>).
/// The destination must be a .png path: only a PNG can carry the alpha channel, and
/// ApplyBloomTransparencyToFile does nothing to any other extension. A source that is not
/// itself a PNG (a JPEG cover, say) is re-saved as PNG there rather than copied.
/// </summary>
internal static bool MakeTransparentBackground(string sourcePath, string destinationPath)
{
RobustFile.Copy(sourcePath, destinationPath, true);
if (sourcePath.EndsWith(".png", StringComparison.InvariantCultureIgnoreCase))
{
RobustFile.Copy(sourcePath, destinationPath, true);
}
else
{
using (var imageInfo = PalasoImage.FromFileRobustly(sourcePath))
{
RobustImageIO.SaveImage(imageInfo.Image, destinationPath, ImageFormat.Png);
}
}
ApplyBloomTransparencyToFile(destinationPath);
return true;
}
Expand Down
97 changes: 0 additions & 97 deletions src/BloomExe/ImageProcessing/RuntimeImageProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,102 +644,5 @@ private static ColorPalette GivePaletteTransparentBackground(Image bitmap)
}
return palette; // assigning this back to the bitmap will actually update it.
}

/// <summary>
/// Make the image background transparent if the image is PNG and appears to be a black
/// and white drawing.
/// </summary>
/// <returns>true if an image with transparent background is created</returns>
public static bool MakePngBackgroundTransparentIfDesirable(
string originalPath,
string pathToProcessedImage
)
{
try
{
//if it's a jpeg, we don't resize, we don't mess with transparency, nothing. These things
//are scary in .net. Just send the original back and wash our hands of it.
//If the filename extension claims to be jpeg, assume it's not lying to us and quit.
if (ImageUtils.HasJpegExtension(originalPath))
return false;

using (var originalImage = PalasoImage.FromFileRobustly(originalPath))
{
// double check whether the file extension was misleading us...
if (ImageUtils.AppearsToBeJpeg(originalImage))
return false;

// Check whether a transparent background is actually needed.
if (!ImageUtils.ShouldMakeBackgroundTransparent(originalImage))
return false;

using (var processedBitmap = MakePngBackgroundTransparent(originalImage))
{
//Hatton July 2012:
//Once or twice I saw a GDI+ error on the Save below, when the app 1st launched.
//I verified that if there is an IO error, that's what you get (a GDI+ error).
//I looked once, and the %temp%/Bloom directory wasn't there, so that's what I think caused the error.
//It's not clear why the temp/bloom directory isn't there... possibly it was there a moment ago
//but then some startup thread cleared and deleted it? (we are now running on a thread responding to the http request)

Exception error = null;
for (var i = 0; i < 3; i++) //try three times
{
try
{
error = null;
RobustImageIO.SaveImage(
processedBitmap,
pathToProcessedImage,
originalImage.Image.RawFormat
);
break;
}
catch (Exception e)
{
Logger.WriteEvent(
"***Error in RuntimeImageProcessor while trying to write image."
);
Logger.WriteEvent(e.Message);
error = e;
//in setting the sleep time, keep in mind that this may be one of 20 images
//so if the problem happens to all of them, then you're looking 20*retries*sleep-time,
//which will look like hung program.
//Meanwhile, this transparency thing is actually just a nice-to-have. If we give
//up, it's ok.
Thread.Sleep(100); //wait a 1/5 second before trying again
}
}

if (error != null)
{
throw error; //will be caught below
}
}
}

return true;
}
// We want to gracefully degrade if this fails (as it did once, see comment in BL-2871)
// We can't help users who respond to a toast and send in an error report.
// Logging it will allow us to possibly correlate an error here with another problem that does get reported.
catch (TagLib.CorruptFileException e)
{
// Don't bother reporting this to the user, but log it in case it might be relevant for a real bug.
Logger.WriteError(
$"Problem with image metadata: MakePngBackgroundTransparentIfDesirable({originalPath}) caught an exception.",
e
);
return false;
}
catch (Exception e)
{
Logger.WriteError(
$"Problem making image transparent: MakePngBackgroundTransparentIfDesirable({originalPath}) caught an exception.",
e
);
return false;
}
}
}
}
94 changes: 94 additions & 0 deletions src/BloomTests/Book/BookDataTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3407,6 +3407,100 @@ SafeXmlElement ReopenBookAndGetCoverImage()
}
}

/// <summary>
/// BL-16819: UpdateDomFromDataset() pushes the member data set, gathered when the BookData
/// was constructed, back to the pages. That must not disturb the cover image's Transparency
/// choice when the data set agrees with the page.
/// </summary>
[Test]
public void UpdateDomFromDataset_CoverImageOpaque_KeepsClass()
{
var dom = new HtmlDom(
@"<html ><head></head><body>
<div id='bloomDataDiv'>
<div data-book='coverImage' lang='*' src='aor.png' class=' bloom-imageLoadError bloom-opaque'>aor.png</div>
</div>
<div class='bloom-page'>
<div class='bloom-canvas'>
<img data-book='coverImage' src='aor.png' class='bloom-opaque'></img>
</div>
</div>
</body></html>"
);
var data = new BookData(dom, _collectionSettings, null);
data.UpdateDomFromDataset();
var pageImage = (SafeXmlElement)
dom.SelectSingleNodeHonoringDefaultNS(
"//div[@class='bloom-page']//img[@data-book='coverImage']"
);
Assert.That(
pageImage.HasClass("bloom-opaque"),
Is.True,
"class attribute is '" + pageImage.GetAttribute("class") + "'"
);
}

/// <summary>
/// BL-16819: after a page save, the member data set's entry for the cover image has been
/// recreated with the new value but without any attribute list. UpdateDomFromDataset() (which
/// Book.SetMultilingualContentLanguages runs every time the Edit tab is entered) must then
/// leave the image's Transparency choice alone rather than treating the missing attribute
/// list as "no classes" and stripping the choice the user just saved.
/// </summary>
[Test]
public void UpdateDomFromDataset_AfterSavingOpaqueChoice_KeepsClass()
{
var dom = new HtmlDom(
@"<html ><head></head><body>
<div id='bloomDataDiv'>
<div data-book='coverImage' lang='*' src='aor.png' class='bloom-imageLoadError'>aor.png</div>
</div>
<div class='bloom-page'>
<div class='bloom-canvas'>
<img data-book='coverImage' src='aor.png'></img>
</div>
</div>
</body></html>"
);
var data = new BookData(dom, _collectionSettings, null);
var editedPageDom = new HtmlDom(
@"<html ><head></head><body>
<div class='bloom-page'>
<div class='bloom-canvas'>
<img data-book='coverImage' src='new.png' class='bloom-opaque'></img>
</div>
</div>
</body></html>"
);
data.SuckInDataFromEditedDom(editedPageDom);
var pageImage = (SafeXmlElement)
dom.SelectSingleNodeHonoringDefaultNS(
"//div[@class='bloom-page']//img[@data-book='coverImage']"
);
Assert.That(
pageImage.HasClass("bloom-opaque"),
Is.True,
"sanity: save should put the class on the page image"
);
data.UpdateDomFromDataset();
pageImage = (SafeXmlElement)
dom.SelectSingleNodeHonoringDefaultNS(
"//div[@class='bloom-page']//img[@data-book='coverImage']"
);
Assert.That(
pageImage.GetAttribute("src"),
Is.EqualTo("new.png"),
"the saved image url should be kept"
);
Assert.That(
pageImage.HasClass("bloom-opaque"),
Is.True,
"the saved Opaque choice should be kept; class attribute is '"
+ pageImage.GetAttribute("class")
+ "'"
);
}

/// <summary>
/// BL-16819: the data-div is authoritative, so if it says the cover image is on Auto (no
/// transparency class), a stale override on the page image must be removed. And copying the
Expand Down
Loading