magento2: HTML minification breaks PHP code

After upgrading from Magento 2.4.1 to 2.4.2 we started getting such an error:

ParseError: syntax error, unexpected ‘<’, expecting elseif (T_ELSEIF) or else (T_ELSE) or endif (T_ENDIF) in /app/m/var/view_preprocessed/pub/static/app/design/frontend/creativestyle/theme-creativeshop/Magento_Catalog/templates/addtocart/button.phtml:1 Stack trace: #0 /app/m/vendor/magento/framework/Interception/Interceptor.php(58): Magento\Framework\View\TemplateEngine\Php->render(Object(Magento\Framework\View\Element\Template\Interceptor), ‘/app/m/var/view…’, Array)

It happens only when html minification is enabled.

This happens for the following file:

<?php
    $buttonType = $block->getButtonType() ?? 'submit';
    $buttonLabel = $block->getAddtocartLabel() ?? __($block->getVar('ajax_addtocart/labels/default', 'Magento_Catalog'));
    $canBeConfigured = $block->getCanBeConfigured();
?>

<div class="cs-addtocart__wrapper">
<button type="<?= $buttonType ?>" title="<?= $buttonLabel ?>"
        class="tocart primary <?= $block->getButtonClass() ?? $block->getVar('ajax_addtocart/css_classes/default_button', 'Magento_Catalog') ?>"
        <?= $block->getButtonParams() ?>
>
    <?php if(!empty($block->getVar('ajax_addtocart/icons/default/path', 'Magento_Catalog'))): ?>
        <?php $variablesPath = 'ajax_addtocart/icons/' . ($canBeConfigured ? 'configure' : 'default'); ?>
        <?= $this->getLayout()
               ->createBlock('MageSuite\ThemeHelpers\Block\Icon')
               ->setIconUrl($block->getVar($variablesPath . '/path', 'Magento_Catalog'))
               ->setCssClass($block->getVar($variablesPath . '/css_class', 'Magento_Catalog'))
               ->setInlined($block->getVar($variablesPath . '/inlined', 'Magento_Catalog'))
               ->setLazyLoaded($block->getVar($variablesPath . '/lazy_loaded', 'Magento_Catalog'))
               ->toHtml();
        ?>
    <?php endif; ?>

    <?php if(!empty($buttonLabel)): ?>
        <strong class="<?= $block->getButtonLabelClass() ?? $block->getVar('ajax_addtocart/css_classes/default_label', 'Magento_Catalog') ?>"><?= $buttonLabel ?></strong>
    <?php endif; ?>

    <?php if($block->getVar('ajax_addtocart/enabled', 'Magento_Catalog')): ?>
        <?php // If you use ajax method for adding products to cart, don't use <span> inside button because Magento will replace its contents with "Adding..." and "Added". Use <strong> instead. ?>
        <?php
            $processingLabel = __($block->getVar('ajax_addtocart/labels/processing', 'Magento_Catalog'));
            $successLabel = __($block->getVar('ajax_addtocart/labels/success', 'Magento_Catalog'));
            $failLabel = __($block->getVar('ajax_addtocart/labels/fail', 'Magento_Catalog'));
        ?>

        <?php // Processing ?>
        <?php include ($block->getTemplateFile('Magento_Catalog::addtocart/loading-indicator.phtml')); ?>
        <?php if(!empty($processingLabel)): ?>
            <strong class="<?= $block->getVar('ajax_addtocart/css_classes/processing_label', 'Magento_Catalog') ?>"><?= $processingLabel ?></strong>
        <?php endif; ?>

        <?php // Done ?>
        <strong class="<?= $block->getVar('ajax_addtocart/css_classes/success_overlay', 'Magento_Catalog') ?>"></strong>
        <?php if(!empty($block->getVar('ajax_addtocart/icons/success/path', 'Magento_Catalog'))): ?>
            <?= $this->getLayout()
                   ->createBlock('MageSuite\ThemeHelpers\Block\Icon')
                   ->setIconUrl($block->getVar('ajax_addtocart/icons/success/path', 'Magento_Catalog'))
                   ->setCssClass($block->getVar('ajax_addtocart/icons/success/css_class', 'Magento_Catalog'))
                   ->setInlined($block->getVar('ajax_addtocart/icons/success/inlined', 'Magento_Catalog'))
                   ->setLazyLoaded($block->getVar('ajax_addtocart/icons/success/lazy_loaded', 'Magento_Catalog'))
                   ->toHtml();
            ?>
        <?php endif; ?>
        <?php if(!empty($successLabel)): ?>
            <strong class="<?= $block->getVar('ajax_addtocart/css_classes/success_label', 'Magento_Catalog') ?>"><?= $successLabel ?></strong>
        <?php endif; ?>

        <?php // Done but failed ?>
        <?php if(!empty($block->getVar('ajax_addtocart/icons/fail/path', 'Magento_Catalog'))): ?>
            <?= $this->getLayout()
                   ->createBlock('MageSuite\ThemeHelpers\Block\Icon')
                   ->setIconUrl($block->getVar('ajax_addtocart/icons/fail/path', 'Magento_Catalog'))
                   ->setCssClass($block->getVar('ajax_addtocart/icons/fail/css_class', 'Magento_Catalog'))
                   ->setInlined($block->getVar('ajax_addtocart/icons/fail/inlined', 'Magento_Catalog'))
                   ->setLazyLoaded($block->getVar('ajax_addtocart/icons/fail/lazy_loaded', 'Magento_Catalog'))
                   ->toHtml();
            ?>
        <?php endif; ?>
        <?php if(!empty($failLabel)): ?>
            <strong class="<?= $block->getVar('ajax_addtocart/css_classes/fail_label', 'Magento_Catalog') ?>"><?= $failLabel ?></strong>
        <?php endif; ?>
    <?php endif; ?>
