Skip to content

Keep custom-layout page audio data (BL-16109) #7829

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

Open
JohnThomson wants to merge 2 commits into
base: master
Choose a base branch
from
+50 −3
Open

Keep custom-layout page audio data (BL-16109) #7829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0f1cebc
Keep custom-layout page audio data (BL-16109)
JohnThomson Apr 9, 2026
700e429
Post review improvements
JohnThomson Apr 9, 2026
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
15 changes: 12 additions & 3 deletions src/BloomExe/Book/BookData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1732,21 +1732,30 @@ public void GatherDataItemsFromXElement(
private List<Tuple<string, XmlString>> GetAttributesToSave(SafeXmlElement node)
{
var result = new List<Tuple<string, XmlString>>();
if (HtmlDom.IsInCustomLayoutPage(node))
if (node.Name == "img" && HtmlDom.IsInCustomLayoutPage(node))
{
// We don't want to transfer attribute values or classes from the custom page
// We don't want to transfer most image attribute values or classes from the custom page
// layout to the standard one. It's likely that special styling or image cropping
// or anything similar will have the wrong effect there. The one exception is the
// src of an image: we allow changing the cover image in one place to change it in
// the other, just as changing the text of a title in one place changes it in both.
// We don't need to worry about re-creating attribute values inside the custom margin
// box, because its whole content is saved.
if (node.Name == "img" && !string.IsNullOrWhiteSpace(node.GetAttribute("src")))
// We do want to transfer data normally for text elements; for example, talking book
// recordings should transfer (and also survive the book being opened in 6.3).
if (!string.IsNullOrWhiteSpace(node.GetAttribute("src")))
result.Add(
Tuple.Create("src", XmlString.FromUnencoded(node.GetAttribute("src")))
);
return result;
}
// Margin box doesn't have any of the properties that would normally make it one of the nodes
// passed to this method. However, it is the node that gets passed for a custom page. It would
// probably be harmless to process it normally, but it would result in the data-div node for
// the custom page content having the class marignBox. That might cause something unexpected,
// and the marginBox doesn't have any classes or attributes we need to preserve, so just skip it.
if (node.HasClass("marginBox"))
return result;
Comment on lines +1735 to +1758
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Behavioral change: all attributes on text elements in custom layout pages now transfer to standard pages

The old code at src/BloomExe/Book/BookData.cs:1735 suppressed ALL attributes for ALL element types in custom layout pages (except img src). The new code only suppresses attributes for img elements and marginBox nodes. This means that for text elements like div[data-book='bookTitle'], not only do the intended talking book attributes (e.g., data-audiorecordingmode, recordingmd5, data-duration) now propagate, but so do other attributes like style, id, and any other custom attributes. The restore path at line 2031 still prevents these from being merged INTO custom layout page elements, but they WILL be merged into standard page elements. This is probably fine since _attributesNotToCopy already filters UI junk, and the _classesNotToCopy list filters bloom-managed classes. However, if a custom layout page has specialized style or layout attributes on text elements, those could unexpectedly transfer to the standard page's version of those same elements.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

foreach (var attr in node.AttributePairs)
{
if (_attributesNotToCopy.Contains(attr.Name))
Expand Down
38 changes: 38 additions & 0 deletions src/BloomTests/Book/BookDataTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3491,6 +3491,44 @@ public void SuckInDataFromEditedDom_CustomLayoutPage_InactivatesNestedDataAndCla
);
}

[Test]
public void SuckInDataFromEditedDom_CustomLayoutPage_DataBookEntriesKeepTalkingBookAttributes()
{
var bookDom = new HtmlDom(
@"<html><head></head><body>
<div id='bloomDataDiv'>
<div data-book='contentLanguage1' lang='*'>xyz</div>
<div data-book='contentLanguage2' lang='*'>en</div>
</div>
<div class='bloom-page bloom-customLayout' data-custom-layout-id='customOutsideFrontCover' id='customCover1'>
<div class='marginBox'></div>
</div>
</body></html>"
);
var data = new BookData(bookDom, _collectionSettings, null);

var editedPageDom = new HtmlDom(
@"<html><head></head><body>
<div class='bloom-page bloom-customLayout' data-custom-layout-id='customOutsideFrontCover' id='customCover1'>
<div class='marginBox'>
<div class='bloom-translationGroup'>
<div class='bloom-editable bloom-visibility-code-on audio-sentence' data-book='bookTitle' lang='en' id='i6a720491' data-audiorecordingmode='TextBox' recordingmd5='5b5efdab7f705554614a6383ae6d9469' data-duration='5.839433'><p>My Title</p></div>
</div>
</div>
</div>
</body></html>"
);

data.SuckInDataFromEditedDom(editedPageDom);

AssertThatXmlIn
.Dom(bookDom.RawDom)
.HasSpecifiedNumberOfMatchesForXpath(
"//div[@id='bloomDataDiv']/div[@data-book='bookTitle' and @lang='en' and @id='i6a720491' and @data-audiorecordingmode='TextBox' and @recordingmd5='5b5efdab7f705554614a6383ae6d9469' and @data-duration='5.839433']",
1
);
}

[Test]
public void GatherDataItemsFromXElement_CustomLayoutPageWithXmatterPage_GathersXmatterAttributes()
{
Expand Down