Skip to content
Open
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
12 changes: 12 additions & 0 deletions src/BloomExe/Spreadsheet/SpreadsheetExporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Linq;
using System.Security;
using System.Text;
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using System.Windows.Forms;
using Bloom.Api;
Expand Down Expand Up @@ -409,6 +410,17 @@ var editable in translationGroup
"<span class=\"bloom-audio-split-marker\">\u200B</span>",
"|"
);
// Replace <br> line breaks with a space rather than removing them outright, so
// that words on either side of a soft line break (e.g. Shift+Enter in a paragraph)
// are not run together in the exported text.
content = content.Replace("<br>", " ");
content = content.Replace("<br />", " ");
Comment thread
StephenMcConnel marked this conversation as resolved.
// Collapse runs of ordinary (breaking) whitespace to a single space. We deliberately
// do NOT use \s here: in .NET, \s also matches non-breaking spaces (U+00A0, U+202F,
// etc.), which are meaningful (e.g. French punctuation spacing, keeping words
// together) and must be preserved in the export.
content = Regex.Replace(content, "[ \t\r\n\f\v]+", " ");
content = content.Trim(); // remove leading and trailing whitespace, which is not meaningful.
// Don't just test content, it typically contains paragraph markup.
if (String.IsNullOrWhiteSpace(editable.InnerText))
{
Expand Down
131 changes: 131 additions & 0 deletions src/BloomTests/Spreadsheet/SpreadsheetAudioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -729,4 +729,135 @@ public void ParseXmlConvertsSegmentMarkers()
Assert.That(parsed.GetRun(2).Text, Is.EqualTo("in the middle of this sentences"));
}
}