</button>
</div>

It gets minified to this:

<?php $buttonType = $block->getButtonType() ?? 'submit'; $buttonLabel = $block->getAddtocartLabel() ?? __($block->getVar('ajax_addtocart/labels/default', 'Magento_Catalog')); $canBeConfigured = $block->getCanBeConfigured(); ?> <div class="cs-addtocart__wrapper"><button type="<?= $buttonType ?>" title="<?= $buttonLabel ?>" class="tocart primary <?= $block->getButtonClass() ?? $block->getVar('ajax_addtocart/css_classes/default_button', 'Magento_Catalog') ?>" <?= $block->getButtonParams() ?> ><?php if(!empty($block->getVar('ajax_addtocart/icons/default/path', 'Magento_Catalog'))): ?> <?php $variablesPath = 'ajax_addtocart/icons/' . ($canBeConfigured ? 'configure' : 'default'); ?> <?= $this->getLayout() ->createBlock('MageSuite\ThemeHelpers\Block\Icon') ->setIconUrl($block->getVar($variablesPath . '/path', 'Magento_Catalog')) ->setCssClass($block->getVar($variablesPath . '/css_class', 'Magento_Catalog')) ->setInlined($block->getVar($variablesPath . '/inlined', 'Magento_Catalog')) ->setLazyLoaded($block->getVar($variablesPath . '/lazy_loaded', 'Magento_Catalog')) ->toHtml(); ?> <?php endif; ?> <?php if(!empty($buttonLabel)): ?> <strong class="<?= $block->getButtonLabelClass() ?? $block->getVar('ajax_addtocart/css_classes/default_label', 'Magento_Catalog') ?>"><?= $buttonLabel ?></strong> <?php endif; ?> <?php if($block->getVar('ajax_addtocart/enabled', 'Magento_Catalog')): ?> <?php <?php $processingLabel = __($block->getVar('ajax_addtocart/labels/processing', 'Magento_Catalog')); $successLabel = __($block->getVar('ajax_addtocart/labels/success', 'Magento_Catalog')); $failLabel = __($block->getVar('ajax_addtocart/labels/fail', 'Magento_Catalog')); ?> <?php ?> <?php include ($block->getTemplateFile('Magento_Catalog::addtocart/loading-indicator.phtml')); ?> <?php if(!empty($processingLabel)): ?> <strong class="<?= $block->getVar('ajax_addtocart/css_classes/processing_label', 'Magento_Catalog') ?>"><?= $processingLabel ?></strong> <?php endif; ?> <?php ?> <strong class="<?= $block->getVar('ajax_addtocart/css_classes/success_overlay', 'Magento_Catalog') ?>"></strong> <?php if(!empty($block->getVar('ajax_addtocart/icons/success/path', 'Magento_Catalog'))): ?> <?= $this->getLayout() ->createBlock('MageSuite\ThemeHelpers\Block\Icon') ->setIconUrl($block->getVar('ajax_addtocart/icons/success/path', 'Magento_Catalog')) ->setCssClass($block->getVar('ajax_addtocart/icons/success/css_class', 'Magento_Catalog')) ->setInlined($block->getVar('ajax_addtocart/icons/success/inlined', 'Magento_Catalog')) ->setLazyLoaded($block->getVar('ajax_addtocart/icons/success/lazy_loaded', 'Magento_Catalog')) ->toHtml(); ?> <?php endif; ?> <?php if(!empty($successLabel)): ?> <strong class="<?= $block->getVar('ajax_addtocart/css_classes/success_label', 'Magento_Catalog') ?>"><?= $successLabel ?></strong> <?php endif; ?> <?php ?> <?php if(!empty($block->getVar('ajax_addtocart/icons/fail/path', 'Magento_Catalog'))): ?> <?= $this->getLayout() ->createBlock('MageSuite\ThemeHelpers\Block\Icon') ->setIconUrl($block->getVar('ajax_addtocart/icons/fail/path', 'Magento_Catalog')) ->setCssClass($block->getVar('ajax_addtocart/icons/fail/css_class', 'Magento_Catalog')) ->setInlined($block->getVar('ajax_addtocart/icons/fail/inlined', 'Magento_Catalog')) ->setLazyLoaded($block->getVar('ajax_addtocart/icons/fail/lazy_loaded', 'Magento_Catalog')) ->toHtml(); ?> <?php endif; ?> <?php if(!empty($failLabel)): ?> <strong class="<?= $block->getVar('ajax_addtocart/css_classes/fail_label', 'Magento_Catalog') ?>"><?= $failLabel ?></strong> <?php endif; ?> <?php endif; ?></button></div>

