Skip to content

Commit cdaecb5

Browse files
hattonhatton
committed
Fix BL-15558 preserve bloompub font simulation
Also, give the user a chance to read notes like ones about font embedding. Review: JohnThomson - progressKind switch cleanup -> clearer fall-through Review: andrew-polk - AddFontFace step number -> rename to Step 3 3rd
1 parent 8a9d127 commit cdaecb5

4 files changed

Lines changed: 248 additions & 25 deletions

File tree

‎src/BloomBrowserUI/publish/commonPublish/PublishProgressDialog.tsx‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ export const PublishProgressDialog: React.FunctionComponent<{
3333
const [accumulatedMessages, setAccumulatedMessages] = useState("");
3434

3535
const [errorEncountered, setErrorEncountered] = useState(false);
36+
const [interestingMessageEncountered, setInterestingMessageEncountered] =
37+
useState(false);
3638

3739
const {
3840
setProgressState: setProgressStateProp,
@@ -47,6 +49,7 @@ export const PublishProgressDialog: React.FunctionComponent<{
4749
setAccumulatedMessages("");
4850
setInstructionMessage(undefined);
4951
setErrorEncountered(false);
52+
setInterestingMessageEncountered(false);
5053
}, [setProgressStateProp, setClosePendingProp]);
5154

5255
//Note, originally this was just a function, closeIfNoError().
@@ -55,12 +58,8 @@ export const PublishProgressDialog: React.FunctionComponent<{
5558
// update we notice that and see about closing.
5659
React.useEffect(() => {
5760
if (props.closePending) {
58-
if (errorEncountered) {
59-
setProgressStateProp(() =>
60-
errorEncountered
61-
? ProgressState.Done
62-
: ProgressState.Closed,
63-
);
61+
if (errorEncountered || interestingMessageEncountered) {
62+
setProgressStateProp(ProgressState.Done);
6463
// Although we may be in state 'Done' and thus not actually closed yet,
6564
// we're no longer in the state that closePending is meant to handle,
6665
// where we've finished but don't yet have enough information to know
@@ -75,11 +74,15 @@ export const PublishProgressDialog: React.FunctionComponent<{
7574
}
7675
}, [
7776
props.closePending,
77+
errorEncountered,
7878
setProgressStateProp,
7979
setClosePendingProp,
8080
closeAndResetDialog,
81+
interestingMessageEncountered,
8182
]);
8283

84+
// Kick off the server-side task only after the websocket is ready,
85+
// so we don't miss early progress messages.
8386
React.useEffect(() => {
8487
props.setProgressState(ProgressState.Working);
8588
// we need to be ready to listen to progress messages from the server,
@@ -103,7 +106,12 @@ export const PublishProgressDialog: React.FunctionComponent<{
103106
},
104107
);
105108
});
106-
}, [props.apiForStartingTask, props.generation]); // Every time the start API endpoint changes, we basically restart the component
109+
}, [
110+
props.apiForStartingTask,
111+
props.generation,
112+
props.setProgressState,
113+
props.webSocketClientContext,
114+
]); // Every time the start API endpoint changes, we basically restart the component
107115

