Skip to content

Preserve Xmatter music when book is updated (BL-16036) #7777

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

Closed
JohnThomson wants to merge 1 commit into from
+191 −29
Closed

Preserve Xmatter music when book is updated (BL-16036) #7777

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
79 changes: 79 additions & 0 deletions src/BloomExe/Book/Book.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2553,6 +2553,7 @@ public void BringXmatterHtmlUpToDate(HtmlDom bookDOM)
.Select(page => page.GetAttribute("data-custom-layout-id"))
.Where(id => !string.IsNullOrEmpty(id))
.ToHashSet();
var xmatterPageAttributesToPreserve = CaptureXmatterPageAttributesToPreserve(bookDOM);
XMatterHelper.RemoveExistingXMatter(bookDOM, oldIds);
// this says, if you can't figure out the page size, use the one we got before we removed the xmatter...
// still requiring it to be a valid layout.
Expand All @@ -2577,12 +2578,90 @@ var page in bookDOM
if (customLayoutIds.Contains(customLayoutId))
page.AddClass("bloom-customLayout");
}
RestoreXmatterPageAttributesToPreserve(bookDOM, xmatterPageAttributesToPreserve);
var dataBookLangs = bookDOM.GatherDataBookLanguages();
TranslationGroupManager.PrepareDataBookTranslationGroups(bookDOM.RawDom, dataBookLangs);

helper.InjectDefaultUserStylesFromXMatter();
}

private static readonly HashSet<string> kXmatterPageAttributesToPreserve = new(
HtmlDom.PageAttributesToSaveAndPreserveInXmatter
);

private static Dictionary<
string,
Dictionary<string, string>
> CaptureXmatterPageAttributesToPreserve(HtmlDom bookDOM)
{
var preservedAttributesByXmatterPage =
new Dictionary<string, Dictionary<string, string>>();
foreach (
var page in bookDOM
.SafeSelectNodes("//div[contains(@class, 'bloom-page') and @data-xmatter-page]")
.Cast<SafeXmlElement>()
)
{
var xmatterPageKey = page.GetAttribute("data-xmatter-page").Trim();
if (string.IsNullOrEmpty(xmatterPageKey))
continue;

if (
!preservedAttributesByXmatterPage.TryGetValue(
xmatterPageKey,
out var attributes
)
)
{
attributes = new Dictionary<string, string>();
preservedAttributesByXmatterPage.Add(xmatterPageKey, attributes);
}

foreach (var attributeName in kXmatterPageAttributesToPreserve)
{
if (page.HasAttribute(attributeName) && !attributes.ContainsKey(attributeName))
{
attributes.Add(attributeName, page.GetAttribute(attributeName));
}
}
}

return preservedAttributesByXmatterPage;
}

private static void RestoreXmatterPageAttributesToPreserve(
HtmlDom bookDOM,
Dictionary<string, Dictionary<string, string>> preservedAttributesByXmatterPage
)
{
if (preservedAttributesByXmatterPage.Count == 0)
return;

foreach (
var page in bookDOM
.SafeSelectNodes("//div[contains(@class, 'bloom-page') and @data-xmatter-page]")
.Cast<SafeXmlElement>()
)
{
var xmatterPageKey = page.GetAttribute("data-xmatter-page").Trim();
if (
string.IsNullOrEmpty(xmatterPageKey)
|| !preservedAttributesByXmatterPage.TryGetValue(
xmatterPageKey,
out var attributes
)
)
{
continue;
}

foreach (var attribute in attributes)
{
page.SetAttribute(attribute.Key, attribute.Value);
}
}
}

// Around May 2014 we added a class, .bloom-requireParagraphs, backed by javascript that makes geckofx
// emit <p>s instead of <br>s (which you can't style and don't leave a space in wkhtmltopdf).
// If there is existing text after we added this, it needs code to do the conversion. There
Expand Down
18 changes: 7 additions & 11 deletions src/BloomExe/Book/BookData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ public class BookData
/// </summary>
private const string kDataXmatterPage = "data-xmatter-page";

private static readonly HashSet<string> _xmatterPageAttributesToPreserveSet =
new HashSet<string>(HtmlDom.PageAttributesToSaveAndPreserveInXmatter);

