Fix book icon when branding has an image but cover has none (BL-16780) - #8256
Fix book icon when branding has an image but cover has none (BL-16780)#8256hatton wants to merge 3 commits into
Conversation
When a branded book's cover picture was still the placeholder, the search for a fallback cover image accepted any img on the cover. It therefore chose the branding logo, and for the ABC brandings that logo is an SVG, which PalasoImage cannot load, so the book had no thumbnail at all. The search now considers only a picture inside an image container, and rejects any element carrying the branding, licenseImage, or QR code class. Both rules are needed: the image container is the main guard, but a branding pack supplies its own markup, so it may wrap its logo in a container. The branding, license, and QR code test is now the shared helper IsBrandingOrLicenseImage, used by both the cover-image search and HasImages(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
| Filename | Overview |
|---|---|
| src/BloomExe/Book/Book.cs | Refines cover-image fallback selection and reuses a shared branding/license exclusion helper; no actionable defect was established. |
| src/BloomTests/Book/BookTests.cs | Adds focused regression coverage for branding images outside and inside image containers. |
Reviews (1): Last reviewed commit: "Fix the book icon taking the branding lo..." | Re-trigger Greptile
|
[Claude Opus 5 from Hatton's machine during preflight] Consulted Devin on 2026-08-28 20:34 UTC up to commit Devin found no bugs and nothing to investigate. It raised one informational item, which we agree with and are not acting on: restricting the fallback search to images inside an image container means a bare Greptile also reviewed this commit and raised nothing. CodeRabbit's auto-review is turned off for this repo ( |
| if (candidate == designatedCoverImage) | ||
| continue; | ||
|
|
||
| // The image container is the main guard, but a branding pack supplies its own |
There was a problem hiding this comment.
nah, don't be pessimistic. remove this guard.
Replaces the two-stage search with the rule stated plainly: take whatever the book marks as its cover, failing that an image out of an image container, and fall back to the placeholder only when nothing real turns up. This drops the IsBrandingOrLicenseImage helper and leaves NonTrivialImageFileExists exactly as it was. Restricting the search to image containers already keeps the branding logo, the license image, and the QR code out, because none of them sits in a container, so the class test was redundant here and its only other caller had it inline already. The mark is read from the img, from the image container, or from an image inside a marked container. Only the img carries it on this branch; marking the container came in on Version6.5, which is downstream, so accepting all three shapes lets this survive the merge forward. A marked container has no image URL of its own, so without the third shape the search would fall through to an earlier container and take the wrong picture. The candidate order is built with Concat rather than one XPath union, because a union returns document order and would lose the preference for the marked image. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
[Claude Opus 5 from Hatton's machine during preflight] Consulted Devin on 2026-08-28 21:02 UTC up to commit No bugs and nothing to investigate. Two informational items, neither of which we are acting on:
|
…780) The cover image search had no test for two of its rules. The first is the ordinary case: the book marks its own cover picture, that picture is real, and the mark must win even though a decoration sits earlier on the page. That ordering is why the marked candidates are searched before the image containers rather than in one pass over the document. The second is the obsolete image representation, where the picture is a background image on the image container itself with no img inside, which is why the container is a candidate in its own right. Both tests fail if the code that they cover is removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Problem
A branded book whose cover picture is still the placeholder gets the wrong book icon, or no icon at all. Bloom picks the branding logo as the book's cover image. With the ABC brandings that logo is an SVG, which
PalasoImagecannot load, so the thumbnail fails completely. Example book: ABC-BARMM-007297-2656.Cause
When the designated cover image is a placeholder,
GetCoverImagePathAndEltlooks for a fallback. That search accepted anyimganywhere on the front cover. A cover also carries images that belong to the branding and to the license, and those are not pictures of the book.Fix
The search is now one ordered list of candidates:
Restricting step 2 to image containers is what fixes the bug: the branding logo, the license image, and the QR code of the "Made with Bloom" badge never sit in a container.
The mark in step 1 is read from the
img, from the image container, or from an image inside a marked container. Only theimgcarries it on this branch; marking the container came in onVersion6.5, which is downstream, so accepting all three shapes lets this survive the merge forward. A marked container has no image URL of its own, so without the third shape the search would fall through to an earlier container and take the wrong picture.Four tests cover the rules: the reported branding case, a loose real image on the cover, the container-level mark, and the pre-existing behaviour that a designated placeholder yields to a real picture.
Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16780
Devin review
This change is