108116
useSubscribeToWebSocketForEvent(
109117
props.webSocketClientContext,
@@ -122,10 +130,10 @@ export const PublishProgressDialog: React.FunctionComponent<{
122130
case "Warning":
123131
setErrorEncountered(true);
124132
// deliberately fall through
125-
case "Progress":
126-
127-
// eslint-disable-next-line no-fallthrough
128133
case "Note":
134+
setInterestingMessageEncountered(true);
135+
// deliberately fall through
136+
case "Progress":
129137
setAccumulatedMessages(
130138
(oldMessages) => oldMessages + html,
131139
);

‎src/BloomExe/Publish/BloomPub/BloomPubMaker.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -930,14 +930,15 @@ out HashSet<string> badFonts
930930
}
931931
// Create the fonts.css file, which tells the browser where to find the fonts for those families.
932932
var sb = new StringBuilder();
933+
var normalFacesAdded = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
933934
foreach (var font in fontsWanted.OrderBy(x => x.ToString()))
934935
{
935936
if (badFonts.Contains(font.fontFamily))
936937
continue;
937938
var group = fontFileFinder.GetGroupForFont(font.fontFamily);
938939
if (group != null)
939940
{
940-
EpubMaker.AddFontFace(sb, font, group);
941+
EpubMaker.AddFontFace(sb, font, group, normalFacesAdded);
941942
}
942943
// We don't need (or want) a rule to use Andika instead.
943944
// The reader typically WILL use Andika, because we have a rule making it the default font

‎src/BloomExe/Publish/Epub/EpubMaker.cs‎

Lines changed: 119 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2791,6 +2791,7 @@ out HashSet<string> badFonts
27912791
}
27922792
}
27932793
var sb = new StringBuilder();
2794+
var normalFacesAdded = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
27942795
foreach (var font in _fontsUsedInBook.OrderBy(x => x.ToString()))
27952796
{
27962797
if (badFonts.Contains(font.fontFamily))
@@ -2816,7 +2817,14 @@ out HashSet<string> badFonts
28162817
// The fonts.css file is stored in a subfolder as are the font files. They are in different
28172818
// subfolders, and the reference to the font file has to take the relative path to fonts.css
28182819
// into account.
2819-
AddFontFace(sb, font, group, "../" + kFontsFolder + "/", true);
2820+
AddFontFace(
2821+
sb,
2822+
font,
2823+
group,
2824+
normalFacesAdded,
2825+
"../" + kFontsFolder + "/",
2826+
true
2827+
);
28202828
}
28212829
}
28222830
if (
@@ -2889,26 +2897,123 @@ internal static void AddFontFace(
28892897
StringBuilder sb,
28902898
PublishHelper.FontInfo font,
28912899
FontGroup group,
2900+
HashSet<string> normalFacesAdded,
28922901
string relativePathFromCss = "",
28932902
bool sanitizeFileName = false
28942903
)
28952904
{
2896-
var weight = font.fontWeight == "700" ? "bold" : "normal";
2897-
string path = null;
2898-
if (font.fontStyle == "italic" && font.fontWeight == "700")
2899-
path = group.BoldItalic;
2900-
if (string.IsNullOrEmpty(path) && font.fontStyle == "italic")
2901-
path = group.Italic;
2902-
if (string.IsNullOrEmpty(path) && font.fontWeight == "700")
2903-
path = group.Bold;
2904-
if (string.IsNullOrEmpty(path))
2905-
path = group.Normal;
2905+
// What we are doing here (See BL-15558)
2906+
// On the one hand, we have the fonts that the html/css call for.
2907+
// On the other hand, we have the actual fonts (not typefaces, actual "fonts")
2908+
// that are available on this machine. When we "add a font face" here, we are
2909+
// creating a css rule that points to an actual font file that will be embedded.
2910+
// If the font face we want isn't available, the browser can synthesize one.
2911+
// So for example if you want italic, but we only have normal, that's fine we
2912+
// have no italic font face to add, so we just add the normal one and the browser will
2913+
// do its best. This is what is needed for WYSIWYG, because that is what is happening
2914+
// in the Editor view. But if you *do* have a font file matching the requested face,
2915+
// then we want to emit a font-face rule for it which points to the font file.
2916+
2917+
var wantsItalic = font.fontStyle == "italic";
2918+
var wantsBold = font.fontWeight == "700";
2919+
string chosenPath = null;
2920+
string declaredWeight = null;
2921+
string declaredStyle = null;
2922+
2923+
// Step 1: Find the best available face for the requested style/weight.
2924+
// For bold-italic requests, we prefer: BoldItalic > Italic > Bold > Normal
2925+
// This cascade means the browser only synthesizes what's missing (e.g., if we have
2926+
// Italic but not BoldItalic, the browser just synthesizes bold on top of real italic).
2927+
// For non-combined requests (just bold or just italic), we require an exact match
2928+
// or fall back to Normal in Step 3.
2929+
if (wantsItalic && wantsBold)
2930+
{
2931+
// For combined requests, emit the most specific real face we have and let the
2932+
// browser synthesize the missing axis.
2933+
if (!string.IsNullOrEmpty(group.BoldItalic))
2934+
{
2935+
chosenPath = group.BoldItalic;
2936+
declaredWeight = "bold";
2937+
declaredStyle = "italic";
2938+
}
2939+
else if (!string.IsNullOrEmpty(group.Italic))
2940+
{
2941+
chosenPath = group.Italic;
2942+
declaredWeight = "normal";
2943+
declaredStyle = "italic";
2944+
}
2945+
else if (!string.IsNullOrEmpty(group.Bold))
2946+
{
2947+
chosenPath = group.Bold;
2948+
declaredWeight = "bold";
2949+
declaredStyle = "normal";
2950+
}
2951+
else if (!string.IsNullOrEmpty(group.Normal))
2952+
{
2953+
chosenPath = group.Normal;
2954+
declaredWeight = "normal";
2955+
declaredStyle = "normal";
2956+
}
2957+
}
2958+
else if (wantsItalic && !wantsBold && !string.IsNullOrEmpty(group.Italic))
2959+
{
2960+
chosenPath = group.Italic;
2961+
declaredWeight = "normal";
2962+
declaredStyle = "italic";
2963+
}
2964+
else if (wantsBold && !wantsItalic && !string.IsNullOrEmpty(group.Bold))
2965+
{
2966+
chosenPath = group.Bold;
2967+
declaredWeight = "bold";
2968+
declaredStyle = "normal";
2969+
}
2970+
else if (!wantsItalic && !wantsBold && !string.IsNullOrEmpty(group.Normal))
2971+
{
2972+
chosenPath = group.Normal;
2973+
declaredWeight = "normal";
2974+
declaredStyle = "normal";
2975+
}
2976+
2977+
if (!string.IsNullOrEmpty(chosenPath))
2978+
{
2979+
// Step 2: only emit one normal/normal rule per family to keep output deterministic.
2980+
// Note: we could still duplicate non-normal faces if a bold-italic request falls back
2981+
// to italic/bold that is also directly requested. This is rare and harmless, so we
2982+
// do not add broader dedupe logic.
2983+
if (declaredStyle == "normal" && declaredWeight == "normal")
2984+
{
2985+
if (normalFacesAdded.Contains(font.fontFamily))
2986+
return;
2987+
normalFacesAdded.Add(font.fontFamily);
2988+
}
2989+
AddFontFace(
2990+
sb,
2991+
font.fontFamily,
2992+
declaredWeight,
2993+
declaredStyle ?? font.fontStyle,
2994+
chosenPath,
2995+
relativePathFromCss,
2996+
sanitizeFileName
2997+
);
2998+
return;
2999+
}
3000+
3001+
// Step 3: no exact face. Don't fake a bold/italic face by pointing at some other file.
3002+
// Instead, declare just a normal/normal face so the reading browser can synthesize
3003+
// missing styles, matching what happens in Bloom's editor (See BL-15558).
3004+
if (string.IsNullOrEmpty(group.Normal))
3005+
// If there's no Normal slot for this family, we avoid substituting bold/italic
3006+
// here; that would misrepresent the file and still not satisfy a normal request.
3007+
return;
3008+
if (normalFacesAdded.Contains(font.fontFamily))
3009+
return;
3010+
normalFacesAdded.Add(font.fontFamily);
29063011
AddFontFace(
29073012
sb,
29083013
font.fontFamily,
2909-
weight,
2910-
font.fontStyle,
2911-
path,
3014+
"normal",
3015+
"normal",
3016+
group.Normal,
29123017
relativePathFromCss,
29133018
sanitizeFileName
29143019
);

‎src/BloomTests/Publish/Epub/ExportEpubTests.cs‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Xml.Linq;
99
using Bloom;
1010
using Bloom.Book;
11+
using Bloom.FontProcessing;
1112
using Bloom.Publish;
1213
using Bloom.Publish.Epub;
1314
using Bloom.SafeXml;
@@ -2560,6 +2561,114 @@ public void AddFontFace_WithNullPath_AddsNothing()
25602561
Assert.That(sb.Length, Is.EqualTo(0));
25612562
}
25622563

2564+
[Test]
2565+
public void AddFontFace_MissingItalicFace_DoesNotDeclareItalicStyle()
2566+
{
2567+
var sb = new StringBuilder();
2568+
var group = new FontGroup { Normal = "Test Font R.ttf" };
2569+
var normalFacesAdded = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
2570+
var font = new PublishHelper.FontInfo
2571+
{
2572+
fontFamily = "Test Font",
2573+
fontStyle = "italic",
2574+
fontWeight = "400",
2575+
};
2576+
2577+
EpubMaker.AddFontFace(sb, font, group, normalFacesAdded);
2578+
2579+
var css = sb.ToString();
2580+
Assert.That(css, Does.Contain("font-style:normal"));
2581+
Assert.That(css, Does.Not.Contain("font-style:italic"));
2582+
Assert.That(css, Does.Contain("Test Font R.ttf"));
2583+
}
2584+
2585+
[Test]
2586+
public void AddFontFace_BoldItalicRequest_WithOnlyItalic_DeclaresItalicNormalFace()
2587+
{
2588+
var sb = new StringBuilder();
2589+
var group = new FontGroup { Italic = "Test Font I.ttf" };
2590+
var normalFacesAdded = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
2591+
var font = new PublishHelper.FontInfo
2592+
{
2593+
fontFamily = "Test Font",
2594+
fontStyle = "italic",
2595+
fontWeight = "700",
2596+
};
2597+
2598+
EpubMaker.AddFontFace(sb, font, group, normalFacesAdded);
2599+
2600+
var css = sb.ToString();
2601+
Assert.That(css, Does.Contain("font-style:italic"));
2602+
Assert.That(css, Does.Contain("font-weight:normal"));
2603+
Assert.That(css, Does.Contain("Test Font I.ttf"));
2604+
Assert.That(css, Does.Not.Contain("font-weight:bold"));
2605+
}
2606+
2607+
[Test]
2608+
public void AddFontFace_BoldItalicRequest_WithOnlyBold_DeclaresBoldNormalStyleFace()
2609+
{
2610+
var sb = new StringBuilder();
2611+
var group = new FontGroup { Bold = "Test Font B.ttf" };
2612+
var normalFacesAdded = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
2613+
var font = new PublishHelper.FontInfo
2614+
{
2615+
fontFamily = "Test Font",
2616+
fontStyle = "italic",
2617+
fontWeight = "700",
2618+
};
2619+
2620+
EpubMaker.AddFontFace(sb, font, group, normalFacesAdded);
2621+
2622+
var css = sb.ToString();
2623+
Assert.That(css, Does.Contain("font-style:normal"));
2624+
Assert.That(css, Does.Contain("font-weight:bold"));
2625+
Assert.That(css, Does.Contain("Test Font B.ttf"));
2626+
Assert.That(css, Does.Not.Contain("font-style:italic"));
2627+
}
2628+
2629+
[Test]
2630+
public void AddFontFace_BoldItalicRequest_WithItalicAndBold_PrefersItalicForSynthesis()
2631+
{
2632+
var sb = new StringBuilder();
2633+
var group = new FontGroup { Bold = "Test Font B.ttf", Italic = "Test Font I.ttf" };
2634+
var normalFacesAdded = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
2635+
var font = new PublishHelper.FontInfo
2636+
{
2637+
fontFamily = "Test Font",
2638+
fontStyle = "italic",
2639+
fontWeight = "700",
2640+
};
2641+
2642+
EpubMaker.AddFontFace(sb, font, group, normalFacesAdded);
2643+
2644+
var css = sb.ToString();
2645+
Assert.That(css, Does.Contain("font-style:italic"));
2646+
Assert.That(css, Does.Contain("font-weight:normal"));
2647+
Assert.That(css, Does.Contain("Test Font I.ttf"));
2648+
Assert.That(css, Does.Not.Contain("Test Font B.ttf"));
2649+
}
2650+
2651+
[Test]
2652+
public void AddFontFace_BoldItalicRequest_WithBoldItalic_DeclaresBoldItalicFace()
2653+
{
2654+
var sb = new StringBuilder();
2655+
var group = new FontGroup { BoldItalic = "Test Font BI.ttf" };
2656+
var normalFacesAdded = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
2657+
var font = new PublishHelper.FontInfo
2658+
{
2659+
fontFamily = "Test Font",
2660+
fontStyle = "italic",
2661+
fontWeight = "700",
2662+
};
2663+
2664+
EpubMaker.AddFontFace(sb, font, group, normalFacesAdded);
2665+
2666+
var css = sb.ToString();
2667+
Assert.That(css, Does.Contain("font-style:italic"));
2668+
Assert.That(css, Does.Contain("font-weight:bold"));
2669+
Assert.That(css, Does.Contain("Test Font BI.ttf"));
2670+
}
2671+
25632672
[Test]
25642673
public void MultilingualTitlesOrderedCorrectly()
25652674
{

0 commit comments

Comments
 (0)