Skip to content

Commit

Permalink
Core: Deprecate jQuery.nodeName
Browse files Browse the repository at this point in the history 
Fixes gh-3475
Closes gh-3505
  • Loading branch information
@karan-96 @mgol
karan-96 authored and mgol committed Mar 1, 2017
1 parent bd984f0 commit ac9e301
Show file tree
Hide file tree
Showing 12 changed files with 56 additions and 36 deletions.
5 changes: 3 additions & 2 deletions src/attributes/attr.js
View file
@@ -1,10 +1,11 @@
define( [
"../core",
"../core/access",
"../core/nodeName",
"./support",
"../var/rnothtmlwhite",
"../selector"
], function( jQuery, access, support, rnothtmlwhite ) {
], function( jQuery, access, nodeName, support, rnothtmlwhite ) {

"use strict";

Expand Down Expand Up @@ -74,7 +75,7 @@ jQuery.extend( {
type: {
set: function( elem, value ) {
if ( !support.radioValue && value === "radio" &&
jQuery.nodeName( elem, "input" ) ) {
nodeName( elem, "input" ) ) {
var val = elem.value;
elem.setAttribute( "type", value );
if ( val ) {
Expand Down
7 changes: 4 additions & 3 deletions src/attributes/val.js
View file
Expand Up @@ -2,8 +2,9 @@ define( [
"../core",
"../core/stripAndCollapse",
"./support",
"../core/init"
], function( jQuery, stripAndCollapse, support ) {
"../core/init",
"../core/nodeName"
], function( jQuery, stripAndCollapse, support, nodeName ) {

"use strict";

Expand Down Expand Up @@ -121,7 +122,7 @@ jQuery.extend( {
// Don't return options that are disabled or in a disabled optgroup
!option.disabled &&
( !option.parentNode.disabled ||
!jQuery.nodeName( option.parentNode, "optgroup" ) ) ) {
!nodeName( option.parentNode, "optgroup" ) ) ) {

// Get the specific value for the option
value = jQuery( option ).val();
Expand Down
4 changes: 0 additions & 4 deletions src/core.js
View file
Expand Up @@ -289,10 +289,6 @@ jQuery.extend( {
return string.replace( rmsPrefix, "ms-" ).replace( rdashAlpha, fcamelCase );
},

nodeName: function( elem, name ) {
return elem.nodeName && elem.nodeName.toLowerCase() === name.toLowerCase();
},

each: function( obj, callback ) {
var length, i = 0;

Expand Down
13 changes: 13 additions & 0 deletions src/core/nodeName.js
View file
@@ -0,0 +1,13 @@
define( function() {

"use strict";

function nodeName( elem, name ) {

return elem.nodeName && elem.nodeName.toLowerCase() === name.toLowerCase();

};

return nodeName;

} );
8 changes: 5 additions & 3 deletions src/deprecated.js
View file
@@ -1,6 +1,7 @@
define( [
"./core"
], function( jQuery ) {
"./core",
"./core/nodeName"
], function( jQuery, nodeName ) {

"use strict";

Expand Down Expand Up @@ -29,7 +30,8 @@ jQuery.fn.extend( {
} else {
jQuery.ready( true );
}
}
},
nodeName: nodeName
} );

jQuery.isArray = Array.isArray;
Expand Down
8 changes: 5 additions & 3 deletions src/event.js
View file
Expand Up @@ -7,8 +7,10 @@ define( [
"./var/slice",
"./data/var/dataPriv",
"./core/init",
"./core/nodeName",
"./selector"
], function( jQuery, document, documentElement, rnothtmlwhite, rcheckableType, slice, dataPriv ) {
], function( jQuery, document, documentElement, rnothtmlwhite, rcheckableType, slice, dataPriv,
nodeName ) {

"use strict";

Expand Down Expand Up @@ -477,7 +479,7 @@ jQuery.event = {
// For checkable types, fire native event so checked state will be right
trigger: function() {
if ( rcheckableType.test( this.type ) &&
this.click && jQuery.nodeName( this, "input" ) ) {
this.click && nodeName( this, "input" ) ) {

this.click();
return false;
Expand All @@ -486,7 +488,7 @@ jQuery.event = {

// For cross-browser consistency, don't fire native .click() on links
_default: function( event ) {
return jQuery.nodeName( event.target, "a" );
return nodeName( event.target, "a" );
}
},

Expand Down
7 changes: 4 additions & 3 deletions src/manipulation.js
View file
Expand Up @@ -18,13 +18,14 @@ define( [
"./core/DOMEval",

"./core/init",
"./core/nodeName",

This comment has been minimized.

Sign in to view

Copy link
@dmethvin

dmethvin Jul 17, 2017

Member

This is in the wrong position, it should follow DomEval above.

This comment has been minimized.

Sign in to view

Copy link
@mgol

mgol Jul 17, 2017

Member

This is an old commit, hasn't this all been fixed already?

This comment has been minimized.

Sign in to view

Copy link
@dmethvin

dmethvin Jul 17, 2017

Member

Yep, i got lost in the commit history while tracking down a problem.
"./traversing",
"./selector",
"./event"
], function( jQuery, concat, push, rcheckableType,
access, rtagName, rscriptType,
wrapMap, getAll, setGlobalEval, buildFragment, support,
dataPriv, dataUser, acceptData, DOMEval ) {
dataPriv, dataUser, acceptData, DOMEval, nodeName ) {

"use strict";

Expand All @@ -49,8 +50,8 @@ var

// Prefer a tbody over its parent table for containing new rows
function manipulationTarget( elem, content ) {
if ( jQuery.nodeName( elem, "table" ) &&
jQuery.nodeName( content.nodeType !== 11 ? content : content.firstChild, "tr" ) ) {
if ( nodeName( elem, "table" ) &&
nodeName( content.nodeType !== 11 ? content : content.firstChild, "tr" ) ) {

return jQuery( ">tbody", elem )[ 0 ] || elem;
}
Expand Down
7 changes: 4 additions & 3 deletions src/manipulation/getAll.js
View file
@@ -1,6 +1,7 @@
define( [
"../core"
], function( jQuery ) {
"../core",
"../core/nodeName"
], function( jQuery, nodeName ) {

"use strict";

Expand All @@ -20,7 +21,7 @@ function getAll( context, tag ) {
ret = [];
}

if ( tag === undefined || tag && jQuery.nodeName( context, tag ) ) {
if ( tag === undefined || tag && nodeName( context, tag ) ) {
return jQuery.merge( [ context ], ret );
}

Expand Down
6 changes: 4 additions & 2 deletions src/offset.js
View file
Expand Up @@ -9,9 +9,11 @@ define( [
"./css/support",

"./core/init",
"./core/nodeName",

This comment has been minimized.

Sign in to view

Copy link
@dmethvin

dmethvin Jul 17, 2017

Member

This is in the wrong position, it should follow support above.
"./css",
"./selector" // contains
], function( jQuery, access, document, documentElement, rnumnonpx, curCSS, addGetHookIf, support ) {
], function( jQuery, access, document, documentElement, rnumnonpx, curCSS, addGetHookIf, support,
nodeName ) {

"use strict";

Expand Down Expand Up @@ -129,7 +131,7 @@ jQuery.fn.extend( {

// Get correct offsets
offset = this.offset();
if ( !jQuery.nodeName( offsetParent[ 0 ], "html" ) ) {
if ( !nodeName( offsetParent[ 0 ], "html" ) ) {
parentOffset = offsetParent.offset();
}

Expand Down
7 changes: 4 additions & 3 deletions src/traversing.js
View file
Expand Up @@ -5,9 +5,10 @@ define( [
"./traversing/var/siblings",
"./traversing/var/rneedsContext",
"./core/init",
"./core/nodeName",

This comment has been minimized.

Sign in to view

Copy link
@dmethvin

dmethvin Jul 17, 2017

Member

This is in the wrong position, it should follow rneedsContext above.
"./traversing/findFilter",
"./selector"
], function( jQuery, indexOf, dir, siblings, rneedsContext ) {
], function( jQuery, indexOf, dir, siblings, rneedsContext, nodeName ) {

"use strict";

Expand Down Expand Up @@ -143,14 +144,14 @@ jQuery.each( {
return siblings( elem.firstChild );
},
contents: function( elem ) {
if ( jQuery.nodeName( elem, "iframe" ) ) {
if ( nodeName( elem, "iframe" ) ) {
return elem.contentDocument;
}

// Support: IE 9 - 11 only, iOS 7 only, Android Browser <=4.3 only
// Treat the template element as a regular one in browsers that
// don't support it.
if ( jQuery.nodeName( elem, "template" ) ) {
if ( nodeName( elem, "template" ) ) {
elem = elem.content || elem;
}

Expand Down
4 changes: 2 additions & 2 deletions test/unit/core.js
View file
Expand Up @@ -696,15 +696,15 @@ QUnit.test( "jQuery(element with non-alphanumeric name)", function( assert ) {
var tagName = tag + symbol + "test";
var el = jQuery( "<" + tagName + "></" + tagName + ">" );
assert.ok( el[ 0 ], "Create a " + tagName + " element" );
assert.ok( jQuery.nodeName( el[ 0 ], tagName.toUpperCase() ),
assert.ok( el[ 0 ].nodeName === tagName.toUpperCase(),
tagName + " element has expected node name" );
}
);

var tagName = [ "tr", "multiple", "symbol" ].join( symbol );
var el = jQuery( "<" + tagName + "></" + tagName + ">" );
assert.ok( el[ 0 ], "Create a " + tagName + " element" );
assert.ok( jQuery.nodeName( el[ 0 ], tagName.toUpperCase() ),
assert.ok( el[ 0 ].nodeName === tagName.toUpperCase(),
tagName + " element has expected node name" );
} );
} );
Expand Down
16 changes: 8 additions & 8 deletions test/unit/manipulation.js
View file
Expand Up @@ -474,13 +474,13 @@ QUnit.test( "html(String) tag-hyphenated elements (Bug #1987)", function( assert
jQuery.each( "thead tbody tfoot colgroup caption tr th td".split( " " ), function( i, name ) {
var j = jQuery( "<" + name + "-d></" + name + "-d><" + name + "-d></" + name + "-d>" );
assert.ok( j[ 0 ], "Create a tag-hyphenated element" );
assert.ok( jQuery.nodeName( j[ 0 ], name.toUpperCase() + "-D" ), "Hyphenated node name" );
assert.ok( jQuery.nodeName( j[ 1 ], name.toUpperCase() + "-D" ), "Hyphenated node name" );
assert.ok( j[ 0 ].nodeName === name.toUpperCase() + "-D", "Hyphenated node name" );
assert.ok( j[ 1 ].nodeName === name.toUpperCase() + "-D", "Hyphenated node name" );
} );

var j = jQuery( "<tr-multiple-hyphens><td-with-hyphen>text</td-with-hyphen></tr-multiple-hyphens>" );
assert.ok( jQuery.nodeName( j[ 0 ], "TR-MULTIPLE-HYPHENS" ), "Tags with multiple hyphens" );
assert.ok( jQuery.nodeName( j.children()[ 0 ], "TD-WITH-HYPHEN" ), "Tags with multiple hyphens" );
assert.ok( j[ 0 ].nodeName === "TR-MULTIPLE-HYPHENS", "Tags with multiple hyphens" );
assert.ok( j.children()[ 0 ].nodeName === "TD-WITH-HYPHEN", "Tags with multiple hyphens" );
assert.equal( j.children().text(), "text", "Tags with multiple hyphens behave normally" );
} );

Expand Down Expand Up @@ -2616,14 +2616,14 @@ QUnit.test( "Make sure specific elements with content created correctly (#13232)

jQuery.each( elems, function( name, value ) {
var html = "<" + name + ">" + value + "</" + name + ">";
assert.ok( jQuery.nodeName( jQuery.parseHTML( "<" + name + ">" + value + "</" + name + ">" )[ 0 ], name ), name + " is created correctly" );
assert.ok( jQuery.parseHTML( "<" + name + ">" + value + "</" + name + ">" )[ 0 ].nodeName.toLowerCase() === name, name + " is created correctly" );

results.push( name );
args.push( html );
} );

jQuery.fn.append.apply( jQuery( "<div/>" ), args ).children().each( function( i ) {
assert.ok( jQuery.nodeName( this, results[ i ] ) );
assert.ok( this.nodeName.toLowerCase() === results[ i ] );
} );
} );

Expand All @@ -2634,11 +2634,11 @@ QUnit.test( "Validate creation of multiple quantities of certain elements (#1381

jQuery.each( tags, function( index, tag ) {
jQuery( "<" + tag + "/><" + tag + "/>" ).each( function() {
assert.ok( jQuery.nodeName( this, tag ), tag + " empty elements created correctly" );
assert.ok( this.nodeName.toLowerCase() === tag, tag + " empty elements created correctly" );
} );

jQuery( "<" + this + "></" + tag + "><" + tag + "></" + tag + ">" ).each( function() {
assert.ok( jQuery.nodeName( this, tag ), tag + " elements with closing tag created correctly" );
assert.ok( this.nodeName.toLowerCase() === tag, tag + " elements with closing tag created correctly" );
} );
} );
} );
Expand Down

3 comments on commit ac9e301

@markelog
Copy link
Member

Choose a reason for hiding this comment

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

This breaks amd build, I'm suggesting either revert or immediate fix

@timmywil
Copy link
Member

Choose a reason for hiding this comment

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

Fixed.

@timmywil
Copy link
Member

Choose a reason for hiding this comment

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

@mgol also noticed that nodeName was accidentally put on jQuery.fn instead of jQuery. See #3560.

Please sign in to comment.