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.1.0] HTML minification problem with php tag with a comment and no space at the end #5316

Closed
hostep opened this issue Jun 28, 2016 · 26 comments
Assignees
Labels
Area: Frontend bug report Event: kiev-cd Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Partner: ISM eCompany Pull Request is created by partner ISM eCompany Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@hostep
Copy link
Contributor

hostep commented Jun 28, 2016

Steps to reproduce

  1. Install Magento 2.1.0 using composer
  2. Create a theme and a template file which contains this:
<?php //if ($showDescription):?>
    <div class="product description product-item-description">
        <?php /* @escapeNotVerified */ echo $_helper->productAttribute($_product, $_product->getShortDescription(), 'short_description') ?>
    </div>
<?php //endif; ?>
  1. Turn on html minification in the backend

Expected result

  1. Page is rendered properly without errors

Actual result

  1. Error message appears: Parse error: syntax error, unexpected '<'.

The minified php/html file was turned into:

<?php <div class="product description product-item-description"><?php /* @escapeNotVerified */ echo $_helper->productAttribute($_product, $_product->getShortDescription(), 'short_description') ?></div><?php ?>

Temporary solution

Add a space at the end and it works again, like this:

<?php //if ($showDescription): ?>
@hostep
Copy link
Contributor Author

hostep commented Jul 8, 2016

Any updates on this?

@guz-anton, @okorshenko, @piotrekkaminski: since you guys were involved in another html minification issue, I'm including you in this conversation.

@guz-anton guz-anton self-assigned this Jul 11, 2016
@guz-anton
Copy link
Contributor

Hi @hostep ,
We are going to find solution under MAGETWO-55356.

@m0zziter
Copy link
Contributor

Experienced a similar issue when I installed https://github.com/algolia/algoliasearch-magento-2.

Except the problem line in my case was this line: https://github.com/algolia/algoliasearch-magento-2/blob/master/view/frontend/templates/internals/commonjs.phtml#L41

When html minification is turned on that line looks like
if ($path != '') { $path .= ' }

But I'd expect it to look like
if ($path != '') { $path .= ' /// '; }

Here's a gist of the entire file: https://gist.github.com/m0zziter/b7c2f00afd3684e0642060d19617ab2f

@peec
Copy link

peec commented Sep 2, 2016

Same problem. 2.1.1.

@MomotenkoNatalia MomotenkoNatalia added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report labels Sep 6, 2016
@jmtakahashi
Copy link

This issue hasn't been closed yet and it doesn't seem like there has been a fix to the Minify HTML problem. @guz-anton @okorshenko @piotrekkaminski is there any update on if this issue has been fixed any of the 2.1 release? Thanks.

@ooples
Copy link

ooples commented Apr 24, 2017

I'm using 2.1.6 and I'm having the same issues with html minimication

@chenshanmin
Copy link

Does anyone has a solution?

@hostep
Copy link
Contributor Author

hostep commented Jun 6, 2017

Yes: add a space at the end, like I mention in my initial post under 'temporary solution' ;)

@chenshanmin
Copy link

chenshanmin commented Jun 6, 2017

@hostep maybe that is the best way now,but my project is a secondary development, how can I find all of the End tag :'?>' and add a space... minification library is vendor/magento/framework/View/Template/Html/Minifier.php
right? can I rewrite it?

@pantaoran
Copy link

pantaoran commented Jun 13, 2017

@hostep You have to be joking with your temporary solution. I just ran a regex to find all offending occurrencies in core phtml templates. It found 1109 cases within app/code/Magento.

@chenshanmin I guess it would be possible to try and do this with regular expressions search & replace.
I used the regex [^\s]\?> to search for any non-whitespace characters followed by a php closing tag. You could modify that to insert a space, for example with "sed" on bash.

@hostep
Copy link
Contributor Author

hostep commented Jun 13, 2017

@pantaoran: it's only for php tags with php code which is commented out in phtml files. I haven't searched the Magento core code if there are offending files like that, in our case it was a file in our custom theme.
I don't think your regex searches for commented out code?

Although I agree that all phtml files which contain {something}?> should be replaced with {something} ?> (including a space) to have everything consistent.

@pantaoran
Copy link

pantaoran commented Jun 13, 2017

Apologies, I should have read it more carefully. Yes, my regex searches for any closing tags and does not consider the presence of comments, and it is therefore irrelevant here.

