minifyCSS and custom fragments

[felipemsantana]

Hi, there is a bug using minifyCSS and ignoreCustomFragments, it seems that clean-css is processing fragments that should be ignored

Code:

minify('<style>body{color: red;}{{ foo }}</style>', {
  minifyCSS: true,
  ignoreCustomFragments: [/{{.+}}/]
});

Expected output:

<style>body{color:red}{{ foo }}</style>

Actual output:

<style>body{color:red}</style>

For comparison, minifyJS works as expected:

minify('<script>var a = true;{{ foo }}</script>', {
  minifyJS: true,
  ignoreCustomFragments: [/{{.+}}/]
});
// '<script>var a=!0;{{ foo }}</script>'

Using version 3.5.16

[alexlamsl]

Pull request to fix this is welcome.

[felipemsantana]

Could you give me some directions of where should I look for to fix this?

[alexlamsl]

So the way custom fragments are handled in html-minifier is we replaced them with a random string before passing the content on for processing.

html-minifier/src/htmlminifier.js

Lines 871 to 892 in 9ecabca

	var customFragments = options.ignoreCustomFragments.map(function(re) {
	return re.source;
	});
	if (customFragments.length) {
	var reCustomIgnore = new RegExp('\\s*(?:' + customFragments.join('|') + ')+\\s*', 'g');
	// temporarily replace custom ignored fragments with unique attributes
	value = value.replace(reCustomIgnore, function(match) {
	if (!uidAttr) {
	uidAttr = uniqueId(value);
	uidPattern = new RegExp('(\\s*)' + uidAttr + '([0-9]+)(\\s*)', 'g');
	if (options.minifyCSS) {
	options.minifyCSS = escapeFragments(options.minifyCSS);
	}
	if (options.minifyJS) {
	options.minifyJS = escapeFragments(options.minifyJS);
	}
	}
	var token = uidAttr + ignoredCustomMarkupChunks.length;
	ignoredCustomMarkupChunks.push(/^(\s*)[\s\S]*?(\s*)$/.exec(match));
	return '\t' + token + '\t';
	});
	}

Recent versions of clean-css might have changed its behaviour when encountering said tokens.
Drop a console.log(text) here to see what is being passed onto clean-css:

html-minifier/src/htmlminifier.js

Line 664 in 9ecabca

text = new CleanCSS(value).minify(text).styles;

[alexlamsl]

Here's a bunch of tests to check against the fix for this issue:

diff --git a/tests/minifier.js b/tests/minifier.js
index fe425f5..e4de955 100644
--- a/tests/minifier.js
+++ b/tests/minifier.js
@@ -2333,6 +2333,74 @@ QUnit.test('style attribute minification', function(assert) {
   assert.equal(minify(input, { minifyCSS: true }), output);
 });

+QUnit.test('minification of style with custom fragments', function(assert) {
+  var input;
+
+  input = '<style><?foo?></style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+
+  input = '<style>\t<?foo?>\t</style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+
+  input = '<style><?foo?>{color:red}</style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+
+  input = '<style>\t<?foo?>\t{color:red}</style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+
+  input = '<style>body{<?foo?>}</style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+
+  input = '<style>body{\t<?foo?>\t}</style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+
+  input = '<style><?foo?>body{color:red}</style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+
+  input = '<style>\t<?foo?>\tbody{color:red}</style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+
+  input = '<style>body{<?foo?>color:red}</style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+
+  input = '<style>body{\t<?foo?>\tcolor:red}</style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+
+  input = '<style>body{color:red<?foo?>}</style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+
+  input = '<style>body{color:red\t<?foo?>\t}</style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+
+  input = '<style>body{color:red;<?foo?>}</style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+
+  input = '<style>body{color:red;\t<?foo?>\t}</style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+
+  input = '<style>body{color:red}<?foo?></style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+
+  input = '<style>body{color:red}\t<?foo?>\t</style>';
+  assert.equal(minify(input), input);
+  assert.equal(minify(input, { minifyCSS: true }), input);
+});
+
 QUnit.test('url attribute minification', function(assert) {
   var input, output;