From f1fa169507cb8645e099b84cc4a69d4fd1fe4c6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Go=C5=82e=CC=A8biowski-Owczarek?= Date: Thu, 15 Jul 2021 15:44:38 +0200 Subject: [PATCH 1/2] Manipulation: Don't remove HTML comments from scripts When evaluating scripts, jQuery strips out the possible wrapping HTML comment and a CDATA section. However, all supported browsers are already doing that when loading JS via appending a script tag to the DOM which is how we've been doing `jQuery.globalEval` since jQuery 3.0.0. jQuery logic was imperfect, e.g. it just stripped the `` markers, respectively at the beginning or the end of the script contents. However, browsers are also stripping everything following those markers in the same line, treating them as single-line comments delimiters; this is now also mandated by ECMAScript 2015 in Annex B. Instead of fixing the jQuery logic, just let the browser do its thing. We still need to strip CDATA sections for backwards compatibility. This shouldn't be needed as in XML documents they're already not visible when inspecting element contents and in HTML documents they have no meaning but we're preserving that logic for backwards compatibility. This will be removed completely in 4.0. Fixes gh-4904 --- src/manipulation.js | 9 ++++++++- test/unit/manipulation.js | 13 ++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/manipulation.js b/src/manipulation.js index dec21ea0b4..9e32961eaa 100644 --- a/src/manipulation.js +++ b/src/manipulation.js @@ -40,7 +40,8 @@ var // checked="checked" or checked rchecked = /checked\s*(?:[^=]|=\s*.checked.)/i, - rcleanScript = /^\s*\s*$/g; + + rcleanScript = /^\s*\s*$/g; // Prefer a tbody over its parent table for containing new rows function manipulationTarget( elem, content ) { @@ -195,6 +196,12 @@ function domManip( collection, args, callback, ignored ) { }, doc ); } } else { + + // Clean the CDATA sections from script contents. This shouldn't be + // needed as in XML documents they're already not visible when + // inspecting element contents and in HTML documents they have no + // meaning but we're preserving that logic for backwards compatibility. + // This will be removed completely in 4.0. See gh-4904. DOMEval( node.textContent.replace( rcleanScript, "" ), node, doc ); } } diff --git a/test/unit/manipulation.js b/test/unit/manipulation.js index 22e9ae7470..3fe49aae9b 100644 --- a/test/unit/manipulation.js +++ b/test/unit/manipulation.js @@ -2268,7 +2268,7 @@ QUnit.test( "domManip plain-text caching (trac-6779)", function( assert ) { QUnit.test( "domManip executes scripts containing html comments or CDATA (trac-9221)", function( assert ) { - assert.expect( 3 ); + assert.expect( 4 ); jQuery( [ "" ].join( "\n" ) ).appendTo( "#qunit-fixture" ); + + // ES2015 in Annex B requires HTML-style comment delimiters (``) to act as + // single-line comment delimiters; i.e. they should be treated as `//`. + // See gh-4904 + jQuery( [ + "" + ].join( "\n" ) ).appendTo( "#qunit-fixture" ); } ); testIframe( From a0bcaca48de132b7faf77f4f9ef0a4212c8665be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Go=C5=82=C4=99biowski-Owczarek?= Date: Mon, 19 Jul 2021 19:02:55 +0200 Subject: [PATCH 2/2] Update src/manipulation.js Co-authored-by: Richard Gibson --- src/manipulation.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/manipulation.js b/src/manipulation.js index 9e32961eaa..64a8785e0d 100644 --- a/src/manipulation.js +++ b/src/manipulation.js @@ -197,7 +197,7 @@ function domManip( collection, args, callback, ignored ) { } } else { - // Clean the CDATA sections from script contents. This shouldn't be + // Unwrap a CDATA section containing script contents. This shouldn't be // needed as in XML documents they're already not visible when // inspecting element contents and in HTML documents they have no // meaning but we're preserving that logic for backwards compatibility.