The following regex could work instead: \/\/.*[^\s]\?>
With this I found various occurrencies in third party modules and themes, but none in Magento core.

@chenshanmin
Copy link

I repeated all php tags Ha ha ha

@pniel-cohen
Copy link

I used @pantaoran 's regex to find & fix a few template files, but the minified HTML is still broken.
Any other idea on how to debug this issue?

@franckgarnier21
Copy link

Any fix in Magento 2.2RC ?

@magento-engcom-team magento-engcom-team added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Area: Frontend Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed labels Sep 11, 2017
@magento-engcom-team magento-engcom-team added 2.1.x Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed labels Oct 31, 2017
@magento-engcom-team
Copy link
Contributor

@hostep, thank you for your report.
We've created internal ticket(s) MAGETWO-83074 to track progress on the issue.

@gabs77
Copy link

gabs77 commented Mar 14, 2018

When we have /// in the data:image/gif;base64, the minify html remove all the datas in the image.

Ex:
data:image/gif;base64,R0lGODlhEAAQAPQAAP///wAAAPDw8IqKiuDg4EZGRnp6egAAAFhYWCQkJKysrL6+vhQUFJycnAQEBDY2NmhoaAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACH/C05FVFNDQVBFMi4wAwEAAAAh/hpDcmVhdGVkIHdpdGggYWpheGxvYWQuaW5mbwAh+QQJCgAAACwAAAAAEAAQAAAFdyAgAgIJIeWoAkRCCMdBkKtIHIngyMKsErPBYbADpkSCwhDmQCBethRB6Vj4kFCkQPG4IlWDgrNRIwnO4UKBXDufzQvDMaoSDBgFb886MiQadgNABAokfCwzBA8LCg0Egl8jAggGAA1kBIA1BAYzlyILczULC2UhACH5BAkKAAAALAAAAAAQABAAAAV2ICACAmlAZTmOREEIyUEQjLKKxPHADhEvqxlgcGgkGI1DYSVAIAWMx+lwSKkICJ0QsHi9RgKBwnVTiRQQgwF4I4UFDQQEwi6/3YSGWRRmjhEETAJfIgMFCnAKM0KDV4EEEAQLiF18TAYNXDaSe3x6mjidN1s3IQAh+QQJCgAAACwAAAAAEAAQAAAFeCAgAgLZDGU5jgRECEUiCI+yioSDwDJyLKsXoHFQxBSHAoAAFBhqtMJg8DgQBgfrEsJAEAg4YhZIEiwgKtHiMBgtpg3wbUZXGO7kOb1MUKRFMysCChAoggJCIg0GC2aNe4gqQldfL4l/Ag1AXySJgn5LcoE3QXI3IQAh+QQJCgAAACwAAAAAEAAQAAAFdiAgAgLZNGU5joQhCEjxIssqEo8bC9BRjy9Ag7GILQ4QEoE0gBAEBcOpcBA0DoxSK/e8LRIHn+i1cK0IyKdg0VAoljYIg+GgnRrwVS/8IAkICyosBIQpBAMoKy9dImxPhS+GKkFrkX+TigtLlIyKXUF+NjagNiEAIfkECQoAAAAsAAAAABAAEAAABWwgIAICaRhlOY4EIgjH8R7LKhKHGwsMvb4AAy3WODBIBBKCsYA9TjuhDNDKEVSERezQEL0WrhXucRUQGuik7bFlngzqVW9LMl9XWvLdjFaJtDFqZ1cEZUB0dUgvL3dgP4WJZn4jkomWNpSTIyEAIfkECQoAAAAsAAAAABAAEAAABX4gIAICuSxlOY6CIgiD8RrEKgqGOwxwUrMlAoSwIzAGpJpgoSDAGifDY5kopBYDlEpAQBwevxfBtRIUGi8xwWkDNBCIwmC9Vq0aiQQDQuK+VgQPDXV9hCJjBwcFYU5pLwwHXQcMKSmNLQcIAExlbH8JBwttaX0ABAcNbWVbKyEAIfkECQoAAAAsAAAAABAAEAAABXkgIAICSRBlOY7CIghN8zbEKsKoIjdFzZaEgUBHKChMJtRwcWpAWoWnifm6ESAMhO8lQK0EEAV3rFopIBCEcGwDKAqPh4HUrY4ICHH1dSoTFgcHUiZjBhAJB2AHDykpKAwHAwdzf19KkASIPl9cDgcnDkdtNwiMJCshACH5BAkKAAAALAAAAAAQABAAAAV3ICACAkkQZTmOAiosiyAoxCq+KPxCNVsSMRgBsiClWrLTSWFoIQZHl6pleBh6suxKMIhlvzbAwkBWfFWrBQTxNLq2RG2yhSUkDs2b63AYDAoJXAcFRwADeAkJDX0AQCsEfAQMDAIPBz0rCgcxky0JRWE1AmwpKyEAIfkECQoAAAAsAAAAABAAEAAABXkgIAICKZzkqJ4nQZxLqZKv4NqNLKK2/Q4Ek4lFXChsg5ypJjs1II3gEDUSRInEGYAw6B6zM4JhrDAtEosVkLUtHA7RHaHAGJQEjsODcEg0FBAFVgkQJQ1pAwcDDw8KcFtSInwJAowCCA6RIwqZAgkPNgVpWndjdyohACH5BAkKAAAALAAAAAAQABAAAAV5ICACAimc5KieLEuUKvm2xAKLqDCfC2GaO9eL0LABWTiBYmA06W6kHgvCqEJiAIJiu3gcvgUsscHUERm+kaCxyxa+zRPk0SgJEgfIvbAdIAQLCAYlCj4DBw0IBQsMCjIqBAcPAooCBg9pKgsJLwUFOhCZKyQDA3YqIQAh+QQJCgAAACwAAAAAEAAQAAAFdSAgAgIpnOSonmxbqiThCrJKEHFbo8JxDDOZYFFb+A41E4H4OhkOipXwBElYITDAckFEOBgMQ3arkMkUBdxIUGZpEb7kaQBRlASPg0FQQHAbEEMGDSVEAA1QBhAED1E0NgwFAooCDWljaQIQCE5qMHcNhCkjIQAh+QQJCgAAACwAAAAAEAAQAAAFeSAgAgIpnOSoLgxxvqgKLEcCC65KEAByKK8cSpA4DAiHQ/DkKhGKh4ZCtCyZGo6F6iYYPAqFgYy02xkSaLEMV34tELyRYNEsCQyHlvWkGCzsPgMCEAY7Cg04Uk48LAsDhRA8MVQPEF0GAgqYYwSRlycNcWskCkApIyEAOwAAAAAAAAAAAA==

