From 9e858ba49faf0d76477f4ea8870322dcd2640c23 Mon Sep 17 00:00:00 2001 From: Hatton Date: Fri, 30 Jan 2026 11:01:07 -0700 Subject: [PATCH] Fix BL-15558 preserve bloompub font simulation Also, give the user a chance to read notes like ones about font embedding. --- .../commonPublish/PublishProgressDialog.tsx | 28 ++-- src/BloomExe/FontProcessing/FontFileFinder.cs | 4 +- .../Publish/BloomPub/BloomPubMaker.cs | 3 +- src/BloomExe/Publish/Epub/EpubMaker.cs | 137 ++++++++++++++++-- .../Publish/Epub/ExportEpubTests.cs | 109 ++++++++++++++ 5 files changed, 254 insertions(+), 27 deletions(-) diff --git a/src/BloomBrowserUI/publish/commonPublish/PublishProgressDialog.tsx b/src/BloomBrowserUI/publish/commonPublish/PublishProgressDialog.tsx index 389c72642409..bc643557b116 100644 --- a/src/BloomBrowserUI/publish/commonPublish/PublishProgressDialog.tsx +++ b/src/BloomBrowserUI/publish/commonPublish/PublishProgressDialog.tsx @@ -33,6 +33,8 @@ export const PublishProgressDialog: React.FunctionComponent<{ const [accumulatedMessages, setAccumulatedMessages] = useState(""); const [errorEncountered, setErrorEncountered] = useState(false); + const [interestingMessageEncountered, setInterestingMessageEncountered] = + useState(false); const { setProgressState: setProgressStateProp, @@ -47,6 +49,7 @@ export const PublishProgressDialog: React.FunctionComponent<{ setAccumulatedMessages(""); setInstructionMessage(undefined); setErrorEncountered(false); + setInterestingMessageEncountered(false); }, [setProgressStateProp, setClosePendingProp]); //Note, originally this was just a function, closeIfNoError(). @@ -55,12 +58,8 @@ export const PublishProgressDialog: React.FunctionComponent<{ // update we notice that and see about closing. React.useEffect(() => { if (props.closePending) { - if (errorEncountered) { - setProgressStateProp(() => - errorEncountered - ? ProgressState.Done - : ProgressState.Closed, - ); + if (errorEncountered || interestingMessageEncountered) { + setProgressStateProp(ProgressState.Done); // Although we may be in state 'Done' and thus not actually closed yet, // we're no longer in the state that closePending is meant to handle, // where we've finished but don't yet have enough information to know @@ -75,11 +74,15 @@ export const PublishProgressDialog: React.FunctionComponent<{ } }, [ props.closePending, + errorEncountered, setProgressStateProp, setClosePendingProp, closeAndResetDialog, + interestingMessageEncountered, ]); + // Kick off the server-side task only after the websocket is ready, + // so we don't miss early progress messages. React.useEffect(() => { props.setProgressState(ProgressState.Working); // we need to be ready to listen to progress messages from the server, @@ -103,7 +106,12 @@ export const PublishProgressDialog: React.FunctionComponent<{ }, ); }); - }, [props.apiForStartingTask, props.generation]); // Every time the start API endpoint changes, we basically restart the component + }, [ + props.apiForStartingTask, + props.generation, + props.setProgressState, + props.webSocketClientContext, + ]); // Every time the start API endpoint changes, we basically restart the component useSubscribeToWebSocketForEvent( props.webSocketClientContext, @@ -122,10 +130,10 @@ export const PublishProgressDialog: React.FunctionComponent<{ case "Warning": setErrorEncountered(true); // deliberately fall through - case "Progress": - - // eslint-disable-next-line no-fallthrough case "Note": + setInterestingMessageEncountered(true); + // deliberately fall through + case "Progress": setAccumulatedMessages( (oldMessages) => oldMessages + html, ); diff --git a/src/BloomExe/FontProcessing/FontFileFinder.cs b/src/BloomExe/FontProcessing/FontFileFinder.cs index 83511ee42440..bbbd4e679eb3 100644 --- a/src/BloomExe/FontProcessing/FontFileFinder.cs +++ b/src/BloomExe/FontProcessing/FontFileFinder.cs @@ -123,10 +123,10 @@ public string GetFileForFont(string fontName, string fontStyle, string fontWeigh { if (!string.IsNullOrEmpty(group.BoldItalic)) return group.BoldItalic; - else if (!string.IsNullOrEmpty(group.Bold)) - return group.Bold; else if (!string.IsNullOrEmpty(group.Italic)) return group.Italic; + else if (!string.IsNullOrEmpty(group.Bold)) + return group.Bold; else if (!string.IsNullOrEmpty(group.Normal)) return group.Normal; } diff --git a/src/BloomExe/Publish/BloomPub/BloomPubMaker.cs b/src/BloomExe/Publish/BloomPub/BloomPubMaker.cs index e11168802f07..71f930241105 100644 --- a/src/BloomExe/Publish/BloomPub/BloomPubMaker.cs +++ b/src/BloomExe/Publish/BloomPub/BloomPubMaker.cs @@ -930,6 +930,7 @@ out HashSet badFonts } // Create the fonts.css file, which tells the browser where to find the fonts for those families. var sb = new StringBuilder(); + var normalFacesAdded = new HashSet(StringComparer.OrdinalIgnoreCase); foreach (var font in fontsWanted.OrderBy(x => x.ToString())) { if (badFonts.Contains(font.fontFamily)) @@ -937,7 +938,7 @@ out HashSet badFonts var group = fontFileFinder.GetGroupForFont(font.fontFamily); if (group != null) { - EpubMaker.AddFontFace(sb, font, group); + EpubMaker.AddFontFace(sb, font, group, normalFacesAdded); } // We don't need (or want) a rule to use Andika instead. // The reader typically WILL use Andika, because we have a rule making it the default font diff --git a/src/BloomExe/Publish/Epub/EpubMaker.cs b/src/BloomExe/Publish/Epub/EpubMaker.cs index 207b7923c055..3226b13c05e6 100644 --- a/src/BloomExe/Publish/Epub/EpubMaker.cs +++ b/src/BloomExe/Publish/Epub/EpubMaker.cs @@ -2791,6 +2791,7 @@ out HashSet badFonts } } var sb = new StringBuilder(); + var normalFacesAdded = new HashSet(StringComparer.OrdinalIgnoreCase); foreach (var font in _fontsUsedInBook.OrderBy(x => x.ToString())) { if (badFonts.Contains(font.fontFamily)) @@ -2816,7 +2817,14 @@ out HashSet badFonts // The fonts.css file is stored in a subfolder as are the font files. They are in different // subfolders, and the reference to the font file has to take the relative path to fonts.css // into account. - AddFontFace(sb, font, group, "../" + kFontsFolder + "/", true); + AddFontFace( + sb, + font, + group, + normalFacesAdded, + "../" + kFontsFolder + "/", + true + ); } } if ( @@ -2889,26 +2897,127 @@ internal static void AddFontFace( StringBuilder sb, PublishHelper.FontInfo font, FontGroup group, + HashSet normalFacesAdded, string relativePathFromCss = "", bool sanitizeFileName = false ) { - var weight = font.fontWeight == "700" ? "bold" : "normal"; - string path = null; - if (font.fontStyle == "italic" && font.fontWeight == "700") - path = group.BoldItalic; - if (string.IsNullOrEmpty(path) && font.fontStyle == "italic") - path = group.Italic; - if (string.IsNullOrEmpty(path) && font.fontWeight == "700") - path = group.Bold; - if (string.IsNullOrEmpty(path)) - path = group.Normal; + // What we are doing here (See BL-15558) + // On the one hand, we have the fonts that the html/css call for. + // On the other hand, we have the actual fonts (not typefaces, actual "fonts") + // that are available on this machine. When we "add a font face" here, we are + // creating a css rule that points to an actual font file that will be embedded. + // If the font face we want isn't available, the browser can synthesize one. + // So for example if you want italic, but we only have normal, that's fine we + // have no italic font face to add, so we just add the normal one and the browser will + // do its best. This is what is needed for WYSIWYG, because that is what is happening + // in the Editor view. But if you *do* have a font file matching the requested face, + // then we want to emit a font-face rule for it which points to the font file. + + var wantsItalic = font.fontStyle == "italic"; + var wantsBold = font.fontWeight == "700"; + string chosenPath = null; + string declaredWeight = null; + string declaredStyle = null; + + // Step 1: Find the best available face for the requested style/weight. + // For bold-italic requests, we prefer: BoldItalic > Italic > Bold > Normal + // This cascade means the browser only synthesizes what's missing (e.g., if we have + // Italic but not BoldItalic, the browser just synthesizes bold on top of real italic). + // For non-combined requests (just bold or just italic), we require an exact match + // or fall back to Normal in Step 3. + if (wantsItalic && wantsBold) + { + // For combined requests, emit the most specific real face we have and let the + // browser synthesize the missing axis. + if (!string.IsNullOrEmpty(group.BoldItalic)) + { + chosenPath = group.BoldItalic; + declaredWeight = "bold"; + declaredStyle = "italic"; + } + // the order of italic vs bold here is intentional, + // the thinking being that bold is easier to synthesize than italic, + // so if we have to pick just one, we prefer to provide the italic face + // and let the browser fake the bold. + else if (!string.IsNullOrEmpty(group.Italic)) + { + chosenPath = group.Italic; + declaredWeight = "normal"; + declaredStyle = "italic"; + } + else if (!string.IsNullOrEmpty(group.Bold)) + { + chosenPath = group.Bold; + declaredWeight = "bold"; + declaredStyle = "normal"; + } + else if (!string.IsNullOrEmpty(group.Normal)) + { + chosenPath = group.Normal; + declaredWeight = "normal"; + declaredStyle = "normal"; + } + } + else if (wantsItalic && !wantsBold && !string.IsNullOrEmpty(group.Italic)) + { + chosenPath = group.Italic; + declaredWeight = "normal"; + declaredStyle = "italic"; + } + else if (wantsBold && !wantsItalic && !string.IsNullOrEmpty(group.Bold)) + { + chosenPath = group.Bold; + declaredWeight = "bold"; + declaredStyle = "normal"; + } + else if (!wantsItalic && !wantsBold && !string.IsNullOrEmpty(group.Normal)) + { + chosenPath = group.Normal; + declaredWeight = "normal"; + declaredStyle = "normal"; + } + + if (!string.IsNullOrEmpty(chosenPath)) + { + // Step 2: only emit one normal/normal rule per family to keep output deterministic. + // Note: we could still duplicate non-normal faces if a bold-italic request falls back + // to italic/bold that is also directly requested. This is rare and harmless, so we + // do not add broader dedupe logic. + if (declaredStyle == "normal" && declaredWeight == "normal") + { + if (normalFacesAdded.Contains(font.fontFamily)) + return; + normalFacesAdded.Add(font.fontFamily); + } + AddFontFace( + sb, + font.fontFamily, + declaredWeight, + declaredStyle ?? font.fontStyle, + chosenPath, + relativePathFromCss, + sanitizeFileName + ); + return; + } + + // Step 3: no exact face. Don't fake a bold/italic face by pointing at some other file. + // Instead, declare just a normal/normal face so the reading browser can synthesize + // missing styles, matching what happens in Bloom's editor (See BL-15558). + if (string.IsNullOrEmpty(group.Normal)) + // If there's no Normal slot for this family, we avoid substituting bold/italic + // here; that would misrepresent the file and still not satisfy a normal request. + return; + if (normalFacesAdded.Contains(font.fontFamily)) + return; + normalFacesAdded.Add(font.fontFamily); AddFontFace( sb, font.fontFamily, - weight, - font.fontStyle, - path, + "normal", + "normal", + group.Normal, relativePathFromCss, sanitizeFileName ); diff --git a/src/BloomTests/Publish/Epub/ExportEpubTests.cs b/src/BloomTests/Publish/Epub/ExportEpubTests.cs index a2cfb5dba0cc..b1e24ecca1bd 100644 --- a/src/BloomTests/Publish/Epub/ExportEpubTests.cs +++ b/src/BloomTests/Publish/Epub/ExportEpubTests.cs @@ -8,6 +8,7 @@ using System.Xml.Linq; using Bloom; using Bloom.Book; +using Bloom.FontProcessing; using Bloom.Publish; using Bloom.Publish.Epub; using Bloom.SafeXml; @@ -2560,6 +2561,114 @@ public void AddFontFace_WithNullPath_AddsNothing() Assert.That(sb.Length, Is.EqualTo(0)); } + [Test] + public void AddFontFace_MissingItalicFace_DoesNotDeclareItalicStyle() + { + var sb = new StringBuilder(); + var group = new FontGroup { Normal = "Test Font R.ttf" }; + var normalFacesAdded = new HashSet(StringComparer.OrdinalIgnoreCase); + var font = new PublishHelper.FontInfo + { + fontFamily = "Test Font", + fontStyle = "italic", + fontWeight = "400", + }; + + EpubMaker.AddFontFace(sb, font, group, normalFacesAdded); + + var css = sb.ToString(); + Assert.That(css, Does.Contain("font-style:normal")); + Assert.That(css, Does.Not.Contain("font-style:italic")); + Assert.That(css, Does.Contain("Test Font R.ttf")); + } + + [Test] + public void AddFontFace_BoldItalicRequest_WithOnlyItalic_DeclaresItalicNormalFace() + { + var sb = new StringBuilder(); + var group = new FontGroup { Italic = "Test Font I.ttf" }; + var normalFacesAdded = new HashSet(StringComparer.OrdinalIgnoreCase); + var font = new PublishHelper.FontInfo + { + fontFamily = "Test Font", + fontStyle = "italic", + fontWeight = "700", + }; + + EpubMaker.AddFontFace(sb, font, group, normalFacesAdded); + + var css = sb.ToString(); + Assert.That(css, Does.Contain("font-style:italic")); + Assert.That(css, Does.Contain("font-weight:normal")); + Assert.That(css, Does.Contain("Test Font I.ttf")); + Assert.That(css, Does.Not.Contain("font-weight:bold")); + } + + [Test] + public void AddFontFace_BoldItalicRequest_WithOnlyBold_DeclaresBoldNormalStyleFace() + { + var sb = new StringBuilder(); + var group = new FontGroup { Bold = "Test Font B.ttf" }; + var normalFacesAdded = new HashSet(StringComparer.OrdinalIgnoreCase); + var font = new PublishHelper.FontInfo + { + fontFamily = "Test Font", + fontStyle = "italic", + fontWeight = "700", + }; + + EpubMaker.AddFontFace(sb, font, group, normalFacesAdded); + + var css = sb.ToString(); + Assert.That(css, Does.Contain("font-style:normal")); + Assert.That(css, Does.Contain("font-weight:bold")); + Assert.That(css, Does.Contain("Test Font B.ttf")); + Assert.That(css, Does.Not.Contain("font-style:italic")); + } + + [Test] + public void AddFontFace_BoldItalicRequest_WithItalicAndBold_PrefersItalicForSynthesis() + { + var sb = new StringBuilder(); + var group = new FontGroup { Bold = "Test Font B.ttf", Italic = "Test Font I.ttf" }; + var normalFacesAdded = new HashSet(StringComparer.OrdinalIgnoreCase); + var font = new PublishHelper.FontInfo + { + fontFamily = "Test Font", + fontStyle = "italic", + fontWeight = "700", + }; + + EpubMaker.AddFontFace(sb, font, group, normalFacesAdded); + + var css = sb.ToString(); + Assert.That(css, Does.Contain("font-style:italic")); + Assert.That(css, Does.Contain("font-weight:normal")); + Assert.That(css, Does.Contain("Test Font I.ttf")); + Assert.That(css, Does.Not.Contain("Test Font B.ttf")); + } + + [Test] + public void AddFontFace_BoldItalicRequest_WithBoldItalic_DeclaresBoldItalicFace() + { + var sb = new StringBuilder(); + var group = new FontGroup { BoldItalic = "Test Font BI.ttf" }; + var normalFacesAdded = new HashSet(StringComparer.OrdinalIgnoreCase); + var font = new PublishHelper.FontInfo + { + fontFamily = "Test Font", + fontStyle = "italic", + fontWeight = "700", + }; + + EpubMaker.AddFontFace(sb, font, group, normalFacesAdded); + + var css = sb.ToString(); + Assert.That(css, Does.Contain("font-style:italic")); + Assert.That(css, Does.Contain("font-weight:bold")); + Assert.That(css, Does.Contain("Test Font BI.ttf")); + } + [Test] public void MultilingualTitlesOrderedCorrectly() {