Manipulation: Don't remove HTML comments from scripts

mgol · web-flow · commit 2f8f39e457c3 · 2021-07-19T19:04:23.000+02:00
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 also used to strip CDATA sections. However, 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. We've preserved that behavior for backwards compatibility in 3.x but we're removing it for 4.0. Fixes gh-4904 Closes gh-4906
diff --git a/src/manipulation.js b/src/manipulation.js
@@ -25,9 +25,7 @@ var
 
 	// Support: IE <=10 - 11+
 	// In IE using regex groups here causes severe slowdowns.
-	rnoInnerhtml = /<script|<style|<link/i,
-
-	rcleanScript = /^\s*<!(?:\[CDATA\[|--)|(?:\]\]|--)>\s*$/g;
+	rnoInnerhtml = /<script|<style|<link/i;
 
 // Prefer a tbody over its parent table for containing new rows
 function manipulationTarget( elem, content ) {
@@ -161,7 +159,7 @@ function domManip( collection, args, callback, ignored ) {
 								}, doc );
 							}
 						} else {
-							DOMEval( node.textContent.replace( rcleanScript, "" ), node, doc );
+							DOMEval( node.textContent, node, doc );
 						}
 					}
 				}
diff --git a/test/data/cleanScript.html b/test/data/cleanScript.html
@@ -4,7 +4,7 @@
 -->
 </script>
 <script>
-<![CDATA[
+<!--//--><![CDATA[//><!--
 QUnit.assert.ok( true, "script within CDATA executed" );
-]]>
+//--><!]]>
 </script>
diff --git a/test/unit/manipulation.js b/test/unit/manipulation.js
@@ -2233,19 +2233,31 @@ QUnit.test( "domManip executes scripts containing html comments or CDATA (trac-9
 		"</script>"
 	].join( "\n" ) ).appendTo( "#qunit-fixture" );
 
+	// This test requires XHTML mode as CDATA is not recognized in HTML.
+	// jQuery( [
+	// 	"<script type='text/javascript'>",
+	// 	"<![CDATA[",
+	// 	"QUnit.assert.ok( true, '<![CDATA[ handled' );",
+	// 	"//]]>",
+	// 	"</script>"
+	// ].join( "\n" ) ).appendTo( "#qunit-fixture" );
+
 	jQuery( [
 		"<script type='text/javascript'>",
-		"<![CDATA[",
-		"QUnit.assert.ok( true, '<![CDATA[ handled' );",
-		"//]]>",
+		"<!--//--><![CDATA[//><!--",
+		"QUnit.assert.ok( true, '<!--//--><![CDATA[//><!-- (Drupal case) handled' );",
+		"//--><!]]>",
 		"</script>"
 	].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( [
 		"<script type='text/javascript'>",
-		"<!--//--><![CDATA[//><!--",
-		"QUnit.assert.ok( true, '<!--//--><![CDATA[//><!-- (Drupal case) handled' );",
-		"//--><!]]>",
+		"<!-- Same-line HTML comment",
+		"QUnit.assert.ok( true, '<!-- Same-line HTML comment' );",
+		"-->",
 		"</script>"
 	].join( "\n" ) ).appendTo( "#qunit-fixture" );
 } );

Original file line number	Diff line number	Diff line change
`@@ -25,9 +25,7 @@ var`
`25`	`25`
`26`	`26`	`// Support: IE <=10 - 11+`
`27`	`27`	`// In IE using regex groups here causes severe slowdowns.`
`28`		`- rnoInnerhtml = /<script\|<style\|<link/i,`
`29`		`-`
`30`		`- rcleanScript = /^\s<!(?:\[CDATA\[\|--)\|(?:\]\]\|--)>\s$/g;`
	`28`	`+ rnoInnerhtml = /<script\|<style\|<link/i;`
`31`	`29`
`32`	`30`	`// Prefer a tbody over its parent table for containing new rows`
`33`	`31`	`function manipulationTarget( elem, content ) {`
`@@ -161,7 +159,7 @@ function domManip( collection, args, callback, ignored ) {`
`161`	`159`	`}, doc );`
`162`	`160`	`}`
`163`	`161`	`} else {`
`164`		`- DOMEval( node.textContent.replace( rcleanScript, "" ), node, doc );`
	`162`	`+ DOMEval( node.textContent, node, doc );`
`165`	`163`	`}`
`166`	`164`	`}`
`167`	`165`	`}`