Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 18 additions & 10 deletions src/BloomBrowserUI/publish/commonPublish/PublishProgressDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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().
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
);
Expand Down
6 changes: 4 additions & 2 deletions src/BloomExe/FontProcessing/FontFileFinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,14 +119,16 @@ public string GetFileForFont(string fontName, string fontStyle, string fontWeigh
// The order of the tests is important, since we want to return the most
// specific match possible. Even if the user asked for Bold Italic, we
// might return Normal if that's all we have.
// For bold-italic requests, we prefer: BoldItalic > Italic > Bold > Normal
// This matches EpubMaker.AddFontFace to ensure CSS references align with embedded files.
if (fontStyle == "italic" && fontWeight == "700")
{
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;
}
Expand Down
3 changes: 2 additions & 1 deletion src/BloomExe/Publish/BloomPub/BloomPubMaker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -930,14 +930,15 @@ out HashSet<string> 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<string>(StringComparer.OrdinalIgnoreCase);
foreach (var font in fontsWanted.OrderBy(x => x.ToString()))
{
if (badFonts.Contains(font.fontFamily))
continue;
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
Expand Down
133 changes: 119 additions & 14 deletions src/BloomExe/Publish/Epub/EpubMaker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2791,6 +2791,7 @@ out HashSet<string> badFonts
}
}
var sb = new StringBuilder();
var normalFacesAdded = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
foreach (var font in _fontsUsedInBook.OrderBy(x => x.ToString()))
{
if (badFonts.Contains(font.fontFamily))
Expand All @@ -2816,7 +2817,14 @@ out HashSet<string> 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 (
Expand Down Expand Up @@ -2889,26 +2897,123 @@ internal static void AddFontFace(
StringBuilder sb,
PublishHelper.FontInfo font,
FontGroup group,
HashSet<string> 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";
}
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
);
Expand Down
109 changes: 109 additions & 0 deletions src/BloomTests/Publish/Epub/ExportEpubTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<string>(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<string>(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<string>(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<string>(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<string>(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()
{
Expand Down