After we’ve split the minified file into several lines, we found that the error comes down to this part:

<?php <?php $processingLabel = __($block->getVar('ajax_addtocart/labels/processing', 'Magento_Catalog'));

The original code for this looks like this:

<?php // If you use ajax method for adding products to cart, don't use <span> inside button because Magento will replace its contents with "Adding..." and "Added". Use <strong> instead. ?>
        <?php
            $processingLabel = __($block->getVar('ajax_addtocart/labels/processing', 'Magento_Catalog'));

So as you can see, it incorrectly replaces <?php // ... ?> <?php with <?php <?php .

It should either drop the php comment line completely, or leave it untouched with its’ closing ?> php tag, which it currently “swallows”.

Preconditions (*)

  1. Magento 2.4.2
  2. Theme that is based on https://github.com/magesuite/theme-creativeshop/blob/106aeb7/src/Magento_Catalog/templates/addtocart/button.phtml .

Steps to reproduce (*)

Run magento with that file in a html minification mode.

Current workarounds

Change your .phtml files so that they do not contain <?php // ... ?> lines at all, or at least, change their comment format to a different one (e.g. /* ... */ asterisk one).

  • Severity: S0 - Affects critical data or functionality and leaves users without workaround.
  • Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
  • Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
  • Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
  • Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.

About this issue

  • Original URL
  • State: open
  • Created 3 years ago
  • Reactions: 5
  • Comments: 34 (24 by maintainers)

Commits related to this issue

Most upvoted comments

The problem comes from this set of commits, but it is likely that any attempt to fix it will in turn bring some other problems, because regexes are not the right tool for the job.

Could a solution based for example on nikic/php-parser be considered instead?

+5
blmage on May 12, 2021

But it’s still a bug in core Magento’s html minifier, people can waste many hours in researching why at a certain point their perfectly valid custom code is getting corrupt by the html minifier…

+4
hostep on May 12, 2021

@mrtuvn: I strongly disagree. This is valid PHP code. The html minifier that Magento uses should be able to handle this. It doesn’t matter if such code is part of custom code or not.

Also: please don’t add a label “Cannot Reproduce” if you haven’t properly tested this.

+3
hostep on May 12, 2021

And as you mentioned best solution is use great theme like Hyva and spent much time for optimize site performance, deployment process and so on.

We already use Hyva 😉

@DanielRuf Magento has ton of files and I believe that including minified files is better solution than minify result html

Well, as you can see specific corner cases like optional trailing semicolon and others are not (yet) covered.

PS. Regex is not a vodoo magic if you know how work with it 😉

