Keep custom-layout page audio data (BL-16109)#7829
Keep custom-layout page audio data (BL-16109)#7829JohnThomson wants to merge 2 commits intomasterfrom
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
JohnThomson
left a comment
JohnThomson
left a comment
There was a problem hiding this comment.
@JohnThomson made 1 comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion.
| 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; |
There was a problem hiding this comment.
🚩 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.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
This change is