Skip to content

Commit

Permalink
fix: aria polyfill overrides user values and user values override int…
Browse files Browse the repository at this point in the history 
…ernals values

PiperOrigin-RevId: 565570035
  • Loading branch information
@copybara-github
Elliott Marquez authored and copybara-github committed Sep 15, 2023
1 parent af171df commit 8aa4faf
Show file tree
Hide file tree
Showing 2 changed files with 160 additions and 28 deletions.
76 changes: 48 additions & 28 deletions internal/aria/aria.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,68 +271,64 @@ export function polyfillElementInternalsAria(
throw new Error('Missing setupHostAria()');
}

let firstConnectedCallbacks: Array<() => void> = [];
let firstConnectedCallbacks:
Array<{property: ARIAProperty | 'role', callback: () => void}> = [];
let hasBeenConnected = false;

// Add support for Firefox, which has not yet implement ElementInternals aria
for (const ariaProperty of ARIA_PROPERTIES) {
let ariaValueBeforeConnected: string|null = null;
let internalAriaValue: string|null = null;
Object.defineProperty(internals, ariaProperty, {
enumerable: true,
configurable: true,
get() {
if (!hasBeenConnected) {
return ariaValueBeforeConnected;
}

// Dynamic lookup rather than hardcoding all properties.
// tslint:disable-next-line:no-dict-access-on-struct-type
return host[ariaProperty];
return internalAriaValue;
},
set(value: string|null) {
const setValue = () => {
internalAriaValue = value;
if (!hasBeenConnected) {
firstConnectedCallbacks.push(
{property: ariaProperty, callback: setValue});
return;
}

// Dynamic lookup rather than hardcoding all properties.
// tslint:disable-next-line:no-dict-access-on-struct-type
host[ariaProperty] = value;
};

if (!hasBeenConnected) {
ariaValueBeforeConnected = value;
firstConnectedCallbacks.push(setValue);
return;
}

setValue();
},
});
}

let roleValueBeforeConnected: string|null = null;
let internalRoleValue: string|null = null;
Object.defineProperty(internals, 'role', {
enumerable: true,
configurable: true,
get() {
if (!hasBeenConnected) {
return roleValueBeforeConnected;
}

return host.getAttribute('role');
return internalRoleValue;
},
set(value: string|null) {
const setRole = () => {
internalRoleValue = value;

if (!hasBeenConnected) {
firstConnectedCallbacks.push({
property: 'role',
callback: setRole,
});
return;
}

if (value === null) {
host.removeAttribute('role');
} else {
host.setAttribute('role', value);
}
};

if (!hasBeenConnected) {
roleValueBeforeConnected = value;
firstConnectedCallbacks.push(setRole);
return;
}

setRole();
},
});
Expand All @@ -344,7 +340,31 @@ export function polyfillElementInternalsAria(
}

hasBeenConnected = true;
for (const callback of firstConnectedCallbacks) {

const propertiesSetByUser = new Set<ARIAProperty|'role'>();

// See which properties were set by the user on host before we apply
// internals values as attributes to host. Needs to be done in another
// for loop because the callbacks set these attributes on host.
for (const {property} of firstConnectedCallbacks) {
const wasSetByUser =
host.getAttribute(ariaPropertyToAttribute(property)) !== null ||
// Dynamic lookup rather than hardcoding all properties.
// tslint:disable-next-line:no-dict-access-on-struct-type
host[property] !== undefined;

if (wasSetByUser) {
propertiesSetByUser.add(property);
}
}

for (const {property, callback} of firstConnectedCallbacks) {
// If the user has set the attribute or property, do not override the
// user's value
if (propertiesSetByUser.has(property)) {
continue;
}

callback();
}

Expand Down
112 changes: 112 additions & 0 deletions internal/aria/aria_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,80 @@ describe('aria', () => {
element.remove();
});

it('should not override aria attributes on host when set before connection',
async () => {
const element = new TestElement();
element.setAttribute('aria-label', 'Value set by user');
element.internals.ariaLabel = 'Value set on internals';
document.body.appendChild(element);
await element.updateComplete;
expect(element.getAttribute('aria-label'))
.withContext('aria-label attribute value on host')
.toEqual('Value set by user');
expect(element.internals.ariaLabel)
.withContext('ariaLabel internals property still the same')
.toEqual('Value set on internals');

element.remove();
});

it('should not override aria properties on host when set before connection',
async () => {
const element = new TestElement();
element.ariaLabel = 'Value set by user';
element.internals.ariaLabel = 'Value set on internals';
document.body.appendChild(element);
await element.updateComplete;
expect(element.getAttribute('aria-label'))
.withContext('aria-label attribute value on host')
.toEqual('Value set by user');
expect(element.ariaLabel)
.withContext('ariaLabel property value on host')
.toEqual('Value set by user');
expect(element.internals.ariaLabel)
.withContext('ariaLabel internals property still the same')
.toEqual('Value set on internals');

element.remove();
});

it('should not override role attribute on host when set before connection',
async () => {
const element = new TestElement();
element.setAttribute('role', 'Value set by user');
element.internals.role = 'Value set on internals';
document.body.appendChild(element);
await element.updateComplete;
expect(element.getAttribute('role'))
.withContext('role attribute value on host')
.toEqual('Value set by user');
expect(element.internals.role)
.withContext('role internals property still the same')
.toEqual('Value set on internals');

element.remove();
});

it('should not override role property on host when set before connection',
async () => {
const element = new TestElement();
element.role = 'Value set by user';
element.internals.role = 'Value set on internals';
document.body.appendChild(element);
await element.updateComplete;
expect(element.getAttribute('role'))
.withContext('role attribute value on host')
.toEqual('Value set by user');
expect(element.role)
.withContext('role property value on host')
.toEqual('Value set by user');
expect(element.internals.role)
.withContext('role internals property still the same')
.toEqual('Value set on internals');

element.remove();
});

it('should handle setting role multiple times before connection',
async () => {
const element = new TestElement();
Expand All @@ -216,6 +290,25 @@ describe('aria', () => {
element.remove();
});

it('should handle setting role multiple times before connection when property is set on host',
async () => {
const element = new TestElement();
element.role = 'radio';
element.internals.role = 'button';
element.internals.role = 'checkbox';

expect(element.internals.role)
.withContext('internals.role before connection')
.toEqual('checkbox');
document.body.appendChild(element);
await element.updateComplete;
expect(element.internals.role)
.withContext('internals.role after connection')
.toEqual('checkbox');

element.remove();
});

it('should handle setting aria properties multiple times before connection',
async () => {
const element = new TestElement();
Expand All @@ -234,6 +327,25 @@ describe('aria', () => {
element.remove();
});

it('should handle setting aria properties multiple times before connection when property is set on host',
async () => {
const element = new TestElement();
element.ariaLabel = 'First';
element.internals.ariaLabel = 'First';
element.internals.ariaLabel = 'Second';

expect(element.internals.ariaLabel)
.withContext('internals.ariaLabel before connection')
.toEqual('Second');
document.body.appendChild(element);
await element.updateComplete;
expect(element.internals.ariaLabel)
.withContext('internals.ariaLabel after connection')
.toEqual('Second');

element.remove();
});

it('should handle setting role after first connection while disconnected',
async () => {
const element = new TestElement();
Expand Down

0 comments on commit 8aa4faf

Please sign in to comment.