Doesn’t seem to be the case here as the reverted commits are just trying around with regex (same commit message) and have no clear explanation what they should do and use very complex code which has ultimately lead to these issues.

One example to show that there worked people with not that much experience:

<<<([A-z]+).*?\1;/ims

The i flag is not needed. See https://regex101.com/r/jECWPk/1

Sorry to say that but this minifier is just too complex, introduces too many issues, minification in general should not work like this (PHP does not run faster by trying to compress the HTML code) and the code is not really documented (especially the regular expressions) and extremely hard to review and understand.

You have my full respect if you understand and if you can explain in simple words what all these regular expressions in this file do, what they cover (especially corner cases) and what the last changes did in detail.

+2
DanielRuf on Nov 14, 2021

Now seriously, Magento 2 should only compress / minify files after they were processed by PHP (source => data => result as static html => compress / minify).

Please don’t minify / touch source files which contain PHP code. That makes absolutely no sense and leads to such (corner) cases where it leads to problems.

Would Hugo or any other static site builder do the same then this would lead to broken sourcecode.

+2
DanielRuf on Nov 12, 2021

How can it be 3rd-party modules fault that valid PHP code is not properly handled by the html minifier? Any and all valid PHP code should be handled by the minifier, if it cannot it is clearly a bug.

Just because 3rd party worked around it by modifying their source it does not mean the minifier is bug-free. I’ve seen at least 3 modules that ran into this bug with newer magento versions.

Edit: I agree with hostep: It is very difficult to debug and not always catched by CI/CD processes, especially if it happens in templates that are not easily tested like checkout or other.

+2
davidhiendl on May 12, 2021

hello,

as an addition to this discussion, I had the case of a phtml template file containing an inline comment starting with a # instead of a classic //

once minified, as expected, all PHP code between the # and the end of the PHP block was commented, leading to errors

I can see in https://github.com/magento/magento2/blob/2.4-develop/lib/internal/Magento/Framework/View/Template/Html/Minifier.php that // is handled but not #

+1
aurmil on Jun 13, 2023

@DanielRuf: does #33016 solve your issue?

We should get rid of this weird template-minifier in general.

I’m currently testing this patch for vendor/magento/framework:

diff --git a/View/Template/Html/Minifier.php b/View/Template/Html/Minifier.php
--- a/View/Template/Html/Minifier.php
+++ b/View/Template/Html/Minifier.php
@@ -3,7 +3,6 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
-declare(strict_types=1);

 namespace Magento\Framework\View\Template\Html;

@@ -141,10 +140,10 @@
                         . '(?:<(?>textarea|pre|script)\b|\z))#',
                         ' ',
                         preg_replace(
-                            '#(?<!:|\\\\|\'|"|/)//(?!/)(?!\s*\<\!\[)(?!\s*]]\>)[^\n\r]*#',
+                            '#(?<!:|\\\\|\'|")//(?!\s*\<\!\[)(?!\s*]]\>)[^\n\r]*#',
                             '',
                             preg_replace(
-                                '#(?<!:|\'|")//[^\n\r<]*(\?\>)#',
+                                '#(?<!:|\'|")//[^\n\r]*(\?\>)#',
                                 ' $1',
                                 preg_replace(
                                     '#(?<!:)//[^\n\r]*(\<\?php)[^\n\r]*(\s\?\>)[^\n\r]*#',

I will test this patch now and will look at the linked PR afterwards. But in general: stop doing this regex stuff, this is black magic and voodoo at the least.

+1
DanielRuf on Nov 12, 2021

We got similar issue with the file. Had to remove problem part of file

vendor/dotpay/magento2-payment/view/adminhtml/templates/system/config/information.phtml
+1
ilnytskyi on Feb 24, 2021

I’m not sure but this problem can be reproduce in vanilla code base magento without any 3rd-party modules/themes ? Maybe with approach php comments like this in one place will break other code after it. Should avoid use double backslash in comment , especially when have 2 php block sit next to each other

This code is a valid php code - minifying it shouldn’t break it.

Not sure if it can be reproduced in vanilla magento, unfortunately I don’t have time now to set one up.

+1
jtomaszewski on Feb 14, 2021
Read more comments on GitHub
← magento2: HTML characters not rendered in thumbnail popup
magento2: Huge "product_data_storage" in localStorage hangs the shop →