/// <summary>
/// Regression tests for BL-16518. The export-time content normalization (trim, replace
/// &lt;br&gt; with a space, collapse runs of ordinary whitespace) must not fuse two
/// sentences that are separated only by a line break: the audio splitter relies on there
/// being whitespace between sentences, so if the boundary space were lost the re-imported
/// text would fragment into fewer sentences than there are recorded audio segments,
/// desynchronizing (or dropping) the audio. These verify the boundary survives export in
/// both split-TextBox and Sentence recording modes, with the segment/endtime mapping intact.
/// </summary>
public class Spreadsheet_AudioWhitespaceNormalization_Tests : SpreadsheetAudioTestsBase
{
// A split-TextBox editable whose two timed sentences are separated by a <br> and a run
// of spaces (as can happen from a Shift+Enter in the editor). id matches a real test mp3.
private const string splitTextBoxPage =
@" <div class=""bloom-page numberedPage customPage A5Portrait side-right bloom-monolingual"" data-page="""" id=""page-split"" data-page-number=""1"" lang="""">
<div class=""pageLabel"" lang=""en"">Basic Text</div>
<div class=""pageDescription"" lang=""en""></div>
<div class=""marginBox"">
<div class=""bloom-translationGroup bloom-trailingElement"" data-default-languages=""auto"">
<div class=""bloom-editable normal-style audio-sentence bloom-postAudioSplit bloom-content1 bloom-visibility-code-on"" lang=""fr"" contenteditable=""true"" data-audiorecordingmode=""TextBox"" id=""a9d7b794-7a83-473a-8307-7968176ae4bc"" recordingmd5=""c93312969c38f815f4c9057f94bec2ab"" data-duration=""4.388571"" data-audiorecordingendtimes=""1.500 4.388571"">
<p><span id=""seg1"" class=""bloom-highlightSegment"">First sentence.</span><br /> <span id=""seg2"" class=""bloom-highlightSegment"">Second sentence.</span></p>
</div>
<div class=""bloom-editable normal-style"" lang=""z"" contenteditable=""true""><p></p></div>
</div>
</div>
</div>";

// A sentence-mode editable whose two per-sentence recordings are separated by a <br> and
// a run of spaces. ids match real test mp3s.
private const string sentenceModePage =
@" <div class=""bloom-page numberedPage customPage A5Portrait side-right bloom-monolingual"" data-page="""" id=""page-sentence"" data-page-number=""2"" lang="""">
<div class=""pageLabel"" lang=""en"">Basic Text</div>
<div class=""pageDescription"" lang=""en""></div>
<div class=""marginBox"">
<div class=""bloom-translationGroup bloom-trailingElement"" data-default-languages=""auto"">
<div class=""bloom-editable normal-style bloom-content1 bloom-visibility-code-on"" lang=""fr"" contenteditable=""true"" data-audiorecordingmode=""Sentence"" id=""sentence-editable"">
<p><span id=""i991ae1ac-db9a-4ad1-984b-8e679c1ae901"" class=""audio-sentence"" recordingmd5=""aaa"" data-duration=""2.899592"">First sentence.</span><br /> <span id=""i77c18c83-0224-405f-bb97-70d32078855c"" class=""audio-sentence"" recordingmd5=""bbb"" data-duration=""2.194286"">Second sentence.</span></p>
</div>
<div class=""bloom-editable normal-style"" lang=""z"" contenteditable=""true""><p></p></div>
</div>
</div>
</div>";

[OneTimeSetUp]
public void OneTimeSetUp()
{
// Sanity checks on the test data: a false pass would be easy if the input didn't
// actually contain the <br> and multiple spaces the normalization is meant to handle.
Assert.That(
splitTextBoxPage,
Does.Contain("<br />"),
"test setup: split page should contain a line break between the two segments"
);
Assert.That(
splitTextBoxPage,
Does.Contain(" <span"),
"test setup: split page should contain a run of spaces before the second segment"
);
Assert.That(
sentenceModePage,
Does.Contain("<br />"),
"test setup: sentence page should contain a line break between the two sentences"
);

Setup(splitTextBoxPage + sentenceModePage);

// Sanity check that both content rows were produced in the expected order.
Assert.That(
PageContentRows.Count,
Is.EqualTo(2),
"expected one content row per page (split TextBox, then Sentence mode)"
);
}

[OneTimeTearDown]
public void OneTimeTearDown()
{
TearDown();
}

private int FrIndex => AllRows.First().CellContents.IndexOf("[fr]");

[Test]
public void SplitTextBox_LineBreakBetweenSentences_ExportsAsSpaceSeparatedSentences()
{
var cell = PageContentRows[0].GetCell(FrIndex);
// The <br> and the run of spaces collapse to a single space, so the two sentences
// stay separated (not fused) in the exported text.
Assert.That(cell.Text, Is.EqualTo("First sentence. Second sentence."));
Assert.That(
cell.Content,
Does.Not.Contain("sentence.</span><span"),
"the two segments must not be fused with no whitespace between them"
);
}

[Test]
public void SplitTextBox_ExportsBothEndTimes()
{
var alignmentColIndex = AllRows.First().CellContents.IndexOf("[audio alignments fr]");
var alignment = PageContentRows[0].GetCell(alignmentColIndex).Content;
// Two end-times, matching the two sentences that survive normalization. If the
// sentences had been fused, the re-imported single sentence would mismatch these.
Assert.That(alignment, Is.EqualTo("1.500 4.388571"));
Assert.That(
alignment.Split(' ').Length,
Is.EqualTo(2),
"there should still be two audio segments"
);
}

[Test]
public void SentenceMode_LineBreakBetweenSentences_ExportsAsSpaceSeparatedSentences()
{
var cell = PageContentRows[1].GetCell(FrIndex);
Assert.That(cell.Text, Is.EqualTo("First sentence. Second sentence."));
}

[Test]
public void SentenceMode_ExportsBothAudioFilesInOrder()
{
var audioColIndex = AllRows.First().CellContents.IndexOf("[audio fr]");
Assert.That(
PageContentRows[1].GetCell(audioColIndex).Content,
Is.EqualTo(
"./audio/i991ae1ac-db9a-4ad1-984b-8e679c1ae901.mp3, ./audio/i77c18c83-0224-405f-bb97-70d32078855c.mp3"
)
);
}
}
}
93 changes: 91 additions & 2 deletions src/BloomTests/Spreadsheet/SpreadsheetExporterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,13 @@ public void ExportsTextOnContentPageInAllLanguages()
Assert.That(
_pageContentRows[0].GetCell("[en]").Text,
Is.EqualTo(
"Is this really a cat? The nose looks too long for it to be a cat. But perching on a limb doesn't look like a dog either. So what is it? I can't really tell from first glance!"
"Is this really a cat? The nose looks too long for it to be a cat. But perching on a limb doesn't look like a dog either. So what is it? I can't really tell from first glance!"
)
);
Assert.That(
_pageContentRows[0].GetCell("[es-x-ai-google]").Text,
Is.EqualTo(
"¿Es esto realmente un gato? La nariz parece demasiado larga para ser un gato. Pero posado sobre una rama tampoco parece un perro. Entonces, ¿qué es? ¡Realmente no puedo decirlo a primera vista!"
"¿Es esto realmente un gato? La nariz parece demasiado larga para ser un gato. Pero posado sobre una rama tampoco parece un perro. Entonces, ¿qué es? ¡Realmente no puedo decirlo a primera vista!"
)
);
}
Expand Down Expand Up @@ -166,5 +166,94 @@ public void ExportsDataDivTextInAllLanguages()
Is.EqualTo("Esto es del interior de la contraportada del libro.")
);
}

