Skip to content

Commit

Permalink
fix: remove aria-owns to prevent Chrome crash (#2242)
Browse files Browse the repository at this point in the history
  • Loading branch information
jeripeierSBB authored and github-actions committed Apr 18, 2024
1 parent fe3420b commit f5c7608
Show file tree
Hide file tree
Showing 9 changed files with 12 additions and 45 deletions.
2 changes: 1 addition & 1 deletion src/angular/autocomplete/autocomplete-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export function getSbbAutocompleteMissingPanelError(): Error {
'[attr.aria-autocomplete]': 'autocompleteDisabled ? null : "list"',
'[attr.aria-activedescendant]': '(panelOpen && activeOption) ? activeOption.id : null',
'[attr.aria-expanded]': 'autocompleteDisabled ? null : panelOpen.toString()',
'[attr.aria-owns]': '(autocompleteDisabled || !panelOpen) ? null : autocomplete?.id',
'[attr.aria-controls]': '(autocompleteDisabled || !panelOpen) ? null : autocomplete?.id',
'[attr.aria-haspopup]': 'autocompleteDisabled ? null : "listbox"',
'[class.sbb-focused]': 'panelOpen',
// Note: we use `focusin`, as opposed to `focus`, in order to open the panel
Expand Down
14 changes: 7 additions & 7 deletions src/angular/autocomplete/autocomplete.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2329,24 +2329,24 @@ describe('SbbAutocomplete', () => {
.toBe('false');
}));

it('should set aria-owns based on the attached autocomplete', () => {
it('should set aria-controls based on the attached autocomplete', () => {
fixture.componentInstance.trigger.openPanel();
fixture.detectChanges();

const panel = fixture.debugElement.query(By.css('.sbb-autocomplete-panel')).nativeElement;

expect(input.getAttribute('aria-owns'))
.withContext('Expected aria-owns to match attached autocomplete.')
expect(input.getAttribute('aria-controls'))
.withContext('Expected aria-controls to match attached autocomplete.')
.toBe(panel.getAttribute('id'));
});

it('should not set aria-owns while the autocomplete is closed', () => {
expect(input.getAttribute('aria-owns')).toBeFalsy();
it('should not set aria-controls while the autocomplete is closed', () => {
expect(input.getAttribute('aria-controls')).toBeFalsy();

fixture.componentInstance.trigger.openPanel();
fixture.detectChanges();

expect(input.getAttribute('aria-owns')).toBeTruthy();
expect(input.getAttribute('aria-controls')).toBeTruthy();
});

it('should restore focus to the input when clicking to select a value', fakeAsync(() => {
Expand All @@ -2373,7 +2373,7 @@ describe('SbbAutocomplete', () => {
expect(input.getAttribute('role')).toBeFalsy();
expect(input.getAttribute('aria-autocomplete')).toBeFalsy();
expect(input.getAttribute('aria-expanded')).toBeFalsy();
expect(input.getAttribute('aria-owns')).toBeFalsy();
expect(input.getAttribute('aria-controls')).toBeFalsy();
});
});

Expand Down
2 changes: 1 addition & 1 deletion src/angular/autocomplete/autocomplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ export class SbbAutocomplete implements AfterContentInit, OnDestroy {
/** If set to true, the panel is also displayed if there are no options but hints. */
@Input({ transform: booleanAttribute }) showHintIfNoOptions: boolean = false;

/** Unique ID to be used by autocomplete trigger's "aria-owns" property. */
/** Unique ID to be used by autocomplete trigger's "aria-controls" property. */
id: string = `sbb-autocomplete-${nextId++}`;

/**
Expand Down
1 change: 0 additions & 1 deletion src/angular/chips/chip-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,6 @@ describe('SbbChipList', () => {

expect(label.getAttribute('for')).toBeTruthy();
expect(label.getAttribute('for')).toBe(input.getAttribute('id'));
expect(label.getAttribute('aria-owns')).toBe(input.getAttribute('id'));
});
});

Expand Down
2 changes: 1 addition & 1 deletion src/angular/core/option/option-hint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ let nextId = 0;
},
})
export class SbbOptionHint {
/** Unique ID to be used by autocomplete trigger's "aria-owns" property. */
/** Unique ID to be used by autocomplete trigger's "aria-controls" property. */
id: string = `sbb-option-hint-${nextId++}`;
}
3 changes: 0 additions & 3 deletions src/angular/form-field/form-field.html
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
<div class="sbb-form-field-wrapper">
<div class="sbb-form-field-label-wrapper" (click)="_control.onContainerClick($event)">
<!-- We add aria-owns as a workaround for an issue in JAWS & NVDA where the label isn't
read if it comes before the control in the DOM. -->
<label
class="sbb-label sbb-form-field-label"
[id]="_labelId"
[attr.for]="_control.id"
[attr.aria-owns]="_control.id"
[class.sbb-form-field-empty]="_control.empty"
>
@switch (_hasLabel()) {
Expand Down
14 changes: 0 additions & 14 deletions src/angular/input/input.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,20 +152,6 @@ describe('SbbInput without forms', () => {
expect(inputElement.id).toEqual(labelElement.getAttribute('for')!);
}));

it('should add aria-owns to the label for the associated control', fakeAsync(() => {
const fixture = createComponent(SbbInputTextTestController);
fixture.detectChanges();

const inputElement: HTMLInputElement = fixture.debugElement.query(
By.css('input'),
)!.nativeElement;
const labelElement: HTMLInputElement = fixture.debugElement.query(
By.css('label'),
)!.nativeElement;

expect(labelElement.getAttribute('aria-owns')).toBe(inputElement.id);
}));

it('should add aria-required reflecting the required state', fakeAsync(() => {
const fixture = createComponent(SbbInputWithRequired);
fixture.detectChanges();
Expand Down
13 changes: 1 addition & 12 deletions src/angular/select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -988,7 +988,7 @@ describe('SbbSelect', () => {
it('should set the role of the select to combobox', fakeAsync(() => {
expect(select.getAttribute('role')).toEqual('combobox');
expect(select.getAttribute('aria-autocomplete')).toBe('none');
expect(select.getAttribute('aria-haspopup')).toBe('true');
expect(select.getAttribute('aria-haspopup')).toBe('listbox');
}));

it('should point the aria-controls attribute to the listbox', fakeAsync(() => {
Expand All @@ -1003,17 +1003,6 @@ describe('SbbSelect', () => {
expect(ariaControls).toBe(document.querySelector('.sbb-panel')!.id);
}));

it('should point the aria-owns attribute to the listbox on the trigger', fakeAsync(() => {
expect(select.hasAttribute('aria-owns')).toBe(false);
fixture.componentInstance.select.open();
fixture.detectChanges();
flush();

const ariaOwns = select.getAttribute('aria-owns');
expect(ariaOwns).toBeTruthy();
expect(ariaOwns).toBe(document.querySelector('.sbb-panel')!.id);
}));

it('should set aria-expanded based on the select open state', fakeAsync(() => {
expect(select.getAttribute('aria-expanded')).toBe('false');

Expand Down
6 changes: 1 addition & 5 deletions src/angular/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,15 +154,11 @@ const _SbbSelectMixinBase = mixinVariant(
host: {
role: 'combobox',
'aria-autocomplete': 'none',
// TODO(crisbeto): the value for aria-haspopup should be `listbox`, but currently it's difficult
// to sync into Google, because of an outdated automated a11y check which flags it as an invalid
// value. At some point we should try to switch it back to being `listbox`.
'aria-haspopup': 'true',
'aria-haspopup': 'listbox',
class: 'sbb-select sbb-input-element',
'[attr.id]': 'id',
'[attr.tabindex]': 'readonly || disabled ? -1 : tabIndex',
'[attr.aria-controls]': 'panelOpen ? id + "-panel" : null',
'[attr.aria-owns]': "panelOpen ? id + '-panel' : null",
'[attr.aria-expanded]': 'panelOpen',
'[attr.aria-label]': 'ariaLabel || null',
'[attr.aria-required]': 'required.toString()',
Expand Down

0 comments on commit f5c7608

Please sign in to comment.