Permalink
Browse files

Applied the RegExp issues reported by Jeff Robinson here: http://jmrw…

…are.com/articles/2010/jqueryregex/jQueryRegexes.html Additionally broke out all remaining inline RegExp. Fixes #7062.
  • Loading branch information...
1 parent 19b5d9e commit 9dc6e0c572b9c809a3a4c123071d96d48a01dd1c @jeresig jeresig committed Sep 22, 2010
Showing with 66 additions and 50 deletions.
  1. +7 −6 src/ajax.js
  2. +5 −5 src/attributes.js
  3. +26 −10 src/core.js
  4. +3 −2 src/data.js
  5. +2 −2 src/effects.js
  6. +11 −11 src/event.js
  7. +6 −11 src/manipulation.js
  8. +6 −3 src/offset.js
View
@@ -1,12 +1,13 @@
(function( jQuery ) {
var jsc = jQuery.now(),
- rscript = /<script(.|\s)*?\/script>/gi,
- rselectTextarea = /select|textarea/i,
- rinput = /color|date|datetime|email|hidden|month|number|password|range|search|tel|text|time|url|week/i,
+ rscript = /<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi,
+ rselectTextarea = /^(?:select|textarea)/i,
+ rinput = /^(?:color|date|datetime|email|hidden|month|number|password|range|search|tel|text|time|url|week)$/i,
+ rbracket = /\[\]$/,
jsre = /\=\?(&|$)/,
rquery = /\?/,
- rts = /(\?|&)_=.*?(&|$)/,
+ rts = /([?&])_=[^&]*(&?)/,
rurl = /^(\w+:)?\/\/([^\/?#]+)/,
r20 = /%20/g,
@@ -61,7 +62,7 @@ jQuery.fn.extend({
// See if a selector was specified
self.html( selector ?
// Create a dummy div to hold the results
- jQuery("<div />")
+ jQuery("<div>")
batiste
batiste Sep 23, 2010 Contributor

Is this correct?

dmethvin
dmethvin Sep 23, 2010 Owner

Yes, it's just a self-closed tag.

// inject the contents of the document in, removing the scripts
// to avoid any 'Permission Denied' errors in IE
.append(res.responseText.replace(rscript, ""))
@@ -542,7 +543,7 @@ function buildParams( prefix, obj, traditional, add ) {
if ( jQuery.isArray(obj) ) {
// Serialize array item.
jQuery.each( obj, function( i, v ) {
- if ( traditional || /\[\]$/.test( prefix ) ) {
+ if ( traditional || rbracket.test( prefix ) ) {
// Treat each array item as a scalar.
add( prefix, v );
View
@@ -3,11 +3,11 @@
var rclass = /[\n\t]/g,
rspace = /\s+/,
rreturn = /\r/g,
- rspecialurl = /href|src|style/,
- rtype = /(button|input)/i,
- rfocusable = /(button|input|object|select|textarea)/i,
- rclickable = /^(a|area)$/i,
- rradiocheck = /radio|checkbox/;
+ rspecialurl = /^(?:href|src|style)$/,
+ rtype = /^(?:button|input)$/i,
+ rfocusable = /^(?:button|input|object|select|textarea)$/i,
+ rclickable = /^a(?:rea)?$/i,
+ rradiocheck = /^(?:radio|checkbox)$/i;
davidmurdoch
davidmurdoch Sep 22, 2010 Contributor

Should ^(?:radio|checkbox)$/i; be ^(?:checkbox|radio)$/i; Similar RegExps happen to be alphabetized. Same thing with rnocache in manipulation.js

jeresig
jeresig Sep 22, 2010 Member

Not really that important - looking through the other files many of the items are in non-alphabetical orders.

davidmurdoch
davidmurdoch Sep 22, 2010 Contributor

I definitely agree that its not important. I was just looking through the optimizations in this commit and it seemed like all RegExps that contain node names or attributes have been alphabetized, so I thought I'd point it out.

Since we are on the topic of (mostly trivial?) RegExp optimizations within jQuery, I wonder if it may make sense to order the items within groups by most to least common. Here is a rough JSperf test case demonstrating the effects of re-ordering groups: http://jsperf.com/items-in-regex-group-perf; IE8 nets an improvement of ~20% matching the first grouped item over the last.

jeresig
jeresig Sep 22, 2010 Member

That makes more sense - although it's starting to get into the realm of speculation. IMO the initial RegExp changes proposed made the most sense as there was tangible differences in how the RegExp engine would operate from the old version to the new one - yielding a very obvious gain. After that the gains kind of peter off. Doing the research on the frequency of specific input types for the reordering of an infrequently used RegExp would probably be better spent towards improving the performance of other aspects of the library. If you're hell-bent on it you can absolutely go to town - just send in a pull request and I'll likely land it.

jQuery.fn.extend({
attr: function( name, value ) {
View
@@ -17,21 +17,37 @@ var jQuery = function( selector, context ) {
// A simple way to check for HTML strings or ID strings
// (both of which we optimize for)
- quickExpr = /^[^<]*(<[\w\W]+>)[^>]*$|^#([\w\-]+)$/,
+ quickExpr = /^(?:[^<]*(<[\w\W]+>)[^>]*$|#([\w-]+)$)/,
// Is it a simple selector
isSimple = /^.[^:#\[\.,]*$/,
// Check if a string has a non-whitespace character in it
rnotwhite = /\S/,
+ rwhite = /\s/,
// Used for trimming whitespace
trimLeft = /^\s+/,
trimRight = /\s+$/,
+ // Check for non-word characters
+ rnonword = /\W/,
+
// Match a standalone tag
rsingleTag = /^<(\w+)\s*\/?>(?:<\/\1>)?$/,
+ // JSON RegExp
+ rvalidchars = /^[\],:{}\s]*$/,
+ rvalidescape = /\\(?:["\\\/bfnrt]|u[0-9a-fA-F]{4})/g,
+ rvalidtokens = /"[^"\\\n\r]*"|true|false|null|-?\d+(?:\.\d*)?(?:[eE][+\-]?\d+)?/g,
+ rvalidbraces = /(?:^|:|,)(?:\s*\[)+/g,
+
+ // Useragent RegExp
+ rwebkit = /(webkit)[ \/]([\w.]+)/,
+ ropera = /(opera)(?:.*version)?[ \/]([\w.]+)/,
+ rmsie = /(msie) ([\w.]+)/,
+ rmozilla = /(mozilla)(?:.*? rv:([\w.]+))?/,
+
// Keep a UserAgent string for use with jQuery.browser
userAgent = navigator.userAgent,
@@ -136,7 +152,7 @@ jQuery.fn = jQuery.prototype = {
}
// HANDLE: $("TAG")
- } else if ( !context && /^\w+$/.test( selector ) ) {
+ } else if ( !context && !rnonword.test( selector ) ) {
this.selector = selector;
this.context = document;
selector = document.getElementsByTagName( selector );
@@ -509,9 +525,9 @@ jQuery.extend({
// Make sure the incoming data is actual JSON
// Logic borrowed from http://json.org/json2.js
- if ( /^[\],:{}\s]*$/.test(data.replace(/\\(?:["\\\/bfnrt]|u[0-9a-fA-F]{4})/g, "@")
- .replace(/"[^"\\\n\r]*"|true|false|null|-?\d+(?:\.\d*)?(?:[eE][+\-]?\d+)?/g, "]")
- .replace(/(?:^|:|,)(?:\s*\[)+/g, "")) ) {
+ if ( rvalidchars.test(data.replace(rvalidescape, "@")
+ .replace(rvalidtokens, "]")
+ .replace(rvalidbraces, "")) ) {
// Try to use the native JSON parser first
return window.JSON && window.JSON.parse ?
@@ -761,10 +777,10 @@ jQuery.extend({
uaMatch: function( ua ) {
ua = ua.toLowerCase();
- var match = /(webkit)[ \/]([\w.]+)/.exec( ua ) ||
- /(opera)(?:.*version)?[ \/]([\w.]+)/.exec( ua ) ||
- /(msie) ([\w.]+)/.exec( ua ) ||
- !/compatible/.test( ua ) && /(mozilla)(?:.*? rv:([\w.]+))?/.exec( ua ) ||
+ var match = rwebkit.exec( ua ) ||
+ ropera.exec( ua ) ||
+ rmsie.exec( ua ) ||
+ ua.indexOf("compatible") < 0 && rmozilla.exec( ua ) ||
[];
return { browser: match[1] || "", version: match[2] || "0" };
@@ -792,7 +808,7 @@ if ( indexOf ) {
// Verify that \s matches non-breaking spaces
// (IE fails on this test)
dieseltravis
dieseltravis Sep 22, 2010

IE9 is actually passing this now, yay! Plus it supports native String.trim() too!

-if ( !/\s/.test( "\xA0" ) ) {
+if ( !rwhite.test( "\xA0" ) ) {
trimLeft = /^[\s\xA0]+/;
trimRight = /[\s\xA0]+$/;
}
View
@@ -1,7 +1,8 @@
(function( jQuery ) {
var windowData = {},
- rbrace = /^(?:\{.*\}|\[.*\])$/;
+ rbrace = /^(?:\{.*\}|\[.*\])$/,
+ rdigit = /\d/;
jQuery.extend({
cache: {},
@@ -157,7 +158,7 @@ jQuery.fn.extend({
data = data === "true" ? true :
data === "false" ? false :
data === "null" ? null :
- /\d/.test( data ) && !isNaN( data ) ? parseFloat( data ) :
+ rdigit.test( data ) && !isNaN( data ) ? parseFloat( data ) :
rbrace.test( data ) ? jQuery.parseJSON( data ) :
data;
} catch( e ) {}
View
@@ -1,7 +1,7 @@
(function( jQuery ) {
var elemdisplay = {},
- rfxtypes = /toggle|show|hide/,
+ rfxtypes = /^(?:toggle|show|hide)$/,
rfxnum = /^([+\-]=)?([\d+.\-]+)(.*)$/,
timerId,
fxAttrs = [
@@ -31,7 +31,7 @@ jQuery.fn.extend({
display = elemdisplay[ nodeName ];
} else {
- var elem = jQuery("<" + nodeName + " />").appendTo("body");
+ var elem = jQuery("<" + nodeName + ">").appendTo("body");
display = elem.css("display");
View
@@ -1,10 +1,12 @@
(function( jQuery ) {
var rnamespaces = /\.(.*)$/,
+ rformElems = /^(?:textarea|input|select)$/i,
+ rperiod = /\./g,
+ rspace = / /g,
+ rescape = /[^\w\s.|`]/g,
fcleanup = function( nm ) {
- return nm.replace(/[^\w\s\.\|`]/g, function( ch ) {
- return "\\" + ch;
- });
+ return nm.replace(rescape, "\\$&");
};
/*
@@ -339,7 +341,7 @@ jQuery.event = {
jQuery.event.trigger( event, data, parent, true );
} else if ( !event.isDefaultPrevented() ) {
- var target = event.target, old, targetType = type.replace(/\..*$/, ""),
+ var target = event.target, old, targetType = type.replace(rnamespaces, ""),
isClick = jQuery.nodeName(target, "a") && targetType === "click",
special = jQuery.event.special[ targetType ] || {};
@@ -697,9 +699,7 @@ if ( !jQuery.support.submitBubbles ) {
// change delegation, happens here so we have bind.
if ( !jQuery.support.changeBubbles ) {
- var formElems = /textarea|input|select/i,
-
- changeFilters,
+ var changeFilters,
getVal = function( elem ) {
var type = elem.type, val = elem.value;
@@ -724,7 +724,7 @@ if ( !jQuery.support.changeBubbles ) {
testChange = function testChange( e ) {
var elem = e.target, data, val;
- if ( !formElems.test( elem.nodeName ) || elem.readOnly ) {
+ if ( !rformElems.test( elem.nodeName ) || elem.readOnly ) {
return;
}
@@ -788,13 +788,13 @@ if ( !jQuery.support.changeBubbles ) {
jQuery.event.add( this, type + ".specialChange", changeFilters[type] );
}
- return formElems.test( this.nodeName );
+ return rformElems.test( this.nodeName );
},
teardown: function( namespaces ) {
jQuery.event.remove( this, ".specialChange" );
- return formElems.test( this.nodeName );
+ return rformElems.test( this.nodeName );
}
};
@@ -1073,7 +1073,7 @@ function liveHandler( event ) {
}
function liveConvert( type, selector ) {
- return (type && type !== "*" ? type + "." : "") + selector.replace(/\./g, "`").replace(/ /g, "&");
+ return (type && type !== "*" ? type + "." : "") + selector.replace(rperiod, "`").replace(rspace, "&");
}
jQuery.each( ("blur focus focusin focusout load resize scroll unload click dblclick " +
View
@@ -2,18 +2,13 @@
var rinlinejQuery = / jQuery\d+="(?:\d+|null)"/g,
rleadingWhitespace = /^\s+/,
- rxhtmlTag = /(<([\w:]+)[^>]*?)\/>/g,
- rselfClosing = /^(?:area|br|col|embed|hr|img|input|link|meta|param)$/i,
+ rxhtmlTag = /<(?!area|br|col|embed|hr|img|input|link|meta|param)(([\w:]+)[^>]*)\/>/ig,
rtagName = /<([\w:]+)/,
rtbody = /<tbody/i,
rhtml = /<|&#?\w+;/,
- rnocache = /<script|<object|<embed|<option|<style/i,
+ rnocache = /<(?:script|object|embed|option|style)/i,
rchecked = /checked\s*(?:[^=]|=\s*.checked.)/i, // checked="checked" or checked (html5)
- fcloseTag = function( all, front, tag ) {
- return rselfClosing.test( tag ) ?
- all :
- front + "></" + tag + ">";
- },
+ raction = /\=([^="'>\s]+\/)>/g,
wrapMap = {
option: [ 1, "<select multiple='multiple'>", "</select>" ],
legend: [ 1, "<fieldset>", "</fieldset>" ],
@@ -207,7 +202,7 @@ jQuery.fn.extend({
return jQuery.clean([html.replace(rinlinejQuery, "")
// Handle the case in IE 8 where action=/test/> self-closes a tag
- .replace(/\=([^="'>\s]+\/)>/g, '="$1">')
+ .replace(raction, '="$1">')
.replace(rleadingWhitespace, "")], ownerDocument)[0];
} else {
return this.cloneNode(true);
@@ -235,7 +230,7 @@ jQuery.fn.extend({
(jQuery.support.leadingWhitespace || !rleadingWhitespace.test( value )) &&
!wrapMap[ (rtagName.exec( value ) || ["", ""])[1].toLowerCase() ] ) {
- value = value.replace(rxhtmlTag, fcloseTag);
+ value = value.replace(rxhtmlTag, "<$1></$2>");
try {
for ( var i = 0, l = this.length; i < l; i++ ) {
@@ -479,7 +474,7 @@ jQuery.extend({
} else if ( typeof elem === "string" ) {
// Fix "XHTML"-style tags in all browsers
- elem = elem.replace(rxhtmlTag, fcloseTag);
+ elem = elem.replace(rxhtmlTag, "<$1></$2>");
// Trim whitespace, otherwise indexOf won't work as expected
var tag = (rtagName.exec( elem ) || ["", ""])[1].toLowerCase(),
View
@@ -1,5 +1,8 @@
(function( jQuery ) {
+var rtable = /^t(?:able|d|h)$/i,
+ rroot = /^(?:body|html)$/i;
+
if ( "getBoundingClientRect" in document.documentElement ) {
jQuery.fn.offset = function( options ) {
var elem = this[0];
@@ -72,7 +75,7 @@ if ( "getBoundingClientRect" in document.documentElement ) {
top += elem.offsetTop;
left += elem.offsetLeft;
- if ( jQuery.offset.doesNotAddBorder && !(jQuery.offset.doesAddBorderForTableAndCells && /^t(able|d|h)$/i.test(elem.nodeName)) ) {
+ if ( jQuery.offset.doesNotAddBorder && !(jQuery.offset.doesAddBorderForTableAndCells && rtable.test(elem.nodeName)) ) {
top += parseFloat( computedStyle.borderTopWidth ) || 0;
left += parseFloat( computedStyle.borderLeftWidth ) || 0;
}
@@ -207,7 +210,7 @@ jQuery.fn.extend({
// Get correct offsets
offset = this.offset(),
- parentOffset = /^body|html$/i.test(offsetParent[0].nodeName) ? { top: 0, left: 0 } : offsetParent.offset();
+ parentOffset = rroot.test(offsetParent[0].nodeName) ? { top: 0, left: 0 } : offsetParent.offset();
// Subtract element margins
// note: when an element has margin: auto the offsetLeft and marginLeft
@@ -229,7 +232,7 @@ jQuery.fn.extend({
offsetParent: function() {
return this.map(function() {
var offsetParent = this.offsetParent || document.body;
- while ( offsetParent && (!/^body|html$/i.test(offsetParent.nodeName) && jQuery.css(offsetParent, "position") === "static") ) {
+ while ( offsetParent && (!rroot.test(offsetParent.nodeName) && jQuery.css(offsetParent, "position") === "static") ) {
offsetParent = offsetParent.offsetParent;
}
return offsetParent;

0 comments on commit 9dc6e0c

Please sign in to comment.