Skip to content

fix: extra protection around tagName accessor #13

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.

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

Merged
merged 2 commits into from
May 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 24 additions & 1 deletion src/getTag.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
function isString(value) {
return typeof value === 'string';
}

/**
* Returns the Tag of the element
* @param { Object } element
Expand All @@ -6,7 +10,26 @@
*/
export function getTag( el, filter )
{
const tagName = el.tagName.toLowerCase().replace(/:/g, '\\:')
let tagName = el.tagName;

// If the tagName attribute has been overridden, we should
// check the nodeName property instead. If the nodeName property
// is also not a string, we should return null and ignore tagName
// for selector generation.
//
// This can happen when a <form> element contains an <input>
// with an id of `tagName`. In this case, the form element's
// tagName property is a reference to the input element, not
// a string.
if (!isString(tagName)) {
tagName = el.nodeName;

if (!isString(tagName)) {
return null;
}
}

tagName = tagName.toLowerCase().replace(/:/g, '\\:')

if (filter && !filter('tag', 'tag', tagName)) {
return null;
Expand Down
69 changes: 68 additions & 1 deletion test/unique-selector.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ describe( 'Unique Selector Tests', () =>
expect( uniqueSelector ).to.equal( 'span' );
} );


it( 'Tag', () =>
{
$( 'body' ).append( '<div class="test5"><span></span></div><div class="test5"><span></span></div>' );
Expand All @@ -129,6 +128,74 @@ describe( 'Unique Selector Tests', () =>
expect( uniqueSelector ).to.equal( 'a' );
} );

it( 'Tag - fallback to nodeName', () =>
{
$( 'body' ).append(`
<div class="test2">
<form action="" method="get">
<div class="form-example">
<label for="name">Enter your name: </label>
<input type="text" name="name" id="tagName" required />
</div>
</form>
</div>
`);

const formNode = $( 'form' ).get( 0 );

// JSDOM doesn't actually exhibit this behavior;
// forcing the test to behave as a browser does.
Object.defineProperty(formNode, 'tagName', {
get: () => {
return $( 'input#tagName' ).get( 0 );
}
})

expect(typeof formNode.tagName).to.not.equal('string')

const uniqueSelector = unique( formNode );
// nodeName === 'form'
expect( uniqueSelector ).to.equal( 'form' );
} );

it( 'Tag - ignored due to property override', () =>
{
$( 'body' ).append(`
<div class="test2">
<form action="" method="get">
<div class="form-example">
<label for="name">Enter your name: </label>
<input type="text" name="name" id="tagName" required />
</div>
</form>
</div>
`);

const formNode = $( 'form' ).get( 0 );

// JSDOM doesn't actually exhibit this behavior;
// forcing the test to behave as a browser does.
Object.defineProperty(formNode, 'tagName', {
get: () => {
return $( 'input#tagName' ).get( 0 );
}
})
Object.defineProperty(formNode, 'nodeName', {
get: () => {
return $( 'input#tagName' ).get( 0 );
}
})

expect(typeof formNode.tagName).to.not.equal('string')
expect(typeof formNode.nodeName).to.not.equal('string')

const uniqueSelector = unique( formNode );
// with nodeName overridden, the isElement check will fail
// and the wildcard selector is returned for that element.
// This really shouldn't happen in practice.
expect( uniqueSelector ).to.equal( '.test2 > *' );
} );

it( 'Attributes', () =>
{
$( 'body' ).append( '<div class="test5" test="5"></div>' );
Expand Down