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

Fix merging nested <var /> in view.xml #7556

Merged
merged 1 commit into from
Apr 12, 2017

Conversation

torreytsui
Copy link
Contributor

@torreytsui torreytsui commented Nov 24, 2016

Nested <var /> in view.xml cannot be merged

Problem

The view.xsd defined that <var /> element can contain unlimited <var /> children.

<!-- magento/framework/Config/etc/view.xsd -->
<!-- ... -->
    <xs:complexType name="varType" mixed="true">
        <xs:sequence minOccurs="0" maxOccurs="unbounded">
            <xs:element name="var" type="varType"/>
        </xs:sequence>
        <!-- ... -->
    </xs:complexType>
<!-- ... -->

Despite of the nested <var /> definition in xsd, it is not fully configured in the \Magento\Framework\Config\View, that is, /view/vars/var does not support nested <var />.

// \Magento\Framework\Config\View::getIdAttributes
protected function getIdAttributes()
    {
        $idAttributes = [
            '/view/vars' => 'module',
+           '/view/vars/(var/)*var' => 'name',
-            '/view/vars/var' => 'name',
            '/view/exclude/item' => ['type', 'item'],
        ];
        foreach ($this->xpath as $attribute) {
            if (is_array($attribute)) {
                foreach ($attribute as $key => $id) {
                    if (count($id) > 1) {
                        $idAttributes[$key] = array_values($id);
                    } else {
                        $idAttributes[$key] = array_shift($id);
                    }
                }
            }
        }
        return $idAttributes;
    }

Bug reproduction

Steps

  1. Create a module with a default etc/view.xml which contain more than one nested var

    <!-- Awesome_Module/etc/view.xml -->
    <?xml version="1.0"?>
    <view xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Config/etc/view.xsd">
        <vars module="Awesome_Module">
            <var name="awesome">
                <var name="first">default</var>
                <var name="second">defatul</var>
            </var>
        </vars>
    </view>
  2. In the local view.xml, overwrite the default settings.

    <!-- app/design/frontend/Awesome/theme/etc/view.xml -->
    ...
    +    <vars module="Awesome_Module">
    +        <var name="awesome">
    +            <var name="first">This is awesome</var>
    +            <var name="second">This is also awesome</var>
    +        </var>
    +    </vars>
    ...
  3. Call \Magento\Framework\View\Element\AbstractBlock::getVar from anywhere

<?php echo $block->getVar('awesome/first'); ?>

Expect Result

This is awesome

Actual Result

Exception #0 (Magento\Framework\Exception\LocalizedException): More than one node matching the query: /view/vars[@module='Awesome_Module']/var[@name='awesome']/var

Workaround

For the time being, a workaround can be applied while this PR or other relevant fix is waiting to merge. Add below fragment into any di.xml

    <!-- Workaround to fix nested <var /> merge issue -->
    <type name="Magento\Framework\Config\View">
        <arguments>
            <argument name="xpath" xsi:type="array">
                <item name="vars" xsi:type="array">
                    <item name="/view/vars/(var/)*var" xsi:type="array">
                        <item name="id" xsi:type="string">name</item>
                    </item>
                </item>
            </argument>
        </arguments>
    </type>

@okorshenko
Copy link
Contributor

@torreytsui thank you for preparing excellent Pull Request. Your contribution successfully merged to develop branch! Thank you

@torreytsui torreytsui deleted the patch-3 branch April 12, 2017 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants