Skip to content

Commit 81d5f13

Browse files
dmethvindmethvin
committed
Attributes: Fix unquoted hashes in selectors where possible
Fixes #174 Closes #184 Ref #140 (cherry picked from commit d7da19d)
1 parent c09b6a0 commit 81d5f13

File tree

4 files changed

+136
-69
lines changed

4 files changed

+136
-69
lines changed

‎src/core.js‎

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11

22
var matched, browser,
33
oldInit = jQuery.fn.init,
4+
oldFind = jQuery.find,
45
oldParseJSON = jQuery.parseJSON,
56
rspaceAngle = /^\s*</,
6-
rattrHash = /\[\s*\w+\s*[~|^$*]?=\s*(?![\s'"])[^#\]]*#/,
7+
rattrHashTest = /\[(\s*[-\w]+\s*)([~|^$*]?=)\s*([-\w#]*?#[-\w#]*)\s*\]/,
8+
rattrHashGlob = /\[(\s*[-\w]+\s*)([~|^$*]?=)\s*([-\w#]*?#[-\w#]*)\s*\]/g,
79
// Note: XSS check is done below after string is trimmed
810
rquickExpr = /^([^<]*)(<[\w\W]+>)([^>]*)$/;
911

@@ -42,19 +44,6 @@ jQuery.fn.init = function( selector, context, rootjQuery ) {
4244
context || document, true ), context, rootjQuery );
4345
}
4446
}
45-
46-
if ( selector === "#" ) {
47-
48-
// jQuery( "#" ) is a bogus ID selector, but it returned an empty set before jQuery 3.0
49-
migrateWarn( "jQuery( '#' ) is not a valid selector" );
50-
selector = [];
51-
52-
} else if ( rattrHash.test( selector ) ) {
53-
54-
// The nonstandard and undocumented unquoted-hash was removed in jQuery 1.12.0
55-
// Note that this doesn't actually fix the selector due to potential false positives
56-
migrateWarn( "Attribute selectors with '#' must be quoted: '" + selector + "'" );
57-
}
5847
}
5948

6049
ret = oldInit.apply( this, arguments );
@@ -76,6 +65,47 @@ jQuery.fn.init = function( selector, context, rootjQuery ) {
7665
};
7766
jQuery.fn.init.prototype = jQuery.fn;
7867

68+
jQuery.find = function( selector ) {
69+
var args = Array.prototype.slice.call( arguments );
70+
71+
// Support: PhantomJS 1.x
72+
// String#match fails to match when used with a //g RegExp, only on some strings
73+
if ( typeof selector === "string" && rattrHashTest.test( selector ) ) {
74+
75+
// The nonstandard and undocumented unquoted-hash was removed in jQuery 1.12.0
76+
// First see if qS thinks it's a valid selector, if so avoid a false positive
77+
try {
78+
document.querySelector( selector );
79+
} catch ( err1 ) {
80+
81+
// Didn't *look* valid to qSA, warn and try quoting what we think is the value
82+
selector = selector.replace( rattrHashGlob, function( _, attr, op, value ) {
83+
return "[" + attr + op + "\"" + value + "\"]";
84+
} );
85+
86+
// If the regexp *may* have created an invalid selector, don't update it
87+
// Note that there may be false alarms if selector uses jQuery extensions
88+
try {
89+
document.querySelector( selector );
90+
migrateWarn( "Attribute selector with '#' must be quoted: " + args[ 0 ] );
91+
args[ 0 ] = selector;
92+
} catch ( err2 ) {
93+
migrateWarn( "Attribute selector with '#' was not fixed: " + args[ 0 ] );
94+
}
95+
}
96+
}
97+
98+
return oldFind.apply( this, args );
99+
};
100+
101+
// Copy properties attached to original jQuery.find method (e.g. .attr, .isXML)
102+
var findProp;
103+
for ( findProp in oldFind ) {
104+
if ( Object.prototype.hasOwnProperty.call( oldFind, findProp ) ) {
105+
jQuery.find[ findProp ] = oldFind[ findProp ];
106+
}
107+
}
108+
79109
// Let $.parseJSON(falsy_value) return null
80110
jQuery.parseJSON = function( json ) {
81111
if ( !json ) {

‎src/traversing.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
var oldSelf = jQuery.fn.andSelf || jQuery.fn.addBack,
2-
oldFind = jQuery.fn.find;
2+
oldFnFind = jQuery.fn.find;
33

44
jQuery.fn.andSelf = function() {
55
migrateWarn("jQuery.fn.andSelf() replaced by jQuery.fn.addBack()");
66
return oldSelf.apply( this, arguments );
77
};
88

99
jQuery.fn.find = function( selector ) {
10-
var ret = oldFind.apply( this, arguments );
10+
var ret = oldFnFind.apply( this, arguments );
1111
ret.context = this.context;
1212
ret.selector = this.selector ? this.selector + " " + selector : selector;
1313
return ret;

‎test/core.js‎

Lines changed: 87 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -53,57 +53,91 @@ test( "jQuery(html) loose rules", function() {
5353
}
5454
});
5555

56-
test( "jQuery( '#' )", function() {
57-
expect( 2 );
58-
59-
expectWarning( "Selector, through the jQuery constructor, nothing but hash", function() {
60-
var set = jQuery( "#" );
61-
equal( set.length, 0, "empty set" );
62-
});
63-
});
64-
65-
test( "attribute selectors with naked '#'", function() {
66-
expect( 7 );
67-
68-
// These are wrapped in try/catch because they throw on jQuery 1.12.0+
69-
70-
expectWarning( "attribute equals", function() {
71-
try {
72-
jQuery( "a[href=#]" );
73-
} catch( e ) {}
74-
});
75-
76-
expectWarning( "attribute contains", function() {
77-
try {
78-
jQuery( "link[rel*=#stuff]" );
79-
} catch( e ) {}
80-
});
81-
82-
expectWarning( "attribute starts, with spaces", function() {
83-
try {
84-
jQuery( "a[href ^= #junk]" );
85-
} catch( e ) {}
86-
});
87-
88-
expectWarning( "attribute equals, hash not starting", function() {
89-
try {
90-
jQuery( "a[href=space#junk]" );
91-
} catch( e ) {}
92-
});
93-
94-
expectNoWarning( "attribute equals, with single quotes", function() {
95-
try {
96-
jQuery( "a[href='#junk']" );
97-
} catch( e ) {}
98-
});
99-
100-
expectNoWarning( "attribute equals, with double quotes", function() {
101-
try {
102-
jQuery( "a[href=\"#junk\"]" );
103-
} catch( e ) {}
104-
});
105-
106-
expectNoWarning( "function containing tempting string (#178)", function() {
56+
// Selector quoting doesn't work in IE8
57+
if ( !document.querySelector ) {
58+
59+
QUnit.test( "Attribute selectors with unquoted hashes", function( assert ) {
60+
expect( 31 );
61+
62+
var markup = jQuery(
63+
"<div>" +
64+
"<div data-selector='a[href=#main]'></div>" +
65+
"<a href='space#junk'>test</a>" +
66+
"<link rel='good#stuff' />" +
67+
"<p class='space #junk'>" +
68+
"<a href='#some-anchor'>anchor2</a>" +
69+
"<input value='[strange*=#stuff]' />" +
70+
"<a href='#' data-id='#junk'>anchor</a>" +
71+
"</p>" +
72+
"</div>" ).appendTo( "#qunit-fixture" ),
73+
74+
// No warning, no need to fix
75+
okays = [
76+
"a[href='#some-anchor']",
77+
"[data-id=\"#junk\"]",
78+
"div[data-selector='a[href=#main]']",
79+
"input[value~= '[strange*=#stuff]']"
80+
],
81+
82+
// Fixable, and gives warning
83+
fixables = [
84+
"a[href=#]",
85+
"a[href*=#]:not([href=#]):first-child",
86+
".space a[href=#]",
87+
"a[href=#some-anchor]",
88+
"link[rel*=#stuff]",
89+
"p[class *= #junk]",
90+
"a[href=space#junk]"
91+
],
92+
93+
// False positives that still work
94+
positives = [
95+
"div[data-selector='a[href=#main]']:first",
96+
"input[value= '[strange*=#stuff]']:eq(0)"
97+
],
98+
99+
// Failures due to quotes and jQuery extensions combined
100+
failures = [
101+
"p[class ^= #junk]:first",
102+
"a[href=space#junk]:eq(1)"
103+
];
104+
105+
expectNoWarning( "Perfectly cromulent selectors are unchanged", function() {
106+
okays.forEach( function( okay ) {
107+
assert.equal( jQuery( okay, markup ).length, 1, okay );
108+
assert.equal( markup.find( okay ).length, 1, okay );
109+
} );
110+
} );
111+
112+
expectWarning( "Values with unquoted hashes are quoted", fixables.length, function() {
113+
fixables.forEach( function( fixable ) {
114+
assert.equal( jQuery( fixable, markup ).length, 1, fixable );
115+
assert.equal( markup.find( fixable ).length, 1, fixable );
116+
} );
117+
} );
118+
119+
expectWarning( "False positives", positives.length, function() {
120+
positives.forEach( function( positive ) {
121+
assert.equal( jQuery( positive, markup ).length, 1, positive );
122+
assert.equal( markup.find( positive ).length, 1, positive );
123+
} );
124+
} );
125+
126+
expectWarning( "Unfixable cases", failures.length, function() {
127+
failures.forEach( function( failure ) {
128+
try {
129+
jQuery( failure, markup );
130+
assert.ok( false, "Expected jQuery() to die!" );
131+
} catch ( err1 ) { }
132+
try {
133+
markup.find( failure );
134+
assert.ok( false, "Expected .find() to die!" );
135+
} catch ( err2 ) { }
136+
} );
137+
} );
138+
139+
// Ensure we don't process jQuery( x ) when x is a function
140+
expectNoWarning( "ready function with attribute selector", function() {
107141
try {
108142
jQuery( function() {
109143
if ( jQuery.thisIsNeverDefined ) {
@@ -114,6 +148,8 @@ test( "attribute selectors with naked '#'", function() {
114148
});
115149
});
116150

151+
}
152+
117153
QUnit.test( "document.context defined (#178)", function( assert ) {
118154
assert.expect( 1 );
119155

‎warnings.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,10 @@ This is _not_ a warning, but a console log message the plugin shows when it firs
3131

3232
**Solution**: Usually this warning is due to an error in the HTML string, where text is present when it should not be there. Remove the leading or trailing text before passing the string to `$.parseHTML()` if it should not be part of the collection. Alternatively you can use `$($.parseHTML(html)).filter("*")` to remove all top-level text nodes from the set and leave only elements.
3333

34-
### JQMIGRATE: Attribute selectors with '#' must be quoted
34+
### JQMIGRATE: Attribute selector with '#' must be quoted
35+
### JQMIGRATE: Attribute selector with '#' was not fixed
3536

36-
**Cause:** CSS selectors such as `a[href=#main]` are not valid CSS syntax because the value contains special characters that are not quoted. Until jQuery 1.11.3/2.1.4 this syntax was accepted, but the behavior is non-standard and was never documented. In later versions this selector throws an error. *In some rare cases Migrate may mis-diagnose this problem, so it does not attempt a repair.*
37+
**Cause:** Selectors such as `a[href=#main]` are not valid CSS syntax because the value contains special characters that are not quoted. Until jQuery 1.11.3/2.1.4 this was accepted, but the behavior is non-standard and was never documented. In later versions this selector throws an error. *In some cases with complex selectors, Migrate may not attempt a repair.* In those cases a fatal error will be logged on the console and you will need to fix the selector manually.
3738

3839
**Solution**: Put quotes around any attribute values that have special characters, e.g. `a[href="#main"]`. The warning message contains the selector that caused the problem, use that to find the selector in the source files.
3940

0 commit comments

Comments
 (0)