-
-
Notifications
You must be signed in to change notification settings - Fork 437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed cache issue in primary navigation block #4040
Conversation
I think you fixed a bug affecting 90% of magento1 (and 2) stores, where many disable the HTML_Block cache as a workaround. will test asap! |
... rly? I thinks its (only) when people want different navigation for non-category CMS pages. Imho not a bug ... Maybe (untested) if $
Is this a "common" issue or something that could/should be fixed in an extension? Imho. |
I thought I had solved it, but today I find a category selected again on the homepage, so it doesn't work correctly. @F1Red5 I need to go in-depth with the debug because it doesn't occur on all sites. |
Sorry for the confusion, I've finally figure out how this occur. |
block Mage_Catalog_Block_Navigation is only used in catalog list <block type="core/text_list" name="top.menu" as="topMenu" translate="label">
<label>Navigation Bar</label>
<block type="page/html_topmenu" name="catalog.topnav" template="page/html/topmenu.phtml">
<block type="page/html_topmenu_renderer" name="catalog.topnav.renderer" template="page/html/topmenu/renderer.phtml"/>
</block>
</block>
i found Mage_Page_Block_Html_Topmenu read current_entity_key from registry /**
* Retrieve current entity key
*
* @return int|string
*/
public function getCurrentEntityKey()
{
if ($this->_currentEntityKey === null) {
$this->_currentEntityKey = Mage::registry('current_entity_key')
? Mage::registry('current_entity_key') : Mage::app()->getStore()->getRootCategoryId();
}
return $this->_currentEntityKey;
} so i've setted in PDP controller current_entity_key with the full category path like in category controller I know it's intuitive to set current_entity_key with the product ID, but that would create many cache blocks for each product, so I set it with the current path. this way there is one cache foreach category path on PDP |
i can confirm it works, also on PDP the category of product it's selected now |
@empiricompany can I test this problem without a full page cache plugin and with the RWD theme? cause I can't reproduce it on a vanilla OM installation (from a super quick test) |
@fballiano Unfortunately, the latest Docker Desktop update completely destroyed my dev environment, but it's a problem I've always encountered on all installations without an FPC. Are you sure you followed these steps?
Issue: The category of the visited product should remain selected. Additionally, I can say that I use the flat catalog and the config catalog/seo/product_use_categories = 1. |
@empiricompany never faced that issue and the category path should be in cache-info ...
Maybe try https://github.com/AOEpeople/Aoe_TemplateHints for easier debugging. |
yes, I have used aoe_templatepathints to debug and issue is on top menu block not in navigation, please try with flat catalog and use category path in product url maybe this cause the error |
Mhh, can you share a screenshot with AoE-TH? |
I've checked it on the latest main and I can confirm the issue. On sample data:
|
In the last commit, I've highlighted the active category. |
no no I knew there was no active category CSS, I was checking the source code for the "active" class, I'm tight with time lately, I'll try to recheck asap |
It happens only when FLAT CATALOG is enabled (confirmed, also tested that your PR works and solves the problem). I'm thinking, how is this happening? I mean, the fix doesn't seem to be strictly-related to the problem... anyway since it works I'm still approving, waiting a bit before merging to see if we get some other kind of feedback. |
It's difficult to explain, but when you visit a PDP, a block cache of topmenu is stored without the specific tags of the category path. Then, when there is no category selected, like on the homepage, CMS, or contact pages, this cached block is retrieved without tags. The PR also selects the category on the PDP according to the breadcrumbs. |
Description (*)
On fresh block cache if i reload a PDP page the category of the product still marked as active in other pages if there is not a specific current category (homepage, contacts, cms page etc..)
Related Pull Requests
Fixed Issues (if relevant)
#4041
Manual testing scenarios (*)
The category of first PDP loaded still flagged as active.
PS: The RWD theme does not have a CSS rule to highlight the active menu, so you must inspect the HTML.
This PR also adds css rule to highlight currente active category.
Questions or comments
Contribution checklist (*)