Skip to content
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

[Backport] Prevent layout cache corruption in edge case #40

Merged
merged 32 commits into from
Jul 3, 2018

Conversation

gelanivishal
Copy link
Owner

Original Pull Request

magento#12314

Description

When layout is loaded from the cache, it ought to repopulate the list of applied layout handles to prevent any chance of a layout handle being reapplied later, layering on top of the XML loaded from cache. This pull request adds that missing functionality.

The bug caused by the status quo is triggered when layout is loaded early before the page actually begins to render and before all layout handles may be applied. (Layout is loaded early, for example, when $resultPage->getConfig()->getTitle() is called, as happens in multiple core controllers, including checkout.) If this initial layout load matches a cache key but the latter layout load (with all layout handles assembled) does not, then when the latter layout load occurs, the guard condition which attempts to block duplicate layout handle application (in _merge of Magento\Framework\View\Model\Layout\Merge) does not get triggered, since the layout handles loaded from cache were not populated.

This duplicate XML generally does not cause issues, because earlier nodes are replaced by later nodes via the _overrideElementWorkaround method in \Magento\Framework\View\Layout\ScheduledStructure\Helper, and since the duplicate nodes are the same as the originals, there's no problem. However, because this method removes the entire tree (including descendants) of the first instance of duplicated nodes, it can lead to nodes missing from the final structure, depending on the layout order sequence. (For example, if a parent node such as <container name="content" label="Main Content Area"/> ends up appearing after some element that was supposed to be inserted into that container, the child block never gets re-added.) This method's algorithm is a little clunky itself and could probably be refined, but by merging this fix, we avoid at least one scenario where this algorithm is even needed.

Manual testing scenarios

As far as I've seen this does not cause any issues in core alone, but it can cause issues when custom modules extend core in standard ways. This describes one concrete case where this can occur, but it is likely to occur in other cases as well.

  1. Create a custom module:
    1. Add an afterExecute plugin on Magento\Checkout\Controller\Index\Index.
    2. In this plugin method, conditionally add a layout handle if the cart contains a particular product.
  2. Enable and clear layout cache.
  3. Add a product to the cart other than the one checked in the plugin condition.
  4. Load checkout. (Checkout should appear correctly.)
  5. Add the product to the cart checked in the plugin condition.
  6. Load checkout again. (Without this fix, checkout will break, as the customer.customer.data block will not be loaded. With this fix, checkout will load correctly.)

omiroshnichenko and others added 30 commits April 25, 2018 14:38
…d and bundle folder contains any other files
[thunder] MAGETWO-90925: Wrong case in classname CLI in ProcessCronQueueObserver
-[MAGETWO-90925] (https://jira.corp.magento.com/browse/MAGETWO-90925) Wrong case in classname CLI in ProcessCronQueueObserver
Merge branch '2.1' of github.com:magento/magento2ce into 2.1.14-develop-update
[TSG] Backporting for 2.1 (pr53) (2.1.14)
…update

[Plankton] Merge release to 2.1.14-develop
…nto#16467

 - Merge Pull Request magento#16467 from gelanivishal/magento2:2.1-develop-PR-port-14029
 - Merged commits:
   1. d2a3065
… name(Magento_Sales::order/info.phtml). magento#16475

 - Merge Pull Request magento#16475 from gelanivishal/magento2:2.1-develop-PR-port-16206
 - Merged commits:
   1. 18f255b
Accepted Public Pull Requests:
 - magento#16467: [Backport] Fix $useCache for container child blocks (by @gelanivishal)
- Merge tag '2.1.14' into 2.1-develop
Accepted Public Pull Requests:
 - magento#16475: [Backport] Declare module namespace before template path name(Magento_Sales::order/info.phtml). (by @gelanivishal)
Alexander Akimov and others added 2 commits July 3, 2018 00:41
@gelanivishal gelanivishal merged commit 8555578 into 2.1-develop Jul 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.