Become : data:image/gif;base64,R0lGODlhEAAQAPQAAP

We can exclude this case in the minify

@ishakhsuvarov ishakhsuvarov added Award: advanced and removed Event: distributed-cd Distributed Contribution Day labels Apr 13, 2018
@ghost
Copy link

ghost commented Apr 24, 2018

Hi folks, any update on this? will this be fixed in 2.3 ?

@RahulKachhadia
Copy link
Member

I am working on this at #dmcdindia

@magento-engcom-team
Copy link
Contributor

@rahul-kachhadiya thank you for joining. Please accept team invitation here and self-assign the issue.

@lisovyievhenii
Copy link
Contributor

#kiev-cd

lisovyievhenii added a commit to lisovyievhenii/magento2 that referenced this issue Jun 22, 2018
@lisovyievhenii lisovyievhenii mentioned this issue Jun 22, 2018
4 tasks
lisovyievhenii added a commit to lisovyievhenii/magento2 that referenced this issue Jun 23, 2018
@magento-engcom-team
Copy link
Contributor

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

The fix will be available with the upcoming 2.3.0 release.

@magento-engcom-team magento-engcom-team added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Jun 26, 2018
@lisovyievhenii lisovyievhenii added the Partner: ISM eCompany Pull Request is created by partner ISM eCompany label Jul 6, 2018
This was referenced Jul 18, 2018
@sidolov
Copy link
Contributor

sidolov commented Jul 20, 2018

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

The fix will be available with the upcoming 2.1.15 release.

@sidolov sidolov added the Fixed in 2.1.x The issue has been fixed in 2.1 release line label Jul 20, 2018
@VladimirZaets
Copy link
Contributor

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

The fix will be available with the upcoming 2.2.6 release.

@VivekShingala
Copy link

Worked for me. Thanks @hostep

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Frontend bug report Event: kiev-cd Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Partner: ISM eCompany Pull Request is created by partner ISM eCompany Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

No branches or pull requests