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

Issue with generated tags #355

Closed
stoyan0v opened this issue Nov 30, 2020 · 2 comments
Closed

Issue with generated tags #355

stoyan0v opened this issue Nov 30, 2020 · 2 comments

Comments

@stoyan0v
Copy link

stoyan0v commented Nov 30, 2020

Hey @tdewolff

Thanks for the awesome minifier.
I'm the author of SG Optimizer plugin, which is used by more than 1 million users and we really love your minifiers and use them for our clients.

We decided to switch our old regex-based HTML minification to this one, but we broke a lot of sites, so we have reverted the change until we fix the issues that have been reported.

One of the main issues was with dynamically generated tags. Here is an example:

<script id='wp-polyfill-js-after'>
	( 'fetch' in window ) || document.write( '<script src="https://site.com/wp-includes/js/dist/vendor/wp-polyfill-fetch.min.js?ver=3.0.0"></scr' + 'ipt>' );( document.contains ) || document.write( '<script src="https://site.com/wp-includes/js/dist/vendor/wp-polyfill-node-contains.min.js?ver=3.42.0"></scr' + 'ipt>' );( window.DOMRect ) || document.write( '<script src="https://site.com/wp-includes/js/dist/vendor/wp-polyfill-dom-rect.min.js?ver=3.42.0"></scr' + 'ipt>' );( window.URL && window.URL.prototype && window.URLSearchParams ) || document.write( '<script src="https://site.com/wp-includes/js/dist/vendor/wp-polyfill-url.min.js?ver=3.6.4"></scr' + 'ipt>' );( window.FormData && window.FormData.prototype.keys ) || document.write( '<script src="https://site.com/wp-includes/js/dist/vendor/wp-polyfill-formdata.min.js?ver=3.0.12"></scr' + 'ipt>' );( Element.prototype.matches && Element.prototype.closest ) || document.write( '<script src="https://site.com/wp-includes/js/dist/vendor/wp-polyfill-element-closest.min.js?ver=2.0.2"></scr' + 'ipt>' );
</script>

When the code above is minified, the end script tag is closed near the following string: </scr' + 'ipt>' and the other part of the script is rendered as html/string.

Here is the command we use: minify --type=html --html-keep-conditional-comments=true --html-keep-quotes=true --html-keep-end-tags=true --html-keep-default-attrvals=true --xml-keep-whitespace=true

Is there a way to exclude some parts of the page(something like exclude list maybe?) or is there a way to ignore such parts of the code?

We would happily contribute to the project in any way possible.

If you have any questions or if there is anything else you want to discuss, you can contact me via email: stanimir.stoyanov@siteground.com

Regards,
Stanimir

@tdewolff
Copy link
Owner

tdewolff commented Nov 30, 2020

Thanks for the issue! I've added a fix to the master branch, please let me know if that works. I'd be very happy to hear of any other website that might break so that those bugs can be fixed, thanks in advance!

PS: I don't think I'd like to add an exclude list or something, I'd prefer fixing the issues for good! ;-)

@stoyan0v
Copy link
Author

stoyan0v commented Dec 1, 2020

WOW, that was fast.
Thanks for fixing this issue. I will test the new master and will let you know if there are any other issues.

Meanwhile, I will prepare a few more issues that we've found.

Regards,
Stanimir

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment