Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#11317: jQuery.ifNumeric #684

Closed
wants to merge 1 commit into from
Closed

#11317: jQuery.ifNumeric #684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/core.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,8 +480,13 @@ jQuery.extend({
return obj != null && obj == obj.window;
},

ifNumeric: function( obj, fallback ) {
// multiplication by 0 coalesces finite numbers to 0 and infinities to NaN
return 0 * parseFloat( obj ) * obj + 1 ? +obj : fallback;
},

isNumeric: function( obj ) {
return !isNaN( parseFloat(obj) ) && isFinite( obj );
return jQuery.ifNumeric( obj ) !== undefined;
},

type: function( obj ) {
Expand Down
15 changes: 8 additions & 7 deletions src/css.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
(function( jQuery ) {

var ralpha = /alpha\([^)]*\)/i,
var ralpha = /alpha\([^)]*\)|$/i,
ropacity = /opacity=([^)]*)/,
// fixed for IE9, see #8346
rupper = /([A-Z]|^ms)/g,
Expand Down Expand Up @@ -319,23 +319,22 @@ if ( !jQuery.support.opacity ) {
jQuery.cssHooks.opacity = {
get: function( elem, computed ) {
// IE uses filters for opacity
return ropacity.test( (computed && elem.currentStyle ? elem.currentStyle.filter : elem.style.filter) || "" ) ?
return ropacity.test( (computed && elem.currentStyle || elem.style).filter || "" ) ?
( parseFloat( RegExp.$1 ) / 100 ) + "" :
computed ? "1" : "";
},

set: function( elem, value ) {
var style = elem.style,
currentStyle = elem.currentStyle,
opacity = jQuery.isNumeric( value ) ? "alpha(opacity=" + value * 100 + ")" : "",
filter = currentStyle && currentStyle.filter || style.filter || "";

// IE has trouble with opacity if it does not have layout
// Force it by setting the zoom level
style.zoom = 1;

// if setting opacity to 1, and no other filters exist - attempt to remove filter attribute #6652
if ( value >= 1 && jQuery.trim( filter.replace( ralpha, "" ) ) === "" ) {
if ( value >= 1 && !jQuery.trim( filter.replace( ralpha, "" ) ) ) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you read the complete discussion here: http://bugs.jquery.com/ticket/6652 or the original patch #416? The check for an empty string is very much intentional. PLEASE read tickets that are cited before making changes like this.

cc @gnarf37

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a matter of fact, I did read http://bugs.jquery.com/ticket/6652. And I also read the source for jQuery.trim, which always returns a string. The equality check may be intentional, but it is not necessary.

// Setting style.filter to null, "" & " " still leave "filter:" in the cssText
// if "filter:" is present at all, clearType is disabled, we want to avoid this
Expand All @@ -349,9 +348,11 @@ if ( !jQuery.support.opacity ) {
}

// otherwise, set new filter values
style.filter = ralpha.test( filter ) ?
filter.replace( ralpha, opacity ) :
filter + " " + opacity;
style.filter = filter.replace( ralpha, function( current, offset ) {
return jQuery.ifNumeric( value ) != null ?
( current ? "" : " " ) + "alpha(opacity=" + value * 100 + ")" :
"";
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this an improvement?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is smaller (5 bytes gzipped) and saves a redundant regular expression test in the "replacement" case.

That being said, it may or may not be faster. I've opened a jsPerf, and would appreciate IE testing if someone can help.

}
};
}
Expand Down
5 changes: 2 additions & 3 deletions src/data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,9 +333,8 @@ function dataAttr( elem, key, data ) {
data = data === "true" ? true :
data === "false" ? false :
data === "null" ? null :
jQuery.isNumeric( data ) ? +data :
rbrace.test( data ) ? jQuery.parseJSON( data ) :
data;
rbrace.test( data ) ? jQuery.parseJSON( data ) :
jQuery.ifNumeric( data, data );
} catch( e ) {}

// Make sure we set the data so it isn't changed later
Expand Down
5 changes: 2 additions & 3 deletions src/dimensions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jQuery.each( { Height: "height", Width: "width" }, function( name, type ) {

jQuery.fn[ type ] = function( value ) {
return jQuery.access( this, function( elem, type, value ) {
var doc, docElemProp, orig, ret;
var doc, docElemProp, orig;

if ( jQuery.isWindow( elem ) ) {
// 3rd condition allows Nokia support, as it supports the docElem prop but not CSS1Compat
Expand All @@ -52,8 +52,7 @@ jQuery.each( { Height: "height", Width: "width" }, function( name, type ) {
// Get width or height on the element
if ( value === undefined ) {
orig = jQuery.css( elem, type );
ret = parseFloat( orig );
return jQuery.isNumeric( ret ) ? ret : orig;
return jQuery.ifNumeric( parseFloat( orig ), orig );
}

// Set the width or height on the element
Expand Down
90 changes: 49 additions & 41 deletions test/unit/core.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,10 +448,15 @@ test("isFunction", function() {
});
});

test( "isNumeric", function() {
expect( 37 );

var t = jQuery.isNumeric,
test( "ifNumeric/isNumeric", function() {
expect( 76 );

var ifNumeric = jQuery.ifNumeric,
isNumeric = jQuery.isNumeric,
testBoth = function( input, expected, expectedIs, description ) {
strictEqual( ifNumeric(input), expected, description + " - ifNumeric" );
equal( isNumeric(input), expectedIs, description + " - isNumeric" );
},
Traditionalists = function(n) {
this.value = n;
this.toString = function(){
Expand All @@ -461,43 +466,46 @@ test( "isNumeric", function() {
answer = new Traditionalists( "42" ),
rong = new Traditionalists( "Devo" );

ok( t("-10"), "Negative integer string");
ok( t("0"), "Zero string");
ok( t("5"), "Positive integer string");
ok( t(-16), "Negative integer number");
ok( t(0), "Zero integer number");
ok( t(32), "Positive integer number");
ok( t("040"), "Octal integer literal string");
ok( t(0144), "Octal integer literal");
ok( t("0xFF"), "Hexadecimal integer literal string");
ok( t(0xFFF), "Hexadecimal integer literal");
ok( t("-1.6"), "Negative floating point string");
ok( t("4.536"), "Positive floating point string");
ok( t(-2.6), "Negative floating point number");
ok( t(3.1415), "Positive floating point number");
ok( t(8e5), "Exponential notation");
ok( t("123e-2"), "Exponential notation string");
ok( t(answer), "Custom .toString returning number");
equal( t(""), false, "Empty string");
equal( t(" "), false, "Whitespace characters string");
equal( t("\t\t"), false, "Tab characters string");
equal( t("abcdefghijklm1234567890"), false, "Alphanumeric character string");
equal( t("xabcdefx"), false, "Non-numeric character string");
equal( t(true), false, "Boolean true literal");
equal( t(false), false, "Boolean false literal");
equal( t("bcfed5.2"), false, "Number with preceding non-numeric characters");
equal( t("7.2acdgs"), false, "Number with trailling non-numeric characters");
equal( t(undefined), false, "Undefined value");
equal( t(null), false, "Null value");
equal( t(NaN), false, "NaN value");
equal( t(Infinity), false, "Infinity primitive");
equal( t(Number.POSITIVE_INFINITY), false, "Positive Infinity");
equal( t(Number.NEGATIVE_INFINITY), false, "Negative Infinity");
equal( t(rong), false, "Custom .toString returning non-number");
equal( t({}), false, "Empty object");
equal( t(function(){} ), false, "Instance of a function");
equal( t( new Date ), false, "Instance of a Date");
equal( t(function(){} ), false, "Instance of a function");
strictEqual( ifNumeric( "0", "not a number" ), 0, "Successful parse skips fallback" );
strictEqual( ifNumeric( " ", "not a number" ), "not a number", "Failed parse uses fallback" );

testBoth( "-10", -10, true, "Negative integer string");
testBoth( "0", 0, true, "Zero string");
testBoth( "5", 5, true, "Positive integer string");
testBoth( -16, -16, true, "Negative integer number");
testBoth( 0, 0, true, "Zero integer number");
testBoth( 32, 32, true, "Positive integer number");
testBoth( "040", 40, true, "Octal integer literal string");
testBoth( 0144, 0144, true, "Octal integer literal");
testBoth( "0xFF", 0xFF, true, "Hexadecimal integer literal string");
testBoth( 0xFFF, 0xFFF, true, "Hexadecimal integer literal");
testBoth( "-1.6", -1.6, true, "Negative floating point string");
testBoth( "4.536", 4.536, true, "Positive floating point string");
testBoth( -2.6, -2.6, true, "Negative floating point number");
testBoth( 3.1415, 3.1415, true, "Positive floating point number");
testBoth( 8e5, 8e5, true, "Exponential notation");
testBoth( "123e-2", 123e-2, true, "Exponential notation string");
testBoth( answer, 42, true, "Custom .toString returning number");
testBoth( "", undefined, false, "Empty string");
testBoth( " ", undefined, false, "Whitespace characters string");
testBoth( "\t\t", undefined, false, "Tab characters string");
testBoth( "abcdefghijklm1234567890", undefined, false, "Alphanumeric character string");
testBoth( "xabcdefx", undefined, false, "Non-numeric character string");
testBoth( true, undefined, false, "Boolean true literal");
testBoth( false, undefined, false, "Boolean false literal");
testBoth( "bcfed5.2", undefined, false, "Number with preceding non-numeric characters");
testBoth( "7.2acdgs", undefined, false, "Number with trailling non-numeric characters");
testBoth( undefined, undefined, false, "Undefined value");
testBoth( null, undefined, false, "Null value");
testBoth( NaN, undefined, false, "NaN value");
testBoth( Infinity, undefined, false, "Infinity primitive");
testBoth( Number.POSITIVE_INFINITY, undefined, false, "Positive Infinity");
testBoth( Number.NEGATIVE_INFINITY, undefined, false, "Negative Infinity");
testBoth( rong, undefined, false, "Custom .toString returning non-number");
testBoth( {}, undefined, false, "Empty object");
testBoth( function(){}, undefined, false, "Instance of a function");
testBoth( new Date, undefined, false, "Instance of a Date");
testBoth( function(){}, undefined, false, "Instance of a function");
});

test("isXMLDoc - HTML", function() {
Expand Down