[2.3] Provide fallback GalleryOptions class when argument missing from layout xml file

[gwharton]

Description (*)

The main product gallery layout file "catalog_product_view.xml" in "Magento_Catalog" now requires the argument "gallery_options" to be injected to the instance of the
"\Magento\Catalog\Block\Product\View\Gallery" class for the "product.info.media.image" block in order for the product page to be rendered correctly (This is a recent change). If this option is not present then the product page will just render the product image, and nothing else as the getGalleryOptions() call will just return null.

This is not a problem in vanilla Magento, as the argument is present in the layout file, however third party module providers, or anyone instantiating an instance of the "\Magento\Catalog\Block\Product\View\Gallery" class, or inheriting from it now needs to inject this argument for the template to render correctly. This has broken backwards compatability with old third party modules code, who now need to update to inject this argument.

This PR introduces code into the phtml template, that detects if the option is not injected, and falls back to hard coded "\Magento\Catalog\Block\Product\View\GalleryOptions" class.

Fixed Issues (if relevant)

1. When viewing product only the image shows #23432: When viewing product only the image shows

Manual testing scenarios (*)

1. Remove argument "gallery_options" from "catalog_product_view.xml" in Magento_Catalog module.
2. Clear cache
3. View product page (product that has an image)
4. Ensure product page renders correctly.

Questions or comments

Someone from Magento will need to determine whether this is required, or whether this is the right place for this check, given it is not needed in vanilla Magento, and is a fallback for when third party modules and users do not implement the argument correctly in layout.xml, or have not updated their module to include the argument. Given there is an issue raised on this, it is happening in the wild and this PR would help. If deemed not required, close accordingly.

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)

[m2-assistant]

Hi @gwharton. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

[gwharton]

There are no fallback options implemented for any of the other countless arguments injected into the various classes within "catalog_product_view.xml" so I'm erring on the side of closing the PR as won't fix and it should be on module developers to update their modules to inject the relevant required arguments if they instantiate the gallery class directly, or inherit from it, however there is the backward compatibility to think about, as many third party modules have now stopped working due to the requirement for a new argument to be injected.

[hostep]

I also don't know what the best solution would be in cases like this. The problem for module developers is then that they have to add this argument to their block but if they want their module to also be backwards compatible with older versions of Magento they can't just use the class Magento\Catalog\Block\Product\View\GalleryOptions as it won't exist in older versions of Magento. But I see you also bring up this point 👍

---

I had to fix a similar problem in one of our custom modules after Magento 2.3.1 got released which also added an a new argument to a certain block which we were also using in a custom module or ours and if you want to remain backwards compatible with older versions of Magento, you have to do a silly thing like adding your own new class as argument to the block containing a bunch of checks to make sure it works on all kinds of Magento versions (not saying this is the best solution, but this worked for us in that particular case, but it's far from ideal):

    <!-- some layout xml file in our custom module -->
    <block class="Magento\Store\Block\Switcher" name="some.name" template="switch/languages.phtml">
        <arguments>
            <argument name="view_model" xsi:type="object">Vendor\ModuleName\ViewModel\SwitcherUrlProvider</argument>
        </arguments>
    </block>

<?php

// This is a dummy class which we need to add a view model to the Magento\Store\Block\Switcher block.
// In Magento 2.3.1, the following change got introduced: https://github.com/magento/magento2/commit/5f2ad89601f00a87313ca25d8e8277872c64637f#diff-c454e4f2b71801fb00105c7492dce9f0
// This adds a view_model to the Magento\Store\Block\Switcher block using xml
// So we need to do that as well in this module
// But because the class Magento\Store\ViewModel\SwitcherUrlProvider doesn't exists in Magento < 2.3.1,
// we need to add this dummy class thingy which checks if that new class exists, and then inherits from it
// And if the new class doesn't exist, then just create an empty class
// -> which still needs to implement the ArgumentInterface though for Magento 2.2
// -> and which needs to implement the CollectionDataSourceInterface for Magento 2.1 (this doesn't make any sense, but it works!)

namespace Vendor\ModuleName\ViewModel;

// For Magento 2.3.1 and higher
if (class_exists(\Magento\Store\ViewModel\SwitcherUrlProvider::class))
{
    class SwitcherUrlProvider extends \Magento\Store\ViewModel\SwitcherUrlProvider
    {

    }
}
// For Magento 2.2.0 - 2.3.0 (including)
else if (interface_exists(\Magento\Framework\View\Element\Block\ArgumentInterface::class))
{
    class SwitcherUrlProvider implements \Magento\Framework\View\Element\Block\ArgumentInterface
    {

    }
}
// For Magento versions lower then 2.2.0
else
{
    class SwitcherUrlProvider implements \Magento\Framework\Data\CollectionDataSourceInterface
    {

    }
}

I believe there aren't really backwards compatibility rules for frontend related code in Magento at this point yet.

[orlangur]

This has broken backwards compatability with old third party modules code, who now need to update to inject this argument.

@gwharton I don't see to which item of https://devdocs.magento.com/guides/v2.3/contributor-guide/backward-compatible-development/ it corresponds.

[orlangur]

Can we obtain it from layout somehow or it's not present? Using ObjectManager in template is quite messy approach.

[orlangur]

@hostep,

but if they want their module to also be backwards compatible with older versions of Magento

This is simply not a right module releasing approach.

[hostep]

@orlangur: Why not? Should module vendors make separate release branches for Magento 2.3.0, 2.3.1 and 2.3.2? That's not really maintainable?

[gwharton]

On reflection on the morning after, I'm going to close this PR. Any objections, feel free to reopen. :)

[m2-assistant]

Hi @gwharton, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[orlangur]

@hostep wow. I didn't expect it could happen within 2.3.x release line. I'll raise corresponding issue in Slack in regards to BC.