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

[2.3.5] Incorrect html structure after MC-30989 #27920

Closed
hostep opened this issue Apr 21, 2020 · 3 comments
Closed

[2.3.5] Incorrect html structure after MC-30989 #27920

hostep opened this issue Apr 21, 2020 · 3 comments
Assignees
Labels
Fixed in 2.4.x The issue has been fixed in 2.4-develop branch Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Reported on 2.3.5 Indicates original Magento version for the Issue report.

Comments

@hostep
Copy link
Contributor

hostep commented Apr 21, 2020

Preconditions (*)

  1. Magento 2.3.5 or 2.4-develop (I'm using commmit 1e28f35)

Steps to reproduce (*)

  1. Look at this piece of code:
    <?php if ($_item->isSaleable()):?>
    <div class="actions-primary">
    <?php if (!$_item->getTypeInstance()->isPossibleBuyFromList($_item)):?>
    <button
    class="action tocart primary"
    data-mage-init='{"redirectUrl": {"url": "<?= $block->escapeUrl($block->getAddToCartUrl($_item)) ?>"}}' type="button" title="<?= $block->escapeHtmlAttr(__('Add to Cart')) ?>">
    <span><?= $block->escapeHtml(__('Add to Cart')) ?></span>
    </button>
    <?php else :?>
    <?php
    /** @var $viewModel PreparePostData */
    $viewModel = $block->getViewModel();
    $postArray = $viewModel->getPostData(
    $block->escapeUrl($block->getAddToCartUrl($_item)),
    ['product' => $_item->getEntityId()]
    );
    $value = $postArray['data'][ActionInterface::PARAM_NAME_URL_ENCODED];
    ?>
    <form data-role="tocart-form"
    data-product-sku="<?= $block->escapeHtmlAttr($_item->getSku()) ?>"
    action="<?= $block->escapeUrl($block->getAddToCartUrl($_item)) ?>"
    method="post">
    <input type="hidden" name="product"
    value="<?= /* @noEscape */ (int)$_item->getEntityId() ?>">
    <input type="hidden"
    name="<?= /* @noEscape */ ActionInterface::PARAM_NAME_URL_ENCODED?>"
    value="<?= /* @noEscape */ $value ?>">
    <?= $block->getBlockHtml('formkey') ?>
    <button type="submit"
    title="<?= $block->escapeHtmlAttr(__('Add to Cart')) ?>"
    class="action tocart primary">
    <span><?= $block->escapeHtml(__('Add to Cart')) ?></span>
    </button>
    </form>
    <?php endif; ?>
    <?php else:?>
    <?php if ($_item->getIsSalable()):?>
    <div class="stock available">
    <span><?= $block->escapeHtml(__('In stock')) ?></span>
    </div>
    <?php else:?>
    <div class="stock unavailable">
    <span><?= $block->escapeHtml(__('Out of stock')) ?></span>
    </div>
    <?php endif; ?>
    <?php endif; ?>
    </div>
    <?php endif; ?>
  2. Remove a bunch of the inner structure until you have the following remaining:
    <?php if ($showCart):?>
        <?php if ($_item->isSaleable()):?>
            <div class="actions-primary">
        <?php else:?>
        <?php endif; ?>
        </div>
    <?php endif; ?>
  1. Notice that the opening <div> and closing </div> aren't in the same block, which can most likely result in incorrect html structure being outputted.

Expected result (*)

Either:

    <?php if ($showCart):?>
        <?php if ($_item->isSaleable()):?>
            <div class="actions-primary">
            </div>
        <?php else:?>
        <?php endif; ?>
    <?php endif; ?>

Or:

    <?php if ($showCart):?>
        <div class="actions-primary">
            <?php if ($_item->isSaleable()):?>
            <?php else:?>
            <?php endif; ?>
        </div>
    <?php endif; ?>

If I have to guess, it should probably take the second form, as that's the structure what other templates are following as well:

  • <div class="actions-primary">
    <?php if ($item->isSaleable()) :?>
    <form data-role="tocart-form"
    action="<?= $block->escapeUrl($this->helper(Magento\Catalog\Helper\Product\Compare::class)->getAddToCartUrl($item)) ?>"
    method="post">
    <?= $block->getBlockHtml('formkey') ?>
    <button type="submit" class="action tocart primary">
    <span><?= $block->escapeHtml(__('Add to Cart')) ?></span>
    </button>
    </form>
    <?php else :?>
    <?php if ($item->getIsSalable()) :?>
    <div class="stock available"><span><?= $block->escapeHtml(__('In stock')) ?></span></div>
    <?php else :?>
    <div class="stock unavailable"><span><?= $block->escapeHtml(__('Out of stock')) ?></span></div>
    <?php endif; ?>
    <?php endif; ?>
    </div>
  • <div class="actions-primary"<?= strpos($pos, $viewMode . '-primary') ? $escaper->escapeHtmlAttr($position) : '' ?>>
    <?php if ($_product->isSaleable()) :?>
    <?php $postParams = $block->getAddToCartPostParams($_product); ?>
    <form data-role="tocart-form"
    data-product-sku="<?= $escaper->escapeHtml($_product->getSku()) ?>"
    action="<?= $escaper->escapeUrl($postParams['action']) ?>"
    method="post">
    <input type="hidden"
    name="product"
    value="<?= /* @noEscape */ $postParams['data']['product'] ?>">
    <input type="hidden" name="<?= /* @noEscape */ Action::PARAM_NAME_URL_ENCODED ?>"
    value="<?= /* @noEscape */ $postParams['data'][Action::PARAM_NAME_URL_ENCODED] ?>">
    <?= $block->getBlockHtml('formkey') ?>
    <button type="submit"
    title="<?= $escaper->escapeHtmlAttr(__('Add to Cart')) ?>"
    class="action tocart primary">
    <span><?= $escaper->escapeHtml(__('Add to Cart')) ?></span>
    </button>
    </form>
    <?php else :?>
    <?php if ($_product->isAvailable()) :?>
    <div class="stock available"><span><?= $escaper->escapeHtml(__('In stock')) ?></span></div>
    <?php else :?>
    <div class="stock unavailable"><span><?= $escaper->escapeHtml(__('Out of stock')) ?></span></div>
    <?php endif; ?>
    <?php endif; ?>
    </div>
  • ...

Actual result (*)

  1. See points 2 & 3 of steps to reproduce

Discussion

This seems to have been introduced in Magento 2.3.5 and 2.4-develop by MC-30989, the <div class="actions-primary"> was moved inside the <?php if ($_item->isSaleable()):?> while it should probably have been left outside of it.

@m2-assistant
Copy link

m2-assistant bot commented Apr 21, 2020

Hi @hostep. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

  • Summary of the issue
  • Information on your environment
  • Steps to reproduce
  • Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

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

@hostep do you confirm that you were able to reproduce the issue on vanilla Magento instance following steps to reproduce?

  • yes
  • no

@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Apr 21, 2020
@ghost ghost assigned hostep Apr 21, 2020
@sdzhepa
Copy link
Contributor

sdzhepa commented Apr 27, 2020

PR with fix for this issue has been merged #27926

The Issue can be closed

@magento-engcom-team
Copy link
Contributor

Hi @hostep. Thank you for your report.
The issue has been fixed in #27926 by @hostep in 2.4-develop branch
Related commit(s):

The fix will be available with the upcoming 2.4.0 release.

@magento-engcom-team magento-engcom-team added the Fixed in 2.4.x The issue has been fixed in 2.4-develop branch label May 1, 2020
@magento-engcom-team magento-engcom-team added the Reported on 2.3.5 Indicates original Magento version for the Issue report. label Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed in 2.4.x The issue has been fixed in 2.4-develop branch Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Reported on 2.3.5 Indicates original Magento version for the Issue report.
Projects
None yet
Development

No branches or pull requests

3 participants