Skip to content

Commit

Permalink
eliminate removeCommentsFromCDATA
Browse files Browse the repository at this point in the history
migrate non-overlapping functionalities to minifyJS and minifyCSS
fixes #579
  • Loading branch information
alexlamsl committed Mar 28, 2016
1 parent f5fe726 commit 97c9bfb
Show file tree
Hide file tree
Showing 9 changed files with 93 additions and 71 deletions.
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ How does HTMLMinifier compare to other solutions — [HTML Minifier from Will Pe
| Option | Description | Default |
|--------------------------------|-----------------|---------|
| `removeComments` | [Strip HTML comments](http://perfectionkills.com/experimenting-with-html-minifier/#remove_comments) | `false` |
| `removeCommentsFromCDATA` | [Strip HTML comments from scripts and styles](http://perfectionkills.com/experimenting-with-html-minifier/#remove_comments_from_scripts_and_styles) | `false` |
| `removeCDATASectionsFromCDATA` | [Remove CDATA sections from script and style elements](http://perfectionkills.com/experimenting-with-html-minifier/#remove_cdata_sections) | `false` |
| `collapseWhitespace` | [Collapse white space that contributes to text nodes in a document tree.](http://perfectionkills.com/experimenting-with-html-minifier/#collapse_whitespace) | `false` |
| `conservativeCollapse` | Always collapse to 1 space (never remove it entirely). Must be used in conjunction with `collapseWhitespace=true` | `false` |
Expand Down
1 change: 0 additions & 1 deletion assets/master.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
function getOptions() {
return {
removeComments: byId('remove-comments').checked,
removeCommentsFromCDATA: byId('remove-comments-from-cdata').checked,
removeCDATASectionsFromCDATA: byId('remove-cdata-sections-from-cdata').checked,
collapseWhitespace: byId('collapse-whitespace').checked,
conservativeCollapse: byId('conservative-collapse').checked,
Expand Down
1 change: 0 additions & 1 deletion cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ var mainOptions = {
removeAttributeQuotes: [[false, 'Remove quotes around attributes when possible.']],
removeCDATASectionsFromCDATA: [[false, 'Remove CDATA sections from script and style elements']],
removeComments: [[false, 'Strip HTML comments']],
removeCommentsFromCDATA: [[false, 'Strip HTML comments from scripts and styles']],
removeEmptyAttributes: [[false, 'Remove all attributes with whitespace-only values']],
removeEmptyElements: [[false, 'Remove all elements with empty contents']],
removeOptionalTags: [[false, 'Remove unrequired tags']],
Expand Down
26 changes: 7 additions & 19 deletions dist/htmlminifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -32121,19 +32121,6 @@ function processScript(text, options, currentAttrs) {
return text;
}

var reStartDelimiter = {
// account for js + html comments (e.g.: //<!--)
script: /^\s*(?:\/\/)?\s*<!--.*\n?/,
style: /^\s*<!--\s*/
};
var reEndDelimiter = {
script: /\s*(?:\/\/)?\s*-->\s*$/,
style: /\s*-->\s*$/
};
function removeComments(text, tag) {
return text.replace(reStartDelimiter[tag], '').replace(reEndDelimiter[tag], '');
}

// Tag omission rules from https://html.spec.whatwg.org/multipage/syntax.html#optional-tags
// with the following deviations:
// - retain <body> if followed by <noscript>
Expand Down Expand Up @@ -32398,8 +32385,10 @@ function minifyURLs(text, options) {
}

function minifyJS(text, options) {
var start = text.match(/^\s*<!--.*/);
var code = start ? text.slice(start[0].length).replace(/\n\s*-->\s*$/, '') : text;
try {
return UglifyJS.minify(text, options).code;
return UglifyJS.minify(code, options).code;
}
catch (err) {
log(err);
Expand All @@ -32408,13 +32397,15 @@ function minifyJS(text, options) {
}

function minifyCSS(text, options, inline) {
var start = text.match(/^\s*<!--/);
var style = start ? text.slice(start[0].length).replace(/-->\s*$/, '') : text;
try {
var cleanCSS = new CleanCSS(options);
if (inline) {
return unwrapCSS(cleanCSS.minify(wrapCSS(text)).styles);
return unwrapCSS(cleanCSS.minify(wrapCSS(style)).styles);
}
else {
return cleanCSS.minify(text).styles;
return cleanCSS.minify(style).styles;
}
}
catch (err) {
Expand Down Expand Up @@ -32732,9 +32723,6 @@ function minify(value, options, partialMarkup) {
}
}
if (currentTag === 'script' || currentTag === 'style') {
if (options.removeCommentsFromCDATA) {
text = removeComments(text, currentTag);
}
if (options.removeCDATASectionsFromCDATA) {
text = removeCDATASections(text);
}
Expand Down
2 changes: 1 addition & 1 deletion dist/htmlminifier.min.js

Large diffs are not rendered by default.

4 changes: 0 additions & 4 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ <h1>HTML Minifier <span>(v1.3.1)</span></h1>
</span>
</label>
</li>
<li>
<input type="checkbox" id="remove-comments-from-cdata" checked>
<label for="remove-comments-from-cdata">Also remove comments from scripts and styles</label>
</li>
<li>
<input type="checkbox" id="remove-cdata-sections-from-cdata" checked>
<label for="remove-cdata-sections-from-cdata">Remove CDATA sections from scripts and styles</label>
Expand Down
1 change: 0 additions & 1 deletion sample-cli-config-file.conf
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
{
"removeComments": true,
"removeCommentsFromCDATA": true,
"removeCDATASectionsFromCDATA": true,
"collapseWhitespace": true,
"conservativeCollapse": false,
Expand Down
26 changes: 7 additions & 19 deletions src/htmlminifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -357,19 +357,6 @@ function processScript(text, options, currentAttrs) {
return text;
}

var reStartDelimiter = {
// account for js + html comments (e.g.: //<!--)
script: /^\s*(?:\/\/)?\s*<!--.*\n?/,
style: /^\s*<!--\s*/
};
var reEndDelimiter = {
script: /\s*(?:\/\/)?\s*-->\s*$/,
style: /\s*-->\s*$/
};
function removeComments(text, tag) {
return text.replace(reStartDelimiter[tag], '').replace(reEndDelimiter[tag], '');
}

// Tag omission rules from https://html.spec.whatwg.org/multipage/syntax.html#optional-tags
// with the following deviations:
// - retain <body> if followed by <noscript>
Expand Down Expand Up @@ -634,8 +621,10 @@ function minifyURLs(text, options) {
}

function minifyJS(text, options) {
var start = text.match(/^\s*<!--.*/);
var code = start ? text.slice(start[0].length).replace(/\n\s*-->\s*$/, '') : text;
try {
return UglifyJS.minify(text, options).code;
return UglifyJS.minify(code, options).code;
}
catch (err) {
log(err);
Expand All @@ -644,13 +633,15 @@ function minifyJS(text, options) {
}

function minifyCSS(text, options, inline) {
var start = text.match(/^\s*<!--/);
var style = start ? text.slice(start[0].length).replace(/-->\s*$/, '') : text;
try {
var cleanCSS = new CleanCSS(options);
if (inline) {
return unwrapCSS(cleanCSS.minify(wrapCSS(text)).styles);
return unwrapCSS(cleanCSS.minify(wrapCSS(style)).styles);
}
else {
return cleanCSS.minify(text).styles;
return cleanCSS.minify(style).styles;
}
}
catch (err) {
Expand Down Expand Up @@ -968,9 +959,6 @@ function minify(value, options, partialMarkup) {
}
}
if (currentTag === 'script' || currentTag === 'style') {
if (options.removeCommentsFromCDATA) {
text = removeComments(text, currentTag);
}
if (options.removeCDATASectionsFromCDATA) {
text = removeCDATASections(text);
}
Expand Down
102 changes: 78 additions & 24 deletions tests/minifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,12 +373,12 @@ test('collapsing space in conditional comments', function() {
equal(minify(input, { removeComments: true }), input);
equal(minify(input, { removeComments: true, collapseWhitespace: true }), input);
output = '<!--[if IE 7]>\n\n \t\n \t\t ' +
'<link rel="stylesheet" href="/css/ie7-fixes.css" type="text/css">\n\t' +
'<![endif]-->';
'<link rel="stylesheet" href="/css/ie7-fixes.css" type="text/css">\n\t' +
'<![endif]-->';
equal(minify(input, { removeComments: true, processConditionalComments: true }), output);
output = '<!--[if IE 7]>' +
'<link rel="stylesheet" href="/css/ie7-fixes.css" type="text/css">' +
'<![endif]-->';
'<link rel="stylesheet" href="/css/ie7-fixes.css" type="text/css">' +
'<![endif]-->';
equal(minify(input, {
removeComments: true,
collapseWhitespace: true,
Expand All @@ -391,8 +391,8 @@ test('collapsing space in conditional comments', function() {
equal(minify(input, { removeComments: true }), input);
equal(minify(input, { removeComments: true, collapseWhitespace: true }), input);
output = '<!--[if lte IE 6]>' +
'<p title=" sigificant whitespace ">blah blah</p>' +
'<![endif]-->';
'<p title=" sigificant whitespace ">blah blah</p>' +
'<![endif]-->';
equal(minify(input, {
removeComments: true,
collapseWhitespace: true,
Expand All @@ -401,30 +401,89 @@ test('collapsing space in conditional comments', function() {
});

test('remove comments from scripts', function() {
input = '<script><!--alert(1)--><\/script>';
input = '<script><!--\nalert(1);\n--><\/script>';
equal(minify(input), input);
output = '<script>alert(1)<\/script>';
equal(minify(input, { minifyJS: true }), output);

input = '<script><!--alert(2);--><\/script>';
equal(minify(input), input);
output = '<script><\/script>';
equal(minify(input, { removeCommentsFromCDATA: true }), output);
equal(minify(input, { minifyJS: true }), output);

input = '<script><!--alert(1)<\/script>';
input = '<script><!--alert(3);\n--><\/script>';
equal(minify(input), input);
output = '<script><\/script>';
equal(minify(input, { removeCommentsFromCDATA: true }), output);
equal(minify(input, { minifyJS: true }), output);

input = '<script type="text/javascript"> <!--\nalert("-->"); -->\n\n <\/script>';
output = '<script type="text/javascript">alert("-->");<\/script>';
equal(minify(input, { removeCommentsFromCDATA: true }), output);
input = '<script><!--\nalert(4);--><\/script>';
equal(minify(input), input);
equal(minify(input, { minifyJS: true }), input);

input = '<script><!--alert(5);\nalert(6);\nalert(7);--><\/script>';
equal(minify(input), input);
equal(minify(input, { minifyJS: true }), input);

input = '<script><!--alert(8)<\/script>';
equal(minify(input), input);
output = '<script><\/script>';
equal(minify(input, { minifyJS: true }), output);

input = '<script type="text/javascript"> \n <!--\nalert("-->"); -->\n\n <\/script>';
equal(minify(input), input);
equal(minify(input, { minifyJS: true }), input);

input = '<script type="text/javascript"> \n <!--\nalert("-->");\n -->\n\n <\/script>';
equal(minify(input), input);
output = '<script type="text/javascript">alert("--\\x3e")<\/script>';
equal(minify(input, { minifyJS: true }), output);

input = '<script> // <!-- \n alert(1) // --> <\/script>';
output = '<script> alert(1)<\/script>';
equal(minify(input, { removeCommentsFromCDATA: true }), output);
equal(minify(input), input);
output = '<script>alert(1)<\/script>';
equal(minify(input, { minifyJS: true }), output);
});

test('remove comments from styles', function() {
input = '<style><!--\np.a{background:red}\n--></style>';
equal(minify(input), input);
output = '<style>p.a{background:red}</style>';
equal(minify(input, { minifyCSS: true }), output);

input = '<style><!--p.b{background:red}--></style>';
equal(minify(input), input);
output = '<style>p.b{background:red}</style>';
equal(minify(input, { minifyCSS: true }), output);

input = '<style><!--p.c{background:red}\n--></style>';
equal(minify(input), input);
output = '<style>p.c{background:red}</style>';
equal(minify(input, { minifyCSS: true }), output);

input = '<style><!--\np.d{background:red}--></style>';
equal(minify(input), input);
output = '<style>p.d{background:red}</style>';
equal(minify(input, { minifyCSS: true }), output);

input = '<style><!--p.e{background:red}\np.f{background:red}\np.g{background:red}--></style>';
equal(minify(input), input);
output = '<style>p.e{background:red}p.f{background:red}p.g{background:red}</style>';
equal(minify(input, { minifyCSS: true }), output);

input = '<style>p.h{background:red}<!--\np.i{background:red}\n-->p.j{background:red}</style>';
equal(minify(input), input);
output = '<style>p.h{background:red}<!-- p.i{background:red}-->p.j{background:red}</style>';
equal(minify(input, { minifyCSS: true }), output);

input = '<style type="text/css"><!-- p { color: red } --><\/style>';
output = '<style type="text/css">p { color: red }<\/style>';
equal(minify(input, { removeCommentsFromCDATA: true }), output);
equal(minify(input), input);
output = '<style type="text/css">p{color:red}<\/style>';
equal(minify(input, { minifyCSS: true }), output);

input = '<style type="text/css">p::before { content: "<!--" }<\/style>';
equal(minify(input, { removeCommentsFromCDATA: true }), input);
equal(minify(input), input);
output = '<style type="text/css">p::before{content:"<!--"}<\/style>';
equal(minify(input, { minifyCSS: true }), output);
});

test('remove CDATA sections from scripts/styles', function() {
Expand Down Expand Up @@ -1507,10 +1566,7 @@ test('script minification', function() {
'</script>';
output = '<script>Platform.Mobile.Bootstrap.init(function(){Platform.Mobile.Core.Navigation.go("Login",{error:""})})</script>';

equal(minify(input, {
removeCommentsFromCDATA: true,
minifyJS: true
}), output);
equal(minify(input, { minifyJS: true }), output);
});

test('minification of scripts with different mimetypes', function() {
Expand Down Expand Up @@ -2087,7 +2143,6 @@ test('markups from Angular 2', function() {
removeAttributeQuotes: true,
removeCDATASectionsFromCDATA: true,
removeComments: true,
removeCommentsFromCDATA: true,
removeEmptyAttributes: true,
removeOptionalTags: true,
removeRedundantAttributes: true,
Expand Down Expand Up @@ -2209,7 +2264,6 @@ test('tests from PHPTAL', function() {
removeAttributeQuotes: true,
removeCDATASectionsFromCDATA: true,
removeComments: true,
removeCommentsFromCDATA: true,
removeEmptyAttributes: true,
removeOptionalTags: true,
removeRedundantAttributes: true,
Expand Down

0 comments on commit 97c9bfb

Please sign in to comment.