Skip the logo markup when a brand or supplier has none - #216
Open
boo-code wants to merge 1 commit into
Open
Conversation
|
Hello @boo-code! This is your first pull request on classic-theme repository of the PrestaShop project. Thank you, and welcome to this Open Source community! |
2 tasks
Hlavtox
approved these changes
Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
image.bySize.small_default.urlunconditionally. Since PrestaShop/PrestaShop#41699 landed in core,ManufacturerLazyArray::getImage()andSupplierLazyArray::getImage()returnnullwhen no logo file exists, so on/brandsand/suppliersevery logo-less entity now dereferencesnullthree times over. The<source>lines were already guarded with!empty; only the<img src>was not. This wraps the<picture>in the same guard, so an entity without a logo renders no image markup instead of a broken one. Its name is still shown, linked, by thebrand-infos/supplier-infosblock right below./brandswith this theme active. Before this change the page emitsTrying to access array offset on value of type nulland, because the warning is printed inside the attribute, the tag comes out assrc="<br /><b>Warning</b>: ..."; with warnings off it issrc="", which the browser resolves as a second request for the page itself. After the change there is no image element and no warning, and brands that do have a logo are unaffected. Suppliers are the same page-for-page, and note that on a stock installimg/su/has no logo files at all, so/suppliersreproduces it for every row oncePS_DISPLAY_SUPPLIERSis on.Measured on a 9.2.0 shop with this theme active, counting the warning in the rendered HTML:
Three warnings per row is the whole chain being walked on
null:image['bySize'], then['small_default'], then['url'].One choice worth naming, since it is a judgement call rather than a forced one. When there is no logo I render nothing in the image slot, keeping the surrounding
brand-img/supplier-imgcontainer so the grid does not reflow. The alternative would be a text fallback inside that slot, which is what the Hummingbird product page does - but on this listing the name is already immediately below inbrand-infos, so it would print twice. If you would rather have the placeholder image back for logo-less rows, that is a one-line change to the same guard and I will redo it.