private static readonly string[] _attributesToInactivate =
{
// key attributes used by the BookData class itself, that could function as a source
Expand Down Expand Up @@ -1662,18 +1665,11 @@ SafeXmlElement element
new HashSet<KeyValuePair<string, string>>();
foreach (var attribute in element.AttributePairs)
{
if (attribute.Name != kDataXmatterPage && attribute.Name.StartsWith("data-"))
if (_xmatterPageAttributesToPreserveSet.Contains(attribute.Name))
{
// xmatter pages are not numbered. See https://issues.bloomlibrary.org/youtrack/issue/BL-7303.
// This will clean up books that have wrongly set backmatter page numbers.
// NB: if we ever decide to allow specifying some xmatter pages to be numbered, we'll need to figure
// out how to handle that information here and in HtmlDom.UpdatePageNumberAndSideClassOfPages().
if (attribute.Name == "data-page-number")
attributes.Add(new KeyValuePair<string, string>(attribute.Name, ""));
else
attributes.Add(
new KeyValuePair<string, string>(attribute.Name, attribute.Value)
);
attributes.Add(
new KeyValuePair<string, string>(attribute.Name, attribute.Value)
);
}
}

Expand Down
43 changes: 25 additions & 18 deletions src/BloomExe/Book/HtmlDom.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1886,6 +1886,29 @@ public static void RemoveTemplateEditingMarkup(SafeXmlElement editedPageDiv)
public const string musicAttrName = "data-backgroundaudio";
public const string musicVolumeName = musicAttrName + "volume";

// Page-level attributes that should be copied from edited page to storage page
// but do not need to survive xmatter replacement.
public static readonly string[] PageAttributesToSaveOnly =
{
"data-correct-sound",
"data-wrong-sound",
"data-same-size",
"data-show-targets-during-play",
"data-show-answers-in-targets",
"data-missing-game-theme",
};

// Additional page-level attributes that should be saved after editing and also
// preserved across xmatter replacement.
public static readonly string[] PageAttributesToSaveAndPreserveInXmatter =
{
musicAttrName,
musicVolumeName,
};

private static readonly string[] _pageAttributesToSaveAfterEditing =
PageAttributesToSaveOnly.Concat(PageAttributesToSaveAndPreserveInXmatter).ToArray();

public static void ProcessPageAfterEditing(
SafeXmlElement destinationPageDiv,
SafeXmlElement edittedPageDiv
Expand Down Expand Up @@ -1921,24 +1944,8 @@ SafeXmlElement edittedPageDiv
//html file in a browser.
destinationPageDiv.SetAttribute("lang", edittedPageDiv.GetAttribute("lang"));

// Copy the two background audio attributes which can be set using the music toolbox.
// Ensuring that volume is missing unless the main attribute is non-empty is
// currently redundant, everything should work if we just copied all attributes.
// (But, it IS important to DELETE any old versions of these attributes if the edited page div
// does NOT have them.)
// Review: should we copy all attributes? All data- attributes?
string[] attrsToCopy = new[]
{
musicAttrName,
musicVolumeName,
"data-correct-sound",
"data-wrong-sound",
"data-same-size",
"data-show-targets-during-play",
"data-show-answers-in-targets",
"data-missing-game-theme",
};
foreach (var attr in attrsToCopy)
// Copy user-editable page attributes from the edited page back to storage.
foreach (var attr in _pageAttributesToSaveAfterEditing)
{
var value = edittedPageDiv.GetAttribute(attr);
if (string.IsNullOrEmpty(value))
Expand Down
80 changes: 80 additions & 0 deletions src/BloomTests/Book/BookTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2375,6 +2375,86 @@ public void BringBookUpToDate_CoverImageHasMetaData_HtmlForCoverPageHasMetaDataA
);
}

[Test]
public void BringBookUpToDate_PreservesCuratedXmatterPageAttributesOnly()
{
_bookDom = new HtmlDom(
@"
<html>
<head>
<meta name='xmatter' content='Factory' />
</head>
<body>
<div id='bloomDataDiv'></div>
<div class='bloom-page cover coverColor bloom-frontMatter frontCover A5Portrait side-right'
data-page='required singleton'
data-export='should-not-stick'
data-xmatter-page='frontCover'
data-backgroundaudio='SoundTrack0.mp3'
data-backgroundaudiovolume='0.42'
id='frontCover-id'>
<div class='marginBox'></div>
</div>
</body>
</html>"
);

var book = CreateBook();
book.BringBookUpToDate(new NullProgress());

var frontCoverPage = (SafeXmlElement)
book.RawDom.SelectSingleNodeHonoringDefaultNS(
"//div[contains(@class, 'bloom-page') and @data-xmatter-page='frontCover']"
);
Assert.That(
frontCoverPage.GetAttribute(HtmlDom.musicAttrName),
Is.EqualTo("SoundTrack0.mp3")
);
Assert.That(frontCoverPage.GetAttribute(HtmlDom.musicVolumeName), Is.EqualTo("0.42"));
Assert.That(
frontCoverPage.GetAttribute("data-export"),
Is.EqualTo("front-matter-cover")
);
}

[Test]
public void BringXmatterHtmlUpToDate_FrontCoverBackgroundAudioAttributes_ArePreserved()
{
_bookDom = new HtmlDom(
@"
<html>
<head>
<meta name='xmatter' content='Factory' />
</head>
<body>
<div id='bloomDataDiv'></div>
<div class='bloom-page cover coverColor bloom-frontMatter frontCover A5Portrait side-right'
data-page='required singleton'
data-export='front-matter-cover'
data-xmatter-page='frontCover'
data-backgroundaudio='SoundTrack0.mp3'
data-backgroundaudiovolume='0.21'
id='frontCover-id'>
<div class='marginBox'>
<div class='bloom-canvas'></div>
</div>
</div>
</body>
</html>"
);

var book = CreateBook();

book.BringXmatterHtmlUpToDate(book.OurHtmlDom);

AssertThatXmlIn
.Dom(book.RawDom)
.HasSpecifiedNumberOfMatchesForXpath(
"//div[contains(@class, 'bloom-page') and @data-xmatter-page='frontCover' and @data-backgroundaudio='SoundTrack0.mp3' and @data-backgroundaudiovolume='0.21']",
1
);
}

[Test]
public void BringBookUpToDate_CustomLayoutPage_PreservesCustomLayoutClassWithSubscription()
{
Expand Down