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 `<!--` and `-->` 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
index f86bd9ab0c70dc5d31c97d6deb683887661b3db4..19c60fcbcc60b7e37b9dac8d40b1dfe527eef1c3 100644 (file)
--- 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
index c37694aacdb5d0631daf91d6c66072e43d2e98cc..60d235b827047c6376ce284264e48e3163d20d19 100644 (file)
--- a/test/data/cleanScript.html
+++ b/test/data/cleanScript.html
@@ -4,7 +4,7 @@ QUnit.assert.ok( true, "script within html comments executed" );
 -->
 </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
index 131109448be5c649507aa0013b2a4ce898753ccf..8262516a9ef8cc9a2a1a36dfff44a5b3b13b8a66 100644 (file)
--- 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" );
 } );