Skip to content

fix: avoid problems from element IDs that exist on object prototype#4060

Merged
WilcoFiers merged 3 commits intodevelopfrom
constructor
Jun 22, 2023
Merged

fix: avoid problems from element IDs that exist on object prototype#4060
WilcoFiers merged 3 commits intodevelopfrom
constructor

Conversation

@WilcoFiers
Copy link
Copy Markdown
Contributor

Closes: #4000

@WilcoFiers WilcoFiers requested a review from a team as a code owner June 20, 2023 11:04
setLookup({ 'aria-bar': { type: 'idrefs' } });
fixture.innerHTML = `<div id="ref" aria-bar="${ids.join(
' '
)}"></div><i id="${id}"></i></b>`;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
)}"></div><i id="${id}"></i></b>`;
)}"></div><i id="${id}"></i>`;

assert.isUndefined(axe.utils.findBy(null, 'id', 'macaque'));
});

it('ignores any non-object elements in the array', function () {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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.

assert.equal(axe.utils.findBy(array, 'id', 'monkeys'), array[0]);
});

it('only looks at owned properties', function () {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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. Object properties won't work since you can't find them by value.

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'
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh wow, yeah that sure wasn't my best work ever!

'menuitemcheckbox',
'menuitemradio',
'meter',
'meter',
Copy link
Copy Markdown
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Runtime issue: n[e].push is not a function

2 participants