Skip to content

Commit c61ae8f

Browse files
committed
Core: review feedback
1 parent 9867ccc commit c61ae8f

File tree

3 files changed

+63
-64
lines changed

3 files changed

+63
-64
lines changed

src/core.js

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,13 @@ var findProp,
77
rattrHashTest = /\[(\s*[-\w]+\s*)([~|^$*]?=)\s*([-\w#]*?#[-\w#]*)\s*\]/,
88
rattrHashGlob = /\[(\s*[-\w]+\s*)([~|^$*]?=)\s*([-\w#]*?#[-\w#]*)\s*\]/g,
99
rxhtmlTag = /<(?!area|br|col|embed|hr|img|input|link|meta|param)(([a-z][^\/\0>\x20\t\r\n\f]*)[^>]*)\/>/gi,
10-
rsingleTag = ( /^<([a-z][^\/\0>:\x20\t\r\n\f]*)[\x20\t\r\n\f]*\/?>(?:<\/\1>|)$/i ),
1110

1211
// Support: Android <=4.0 only
1312
// Make sure we trim BOM and NBSP
1413
rtrim = /^[\s\uFEFF\xA0]+|[\s\uFEFF\xA0]+$/g,
1514
makeMarkup = function( html ) {
16-
var doc = window.document.implementation.createHTMLDocument( "", "", null );
17-
doc.open( "replace" );
18-
doc.write( html );
19-
doc.close();
15+
var doc = window.document.implementation.createHTMLDocument( "" );
16+
doc.body.innerHTML = html;
2017
return doc.body && doc.body.innerHTML;
2118
};
2219

@@ -35,14 +32,9 @@ jQuery.fn.init = function( arg1 ) {
3532
jQuery.fn.init.prototype = jQuery.fn;
3633

3734
jQuery.htmlPrefilter = function( html ) {
38-
var changed;
39-
if ( typeof html === "string" && html.trim().charAt( 0 ) === "<" ) {
40-
if ( !rsingleTag.test( html ) ) {
41-
changed = html.replace( rxhtmlTag, "<$1></$2>" );
42-
if ( changed !== html && makeMarkup( html ) !== makeMarkup( changed ) ) {
43-
migrateWarn( "HTML tags must be properly nested and closed: " + html );
44-
}
45-
}
35+
var changed = html.replace( rxhtmlTag, "<$1></$2>" );
36+
if ( changed !== html && makeMarkup( html ) !== makeMarkup( changed ) ) {
37+
migrateWarn( "HTML tags must be properly nested and closed: " + html );
4638
}
4739
return html;
4840
};

test/core.js

Lines changed: 51 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,15 @@ QUnit.test( "jQuery.migrateDeduplicateWarnings", function( assert ) {
4545
jQuery.migrateDeduplicateWarnings = origValue;
4646
} );
4747

48-
QUnit.test( "jQuery( html, props )", function( assert ) {
48+
QUnit.test( "jQuery(html, props)", function( assert ) {
4949
assert.expect( 3 );
5050

5151
var $el = jQuery( "<input/>", { name: "name", val: "value", size: 42 } );
5252

5353
assert.equal( $el.attr( "name" ), "name", "Name attribute" );
5454
assert.equal( $el.attr( "size" ),
5555
jQuery.isEmptyObject( jQuery.attrFn ) ? undefined : "42", "Size attribute" );
56-
assert.equal( $el.val( ), "value", "Call setter method" );
56+
assert.equal( $el.val(), "value", "Call setter method" );
5757
} );
5858

5959
QUnit.test( "jQuery( '#' )", function( assert ) {
@@ -106,16 +106,16 @@ QUnit.test( "Attribute selectors with unquoted hashes", function( assert ) {
106106
assert.expect( 31 );
107107

108108
var markup = jQuery(
109-
"<div>" +
110-
"<div data-selector='a[href=#main]'></div>" +
111-
"<a href='space#junk'>test</a>" +
112-
"<link rel='good#stuff' />" +
113-
"<p class='space #junk'>" +
114-
"<a href='#some-anchor'>anchor2</a>" +
115-
"<input value='[strange*=#stuff]' />" +
116-
"<a href='#' data-id='#junk'>anchor</a>" +
117-
"</p>" +
118-
"</div>" ).appendTo( "#qunit-fixture" ),
109+
"<div>" +
110+
"<div data-selector='a[href=#main]'></div>" +
111+
"<a href='space#junk'>test</a>" +
112+
"<link rel='good#stuff' />" +
113+
"<p class='space #junk'>" +
114+
"<a href='#some-anchor'>anchor2</a>" +
115+
"<input value='[strange*=#stuff]' />" +
116+
"<a href='#' data-id='#junk'>anchor</a>" +
117+
"</p>" +
118+
"</div>" ).appendTo( "#qunit-fixture" ),
119119

120120
// No warning, no need to fix
121121
okays = [
@@ -128,7 +128,7 @@ QUnit.test( "Attribute selectors with unquoted hashes", function( assert ) {
128128
// Fixable, and gives warning
129129
fixables = [
130130
"a[href=#]",
131-
"a[href*=#]:not( [href=#] ):first-child",
131+
"a[href*=#]:not([href=#]):first-child",
132132
".space a[href=#]",
133133
"a[href=#some-anchor]",
134134
"link[rel*=#stuff]",
@@ -139,13 +139,13 @@ QUnit.test( "Attribute selectors with unquoted hashes", function( assert ) {
139139
// False positives that still work
140140
positives = [
141141
"div[data-selector='a[href=#main]']:first",
142-
"input[value= '[strange*=#stuff]']:eq( 0 )"
142+
"input[value= '[strange*=#stuff]']:eq(0)"
143143
],
144144

145145
// Failures due to quotes and jQuery extensions combined
146146
failures = [
147147
"p[class ^= #junk]:first",
148-
"a[href=space#junk]:eq( 1 )"
148+
"a[href=space#junk]:eq(1)"
149149
];
150150

151151
expectNoWarning( assert, "Perfectly cromulent selectors are unchanged", function() {
@@ -174,11 +174,11 @@ QUnit.test( "Attribute selectors with unquoted hashes", function( assert ) {
174174
failures.forEach( function( failure ) {
175175
try {
176176
jQuery( failure, markup );
177-
assert.ok( false, "Expected jQuery( ) to die!" );
177+
assert.ok( false, "Expected jQuery() to die!" );
178178
} catch ( err1 ) { }
179179
try {
180180
markup.find( failure );
181-
assert.ok( false, "Expected .find( ) to die!" );
181+
assert.ok( false, "Expected .find() to die!" );
182182
} catch ( err2 ) { }
183183
} );
184184
} );
@@ -191,11 +191,11 @@ QUnit.test( "Attribute selectors with unquoted hashes", function( assert ) {
191191
jQuery( "a[href=#]" );
192192
}
193193
} );
194-
} catch ( e ) { }
194+
} catch ( e ) {}
195195
} );
196196
} );
197197

198-
QUnit.test( "XSS injection ( leading hash )", function( assert ) {
198+
QUnit.test( "XSS injection (leading hash)", function( assert ) {
199199
assert.expect( 1 );
200200

201201
var threw = false;
@@ -206,10 +206,10 @@ QUnit.test( "XSS injection ( leading hash )", function( assert ) {
206206
threw = true;
207207
}
208208

209-
assert.equal( threw, true, "Throw on leading-hash HTML ( treated as selector )" );
209+
assert.equal( threw, true, "Throw on leading-hash HTML (treated as selector)" );
210210
} );
211211

212-
QUnit.test( "XSS injection ( XSS via script tag )", function( assert ) {
212+
QUnit.test( "XSS injection (XSS via script tag)", function( assert ) {
213213
assert.expect( 2 );
214214

215215
var threw = false;
@@ -233,11 +233,11 @@ QUnit.test( "XSS injection ( XSS with hash and leading space )", function( asser
233233
} catch ( e ) {
234234
threw = true;
235235
}
236-
assert.equal( threw, true, "Throw on leading-hash HTML and space ( treated as selector )" );
236+
assert.equal( threw, true, "Throw on leading-hash HTML and space (treated as selector)" );
237237
assert.equal( window.XSS, false, "XSS" );
238238
} );
239239

240-
QUnit.test( "XSS injection ( XSS via onerror inline handler )", function( assert ) {
240+
QUnit.test( "XSS injection (XSS via onerror inline handler)", function( assert ) {
241241
assert.expect( 2 );
242242

243243
var start,
@@ -249,27 +249,27 @@ QUnit.test( "XSS injection ( XSS via onerror inline handler )", function( assert
249249
} catch ( e ) {
250250
threw = true;
251251
}
252-
assert.equal( threw, true, "Throw on leading-hash HTML ( treated as selector )" );
252+
assert.equal( threw, true, "Throw on leading-hash HTML (treated as selector)" );
253253

254-
start = assert.async( );
254+
start = assert.async();
255255
setTimeout( function() {
256256
assert.equal( window.XSS, false, "XSS" );
257-
start( );
257+
start();
258258
}, 1000 );
259259
} );
260260

261-
QUnit.test( "jQuery( '<element>' ) usable on detached elements ( #128 )", function( assert ) {
261+
QUnit.test( "jQuery('<element>') usable on detached elements (#128)", function( assert ) {
262262
assert.expect( 1 );
263263

264-
jQuery( "<a>" ).outerWidth( );
264+
jQuery( "<a>" ).outerWidth();
265265
assert.ok( true, "No crash when operating on detached elements with window" );
266266
} );
267267

268268
QUnit.test( ".size", function( assert ) {
269269
assert.expect( 1 );
270270

271271
expectWarning( assert, "size", function() {
272-
jQuery( "<div />" ).size( );
272+
jQuery( "<div />" ).size();
273273
} );
274274
} );
275275

@@ -309,24 +309,24 @@ QUnit.test( "isNumeric", function( assert ) {
309309
assert.ok( t( 8e5 ), "Exponential notation" );
310310
assert.ok( t( "123e-2" ), "Exponential notation string" );
311311
assert.ok( t( "040" ), "Legacy octal integer literal string" );
312-
assert.ok( t( "0xFF" ), "Hexadecimal integer literal string ( 0x... )" );
313-
assert.ok( t( "0Xba" ), "Hexadecimal integer literal string ( 0X... )" );
312+
assert.ok( t( "0xFF" ), "Hexadecimal integer literal string (0x...)" );
313+
assert.ok( t( "0Xba" ), "Hexadecimal integer literal string (0X...)" );
314314
assert.ok( t( 0xFFF ), "Hexadecimal integer literal" );
315315

316316
if ( +"0b1" === 1 ) {
317-
assert.ok( t( "0b111110" ), "Binary integer literal string ( 0b... )" );
318-
assert.ok( t( "0B111110" ), "Binary integer literal string ( 0B... )" );
317+
assert.ok( t( "0b111110" ), "Binary integer literal string (0b...)" );
318+
assert.ok( t( "0B111110" ), "Binary integer literal string (0B...)" );
319319
} else {
320-
assert.ok( true, "Browser does not support binary integer literal ( 0b... )" );
321-
assert.ok( true, "Browser does not support binary integer literal ( 0B... )" );
320+
assert.ok( true, "Browser does not support binary integer literal (0b...)" );
321+
assert.ok( true, "Browser does not support binary integer literal (0B...)" );
322322
}
323323

324324
if ( +"0o1" === 1 ) {
325-
assert.ok( t( "0o76" ), "Octal integer literal string ( 0o... )" );
326-
assert.ok( t( "0O76" ), "Octal integer literal string ( 0O... )" );
325+
assert.ok( t( "0o76" ), "Octal integer literal string (0o...)" );
326+
assert.ok( t( "0O76" ), "Octal integer literal string (0O...)" );
327327
} else {
328-
assert.ok( true, "Browser does not support octal integer literal ( 0o... )" );
329-
assert.ok( true, "Browser does not support octal integer literal ( 0O... )" );
328+
assert.ok( true, "Browser does not support octal integer literal (0o...)" );
329+
assert.ok( true, "Browser does not support octal integer literal (0O...)" );
330330
}
331331

332332
assert.equal( t( new ToString( "42" ) ), false, "Only limited to strings and numbers" );
@@ -353,18 +353,18 @@ QUnit.test( "isNumeric", function( assert ) {
353353
assert.equal( t( new Date( ) ), false, "Instance of a Date" );
354354
} );
355355

356-
QUnit[ typeof Symbol === "function" ? "test" : "skip" ]( "isNumeric( Symbol )", function( assert ) {
356+
QUnit[ typeof Symbol === "function" ? "test" : "skip" ]( "isNumeric(Symbol)", function( assert ) {
357357
assert.expect( 2 );
358358

359-
assert.equal( jQuery.isNumeric( Symbol( ) ), false, "Symbol" );
360-
assert.equal( jQuery.isNumeric( Object( Symbol( ) ) ), false, "Symbol inside an object" );
359+
assert.equal( jQuery.isNumeric( Symbol() ), false, "Symbol" );
360+
assert.equal( jQuery.isNumeric( Object( Symbol() ) ), false, "Symbol inside an object" );
361361
} );
362362

363-
QUnit[ jQueryVersionSince( "3.3.0" ) ? "test" : "skip" ]( ".isNumeric ( warn )",
363+
QUnit[ jQueryVersionSince( "3.3.0" ) ? "test" : "skip" ]( ".isNumeric (warn)",
364364
function( assert ) {
365365
assert.expect( 3 );
366366

367-
expectWarning( assert, "warning on isNumeric ( and possibly type )", function() {
367+
expectWarning( assert, "warning on isNumeric (and possibly type)", function() {
368368
assert.equal( jQuery.isNumeric( 42 ), true, "isNumeric number" );
369369
assert.equal( jQuery.isNumeric( "nope" ), false, "isNumeric non number" );
370370
} );
@@ -412,7 +412,7 @@ QUnit.test( "jQuery.expr.pseudos aliases", function( assert ) {
412412

413413
assert.ok( jQuery.expr.pseudos.mazda, "filters assigned" );
414414
assert.ok( jQuery.expr.pseudos.marginal, "[':'] assigned" );
415-
fixture.find( "p" ).first( ).css( "marginLeftWidth", "40px" );
415+
fixture.find( "p" ).first().css( "marginLeftWidth", "40px" );
416416
assert.equal( fixture.find( "p:marginal" ).length, 1, "One marginal element" );
417417
assert.equal( fixture.find( "div:mazda" ).length, 0, "No mazda elements" );
418418
delete jQuery.expr.pseudos.mazda;
@@ -421,7 +421,7 @@ QUnit.test( "jQuery.expr.pseudos aliases", function( assert ) {
421421

422422
} );
423423

424-
QUnit.test( "jQuery.holdReady ( warn only )", function( assert ) {
424+
QUnit.test( "jQuery.holdReady (warn only)", function( assert ) {
425425
assert.expect( 1 );
426426

427427
expectWarning( assert, "jQuery.holdReady", 2, function() {
@@ -441,7 +441,7 @@ QUnit[ jQueryVersionSince( "3.1.1" ) ? "test" : "skip" ]( "jQuery.trim", functio
441441
assert.equal( jQuery.trim( " hello " ), "hello", "space on both sides" );
442442
assert.equal( jQuery.trim( " " + nbsp + "hello " + nbsp + " " ), "hello", "&nbsp;" );
443443

444-
assert.equal( jQuery.trim( ), "", "Nothing in." );
444+
assert.equal( jQuery.trim(), "", "Nothing in." );
445445
assert.equal( jQuery.trim( undefined ), "", "Undefined" );
446446
assert.equal( jQuery.trim( null ), "", "Null" );
447447
assert.equal( jQuery.trim( 5 ), "5", "Number" );
@@ -494,10 +494,10 @@ QUnit[ jQueryVersionSince( "3.3.0" ) ? "test" : "skip" ]( "jQuery.type ( warn )"
494494
assert.equal( jQuery.type( /foo/ ), "regexp", "RegExp" );
495495
assert.equal( jQuery.type( new RegExp( "asdf" ) ), "regexp", "RegExp" );
496496
assert.equal( jQuery.type( [ 1 ] ), "array", "Array" );
497-
assert.equal( jQuery.type( new Date( ) ), "date", "Date" );
497+
assert.equal( jQuery.type( new Date() ), "date", "Date" );
498498
assert.equal( jQuery.type( new Function( "return;" ) ), "function", "Function" );
499499
assert.equal( jQuery.type( function() {} ), "function", "Function" );
500-
assert.equal( jQuery.type( new Error( ) ), "error", "Error" );
500+
assert.equal( jQuery.type( new Error() ), "error", "Error" );
501501
assert.equal( jQuery.type( window ), "object", "Window" );
502502
assert.equal( jQuery.type( document ), "object", "Document" );
503503
assert.equal( jQuery.type( document.body ), "object", "Element" );
@@ -512,7 +512,7 @@ QUnit[ jQueryVersionSince( "3.3.0" ) ? "test" : "skip" ]( "jQuery.type ( warn )"
512512
assert.equal( jQuery.type( new MyBoolean( true ) ), "boolean", "Boolean" );
513513
assert.equal( jQuery.type( new MyNumber( 1 ) ), "number", "Number" );
514514
assert.equal( jQuery.type( new MyString( "a" ) ), "string", "String" );
515-
assert.equal( jQuery.type( new MyObject( ) ), "object", "Object" );
515+
assert.equal( jQuery.type( new MyObject() ), "object", "Object" );
516516

517517
} );
518518

@@ -522,7 +522,7 @@ QUnit[ jQueryVersionSince( "3.3.0" ) ? "test" : "skip" ]( "jQuery.isArray", func
522522
expectWarning( assert, "isArray", 3, function() {
523523
assert.equal( jQuery.isArray( [] ), true, "empty array" );
524524
assert.equal( jQuery.isArray( "" ), false, "empty string" );
525-
assert.equal( jQuery.isArray( jQuery( ).toArray( ) ), true, "toArray" );
525+
assert.equal( jQuery.isArray( jQuery( ).toArray() ), true, "toArray" );
526526
} );
527527

528528
} );

warnings.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,3 +259,10 @@ See jQuery-ui [commit](https://github.com/jquery/jquery-ui/commit/c0093b599fcd58
259259
**Cause:** In past versions, when a number-typed value was passed to `.css()` jQuery converted it to a string and added `"px"` to the end. As the CSS standard has evolved, an increasingly large set of CSS properties now accept values that are unitless numbers, where this behavior is incorrect. It has become impractical to manage these exceptions in the `jQuery.cssNumber` object. In addition, some CSS properties like `line-height` can accept both a bare number `2` or a pixel value `2px`. jQuery cannot know the correct way to interpret `$.css("line-height", 2)` and currently treats it as `"2px"`.
260260

261261
**Solution:** Always pass string values to `.css()`, and explicitly add units where required. For example, use `$.css("line-height", "2")` to specify 200% of the current line height or `$.css("line-height", "2px")` to specify pixels. When the numeric value is in a variable, ensure the value is converted to string, e.g. `$.css("line-height", String(height))` and `$.css("line-height", height+"px")`.
262+
263+
#### JQMIGRATE: HTML tags must be properly nested and closed: _(HTML string)_
264+
265+
**Cause:** jQuery 3.5.0 changed the way it processes HTML strings. Previously, jQuery would attempt to fix self-closed tags like `<i class="test" />` that the HTML5 specification says are not self-closed, turning it into `<i class="test"></i>`. This processing can create a [security problem](https://nvd.nist.gov/vuln/detail/CVE-2020-11022) with malicious strings, so the functionality had to be removed.
266+
267+
**Solution:** Search for the reported HTML strings and edit the tags to close them explicitly. In some cases the strings passed to jQuery may be created inside the program and thus not searchable. Migrate warning messages include a stack trace that can be used to find the location of the usage in the code.
268+

0 commit comments

Comments
 (0)