-
Notifications
You must be signed in to change notification settings - Fork 766
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
fix: avoid problems from element IDs that exist on object prototype #4060
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -80,6 +80,34 @@ describe('aria.getAccessibleRefs', function () { | |||||
assert.deepEqual(getAccessibleRefs(node), [ref]); | ||||||
}); | ||||||
|
||||||
describe('when JavaScript object names are used as IDs', function () { | ||||||
const ids = [ | ||||||
'prototype', | ||||||
'constructor', | ||||||
'__proto__', | ||||||
'Element', | ||||||
'nodeName', | ||||||
'valueOf', | ||||||
'toString' | ||||||
]; | ||||||
for (const id of ids) { | ||||||
it(`does not break with id="${id}"`, function () { | ||||||
setLookup({ 'aria-bar': { type: 'idrefs' } }); | ||||||
fixture.innerHTML = `<div id="ref" aria-bar="${ids.join( | ||||||
' ' | ||||||
)}"></div><i id="${id}"></i></b>`; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
var node = document.getElementById(id); | ||||||
var ref = document.getElementById('ref'); | ||||||
assert.deepEqual( | ||||||
getAccessibleRefs(node), | ||||||
[ref], | ||||||
`Not equal for ID ${id}` | ||||||
); | ||||||
}); | ||||||
} | ||||||
}); | ||||||
|
||||||
(shadowSupport ? it : xit)('works inside shadow DOM', function () { | ||||||
setLookup({ 'aria-bar': { type: 'idref' } }); | ||||||
fixture.innerHTML = '<div id="foo"></div>'; | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,4 +40,36 @@ describe('axe.utils.findBy', function () { | |
it('should not throw if passed falsey first parameter', function () { | ||
assert.isUndefined(axe.utils.findBy(null, 'id', 'macaque')); | ||
}); | ||
|
||
it('ignores any non-object elements in the array', function () { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test passes even without your changes, might need a different test / setup. |
||
var array = [ | ||
{ | ||
id: 'monkeys', | ||
foo: 'bar' | ||
}, | ||
'bananas', | ||
true, | ||
null, | ||
123 | ||
]; | ||
|
||
assert.equal(axe.utils.findBy(array, 'id', 'monkeys'), array[0]); | ||
}); | ||
|
||
it('only looks at owned properties', function () { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test also passes without your changes. For this test to work you'll need to create an object whos prototype parent defines the property in question. Something like this: const parent = {
value: 'bananas'
}
const child = Object.create(parent);
var array = [
child,
{
id: 'monkeys',
value: 'bananas'
}
];
assert.deepEqual(axe.utils.findBy(array, 'value', 'bananas'), {
id: 'monkeys',
value: 'bananas'
}); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh wow, yeah that sure wasn't my best work ever! |
||
var array = [ | ||
{ | ||
id: 'monkeys', | ||
Constructor: 'monkeys' | ||
}, | ||
{ | ||
id: 'bananas' | ||
} | ||
]; | ||
|
||
assert.deepEqual(axe.utils.findBy(array, 'Constructor', 'monkeys'), { | ||
id: 'monkeys', | ||
Constructor: 'monkeys' | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
??? How the heck did that happen