[Test]
public void ExportNormalizesContent_SpacesOutLineBreaks_CollapsesWhitespace_PreservesNbsp()
{
// Input exercises all three normalization behaviors at once: a soft line break
// between two words, a run of ordinary spaces, and a non-breaking space (U+00A0).
const string editableInnerXml = "<p>Some words<br />joined\u00A0text here</p>";

// Sanity checks on the test data so a green result can't be a false pass.
Assert.That(
editableInnerXml,
Does.Contain("<br />"),
"test setup: input should contain a line break"
);
Assert.That(
editableInnerXml,
Does.Contain("\u00A0"),
"test setup: input should contain a non-breaking space"
);
Assert.That(
editableInnerXml.Contains("Some words"),
Is.True,
"test setup: input should contain a run of ordinary spaces"
);

var text = ExportSingleEnglishContentCellText(editableInnerXml);

// The line break becomes a single space (words are NOT run together), the run of
// ordinary spaces collapses to one space, and the non-breaking space survives.
Assert.That(text, Is.EqualTo("Some words joined\u00A0text here"));
Assert.That(
text,
Does.Not.Contain("wordsjoined"),
"a line break must not fuse the words on either side of it"
);
Assert.That(
text,
Does.Contain("\u00A0"),
"a non-breaking space must be preserved, not collapsed to an ordinary space"
);
}

/// <summary>
/// Exports a minimal one-page book whose single English bloom-editable has the given
/// inner XML, and returns the exported [en] page-content cell's text.
/// </summary>
private static string ExportSingleEnglishContentCellText(string editableInnerXml)
{
var html =
@"<html>
<head></head>
<body data-l1=""en"" data-l2="""" data-l3="""">
<div id=""bloomDataDiv""></div>
<div class=""bloom-page numberedPage customPage"" data-page-number=""1"">
<div class=""marginBox"">
<div class=""bloom-translationGroup bloom-trailingElement"" data-default-languages=""auto"">
<div class=""bloom-editable normal-style bloom-visibility-code-on bloom-content1"" lang=""en"" contenteditable=""true"">"
+ editableInnerXml
+ @"</div>
</div>
</div>
</div>
</body>
</html>";
var dom = new HtmlDom(html, true);

var mockLangDisplayNameResolver = new Mock<ILanguageDisplayNameResolver>();
mockLangDisplayNameResolver
.Setup(x => x.GetLanguageDisplayName("en"))
.Returns("English");
var exporter = new SpreadsheetExporter(mockLangDisplayNameResolver.Object);

using (var bookFolder = new TemporaryFolder("SpreadsheetExporterTests_NormBook"))
using (var sheetFolder = new TemporaryFolder("SpreadsheetExporterTests_NormSheet"))
{
var sheet = exporter.ExportToFolder(
dom,
bookFolder.FolderPath,
sheetFolder.FolderPath,
out _,
new ProgressSpy(),
OverwriteOptions.Overwrite
);
var contentRow = sheet.ContentRows.First(r =>
r.MetadataKey == InternalSpreadsheet.PageContentRowLabel
);
return contentRow.GetCell("[en]").Text;
}
}
}
}