From 9627921cb29b09f60ad00d7573a4f4f01a91d7dd Mon Sep 17 00:00:00 2001 From: Wilson Lin Date: Sat, 25 Jul 2020 13:22:25 +1000 Subject: [PATCH] Do not minify non-JS script data --- bench/README.md | 2 +- src/tests/mod.rs | 2 ++ src/unit/script.rs | 4 ++-- src/unit/tag.rs | 17 ++++++++++++----- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/bench/README.md b/bench/README.md index 75d7571..c70c367 100644 --- a/bench/README.md +++ b/bench/README.md @@ -42,7 +42,7 @@ Since speed depends on the input, speed charts show performance relative to the The settings used for each minifier can be found in [minifiers.js](./minifiers.js). Some settings to note: - CSS minification is disabled for all, as minify-html currently does not support CSS minification (coming soon). -- To increase fairness, all minifiers use esbuild for JS minification, and do so asynchronously and in parallel, similar to how minify-html works. +- All minifiers are configured to use esbuild for JS minification asynchronously and in parallel, similar to how minify-html works. - `conservativeCollapse` is enabled for html-minifier as otherwise some whitespace would be unsafely removed with side affects. minify-html can safely remove whitespace with context if configured properly. ## Running diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 264caaf..e45931a 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -168,6 +168,7 @@ fn test_script_type_attr_value_removal() { eval(b"", b""); eval(b"", b""); eval(b"", b""); + eval(b"", b""); } #[test] @@ -304,4 +305,5 @@ fn test_js_minification() { "#, b""); + eval_with_js_min(b"", b""); } diff --git a/src/unit/script.rs b/src/unit/script.rs index 652681b..c8e32a3 100644 --- a/src/unit/script.rs +++ b/src/unit/script.rs @@ -25,7 +25,7 @@ lazy_static! { }; } -pub fn process_script(proc: &mut Processor, cfg: &Cfg) -> ProcessingResult<()> { +pub fn process_script(proc: &mut Processor, cfg: &Cfg, js: bool) -> ProcessingResult<()> { #[cfg(feature = "js-esbuild")] let start = Checkpoint::new(proc); loop { @@ -35,7 +35,7 @@ pub fn process_script(proc: &mut Processor, cfg: &Cfg) -> ProcessingResult<()> { // `process_tag` will require closing tag. if proc.m(IsSeq(b" TagType::Script, + let mut tag_type = match &proc[tag_name] { + // Unless non-JS MIME `type` is provided, `script` tags contain JS. + b"script" => TagType::ScriptJs, b"style" => TagType::Style, _ => TagType::Other, }; @@ -153,13 +155,17 @@ pub fn process_tag(proc: &mut Processor, cfg: &Cfg, ns: Namespace, mut prev_sibl let ProcessedAttr { name, typ, value } = process_attr(proc, ns, tag_name)?; match (tag_type, &proc[name]) { - (TagType::Script, b"type") => { + // NOTE: We don't support multiple `type` attributes, so can't go from ScriptData => ScriptJs. + (TagType::ScriptJs, b"type") => { // It's JS if the value is empty or one of `JAVASCRIPT_MIME_TYPES`. let script_tag_type_is_js = value .filter(|v| !JAVASCRIPT_MIME_TYPES.contains(&proc[*v])) .is_none(); if script_tag_type_is_js { erase_attr = true; + } else { + // Tag does not contain JS, don't minify JS. + tag_type = TagType::ScriptData; }; } (_, name) => { @@ -203,7 +209,8 @@ pub fn process_tag(proc: &mut Processor, cfg: &Cfg, ns: Namespace, mut prev_sibl }; match tag_type { - TagType::Script => process_script(proc, cfg)?, + TagType::ScriptData => process_script(proc, cfg, false)?, + TagType::ScriptJs => process_script(proc, cfg, true)?, TagType::Style => process_style(proc)?, _ => process_content(proc, cfg, child_ns, Some(tag_name))?, };