Prevent 6.3 from 'fixing' audio IDs in 6.4 custom covers (BL-15902)#7704
Prevent 6.3 from 'fixing' audio IDs in 6.4 custom covers (BL-15902)#7704JohnThomson wants to merge 1 commit intoVersion6.3from
Conversation
| var duplicateAudioIdsFixed = 0; | ||
| foreach (var audioElement in nodes) |
There was a problem hiding this comment.
🔴 duplicateAudioIdsFixed is never incremented, so SynchronizeDataItemsThroughoutDOM() is dead code
In FixDuplicateAudioIdsInDataDiv, the local variable duplicateAudioIdsFixed is initialized to 0 at line 1284 and checked at line 1296, but it is never incremented inside the loop when a duplicate is actually found and fixed (lines 1289–1293). As a result, the condition if (duplicateAudioIdsFixed > 0) is always false, and _bookData.SynchronizeDataItemsThroughoutDOM() is never called.
Root Cause and Impact
The loop at src/BloomExe/Book/Book.cs:1285-1293 calls FixDuplicateAudioId when a duplicate is detected, but never does ++duplicateAudioIdsFixed. Compare with the analogous method FixMissingAudioIdsInDataDiv at src/BloomExe/Book/Book.cs:1343-1360, which correctly checks nodeList.Count > 0 before calling SynchronizeDataItemsThroughoutDOM().
Without the synchronization call, duplicate audio IDs that are fixed in the data div are not propagated to the rest of the book's DOM, leaving the book pages out of sync with the data div.
This is a pre-existing bug but is in the method directly modified by this PR.
(Refers to lines 1284-1297)
Was this helpful? React with 👍 or 👎 to provide feedback.
| // We expect the customCover to have copies of other stuff in the data div. | ||
| .Where(x => | ||
| !( | ||
| x is SafeXmlElement e | ||
| && e.ParentWithAttributeValue("data-book", "customCover") != null | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🚩 customCover string literal only appears in C# code — verify it matches what 6.4 actually emits
The filter added in this PR checks for data-book="customCover" via e.ParentWithAttributeValue("data-book", "customCover"). However, searching the entire repository for customCover only returns these two lines in Book.cs. There are no HTML templates, test fixtures, or other references that confirm this is the exact attribute value used by version 6.4's custom cover feature. The commit c3f8b0378 ("Adds the cover layout control and allows a 'custom' cover (BL-15902)") is in the git log but its contents aren't available locally. It would be worth verifying that customCover is the exact data-book value used in the 6.4 branch.
Was this helpful? React with 👍 or 👎 to provide feedback.
This change is