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 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
Closes gh-4905
Ref gh-4906

diff --git a/src/manipulation.js b/src/manipulation.js
index dec21ea0b46482442e3c6021987ab1f8c434fddd..64a8785e0d31d002fdfb85d86a37c3a237c7db54 100644 (file)
--- 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*<!(?:\[CDATA\[|--)|(?:\]\]|--)>\s*$/g;
+
+       rcleanScript = /^\s*<!\[CDATA\[|\]\]>\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 {
+
+                                                       // 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.
+                                                       // 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 22e9ae7470d18c99f4cdc8d936cbaf7cc164f82c..3fe49aae9b61f1e8b4d878f73778f21799a3c89f 100644 (file)
--- 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( [
                "<script type='text/javascript'>",
@@ -2293,6 +2293,17 @@ QUnit.test( "domManip executes scripts containing html comments or CDATA (trac-9
                "//--><!]]>",
                "</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'>",
+               "<!-- Same-line HTML comment",
+               "QUnit.assert.ok( true, '<!-- Same-line HTML comment' );",
+               "-->",
+               "</script>"
+       ].join( "\n" ) ).appendTo( "#qunit-fixture" );
 } );
 
 testIframe(