Skip to content

Namespace related/up-sell item ids so the blocks stop hiding each other (#37482) - #41171

Open
lbajsarowicz wants to merge 3 commits into
magento:2.4-developfrom
lbajsarowicz:fix/37482-upsell-related-visibility
Open

Namespace related/up-sell item ids so the blocks stop hiding each other (#37482)#41171
lbajsarowicz wants to merge 3 commits into
magento:2.4-developfrom
lbajsarowicz:fix/37482-upsell-related-visibility

Conversation

@lbajsarowicz

@lbajsarowicz lbajsarowicz commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Description

When the same product is assigned as both a Related Product and an Up-Sell Product, the two blocks fight over each other's visibility: an item shown in one block disappears from the other.

app/code/Magento/Catalog/view/frontend/templates/product/list/items.phtml renders every related and up-sell item with an id derived only from the product:

id="product-item_<?= $_item->getId() ?>"

and then emits a hide rule through $secureRenderer->renderStyleAsTag('display:none;', 'li#product-item_' . $_item->getId()).

Two consequences, both present on current 2.4-develop:

  1. The same product rendered in both blocks produces duplicate DOM ids, which is invalid HTML.
  2. The emitted style rule is document-global. When the related block's limit/shuffle logic hides its own entry, the selector also matches the up-sell block's <li> for that product, and vice versa.

The fix namespaces the id by block type, so each block gets its own element and its own selector.

This PR continues #37482 by @igorwulff (Partner: Youwe), rebased onto current 2.4-develop. Their commit and authorship are preserved.

Fixed Issues

No linked issue on the original PR; the defect is described above and is reproducible on current 2.4-develop.

Manual testing scenarios

  1. Create product A and product B.
  2. On product A, assign product B as both a Related Product and an Up-Sell Product. Save.
  3. Open product A on the storefront.
  4. Before: product B appears in only one of the two blocks — the other block's entry is hidden by the shared style rule. Inspecting the DOM shows two elements with the same id="product-item_<B>".
    After: product B appears in both blocks, with distinct ids product-item-related_<B> and product-item-upsell_<B>.

Questions or comments

The only feedback ever left on the original PR was @engcom-Hotel asking for automated coverage:

I suggest you to cover the changes with some automated tests like functional tests.

That is what this adds: StorefrontRelatedAndUpsellSharedProductVisibilityTest assigns one product as both a Related and an Up-Sell product, then asserts there is exactly one li#product-item-related_<id> and exactly one li#product-item-upsell_<id>, that nothing matches the old collapsed li#product-item_<id>, and that neither element computes to display: none.

One note on reading that test: under the default configuration there is no item limit on these blocks, so the "both are visible" assertions alone would pass even without the fix. The assertions that actually pin the regression are the id-uniqueness ones and the absence of the legacy id.

Verified locally on 2.4-develop:

  • The new MFTF test validates against testSchema.xsd, and all eight action groups it references exist in core.
  • git grep confirms the old product-item_<id> id is referenced nowhere else in core — no JS, LESS, CSS or template depends on it, so the rename is self-contained. (The @product-item__hover__* hits in Luma's LESS are unrelated variables.)

The original PR also carried a second commit that rewrote the copyright header and renamed $block->escape* to $escaper->escape*. That was purely cosmetic, conflicted with changes upstream has since made itself, and added nothing to the fix, so it was dropped during the rebase — this PR is limited to the id change.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

Resolved issues:

  1. resolves [Issue] Namespace related/up-sell item ids so the blocks stop hiding each other (#37482) #41176: Namespace related/up-sell item ids so the blocks stop hiding each other (Fix issue where upsell and related products were controlling each oth… #37482)

igorwulff and others added 2 commits August 30, 2026 13:10
…ers visibility

On a PDP with both upsell and related products with a limit set and a shared set of some of the products an issue could arise where products being hidden or shown by the related product js would impact the upsell products and vice versa.
Covers the fix for magento#37482: when the same product is assigned as both a
Related Product and an Up-Sell Product, verifies each block renders it with
a distinct, block-namespaced DOM id and that neither block's hide-on-load
script suppresses the other block's entry.
@lbajsarowicz

Copy link
Copy Markdown
Contributor Author

@magento run all tests

@m2-assistant

m2-assistant Bot commented Aug 30, 2026

Copy link
Copy Markdown

Hi @lbajsarowicz. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

  • @magento run all tests - run or re-run all required tests against the PR changes
  • @magento run <test-build(s)> - run or re-run specific test build(s)
    For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:
  1. Database Compare
  2. Functional Tests CE
  3. Functional Tests EE
  4. Functional Tests B2B
  5. Integration Tests
  6. Magento Health Index
  7. Sample Data Tests CE
  8. Sample Data Tests EE
  9. Sample Data Tests B2B
  10. Static Tests
  11. Unit Tests
  12. WebAPI Tests
  13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.


For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

The original PR's second commit was dropped during the rebase as cosmetic.
It was not: the Magento2 sniff warns on every $block->escape* call in a
touched template, and the upstream Static Tests build fails on warnings.
Convert the 31 call sites to $escaper, wrap the lines the two extra
characters push past 120, and wrap the over-long lines in the new MFTF
test.
@lbajsarowicz
lbajsarowicz force-pushed the fix/37482-upsell-related-visibility branch from 3e2e4fa to c1ca4fc Compare August 30, 2026 11:47
@lbajsarowicz

Copy link
Copy Markdown
Contributor Author

@magento run all tests

@engcom-Bravo

Copy link
Copy Markdown
Contributor

@magento create issue

@igorwulff

Copy link
Copy Markdown
Contributor

@lbajsarowicz thank you for picking this up. I haven't seen the updates nor the time to look into this. Would be great if this can get resolved for others as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: pending review

Projects

Status: Pending Review

Development

Successfully merging this pull request may close these issues.

[Issue] Namespace related/up-sell item ids so the blocks stop hiding each other (#37482)

3 participants