middleman / middleman-minify-html

A HTML whitespace minifier for Middleman
https://middlemanapp.com/
MIT License
77 stars 13 forks source link

Don't enable options by default #30

Open MSch opened 9 years ago

MSch commented 9 years ago

Just got hit by https://github.com/middleman/middleman-minify-html/issues/24 because I expected no potentially unsafe options to be enabled by default.

Furthermore the https://github.com/paolochiodi/htmlcompressor#usage documentation has different "basic and safe default options" than middleman-minify-html

tdreyno commented 9 years ago

Can you explain further?

MSch commented 9 years ago

Yes, I turned on minify_html, enabling remove_comments, expecting this to only remove comments not perform other, potentially unsafe, actions.

The htmlcompressor project by default does not enable any options for precisely this reason.

middleman-minify-html does enable options by default which do break websites.

rmoriz commented 9 years ago

It removes the http://-protocol part in every outgoing link by default. When you deploy your middleman site on https with minify-html enabled, all http-only links will break.

see:

=> go to https://middlemanapp.com/ => click on "Thomas Reynolds" (near the footer)

=> no protocol specified => https assumed (like https://middlemanapp.com/) => but https not configured (in your case a wrong certificate warning)

Update: Okay looks like in the case of https://middlemanapp.com/ it's caused manually in https://github.com/middleman/middleman-guides/blob/master/source/localizable/_footer.html.erb however minify-html does this automatically by default and would have broken the link the same way

Arcovion commented 9 years ago

Further down in their readme is:

The middleware uses a little more aggressive options by default:

Those are the defaults we're using, so please let https://github.com/paolochiodi/htmlcompressor know about this too.