Skip to content

Commit

Permalink
fix(list-key-manager): infinite loop if all items are disabled (#9981)
Browse files Browse the repository at this point in the history
Currently if the `ListKeyManager` is being used in wrap mode, and all items are disabled, pressing the down arrow key will cause the key manager to go into an infinite loop that searches for the next item.

Related #9963
  • Loading branch information
devversion authored and andrewseguin committed Feb 20, 2018
1 parent 5e84808 commit 775f560
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 16 deletions.
10 changes: 10 additions & 0 deletions src/cdk/a11y/key-manager/list-key-manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,16 @@ describe('Key managers', () => {
expect(keyManager.setActiveItem).toHaveBeenCalledWith(0);
});

// This test should pass if all items are disabled and the down arrow key got pressed.
// If the test setup crashes or this test times out, this test can be considered as failed.
it('should not get into an infinite loop if all items are disabled', () => {
keyManager.withWrap();
keyManager.setActiveItem(0);

itemList.items.forEach(item => item.disabled = true);

keyManager.onKeydown(fakeKeyEvents.downArrow);
});
});

describe('typeahead mode', () => {
Expand Down
36 changes: 20 additions & 16 deletions src/cdk/a11y/key-manager/list-key-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
* currently active item and the new active item. It will calculate differently
* depending on whether wrap mode is turned on.
*/
private _setActiveItemByDelta(delta: number, items = this._items.toArray()): void {
private _setActiveItemByDelta(delta: -1 | 1, items = this._items.toArray()): void {
this._wrap ? this._setActiveInWrapMode(delta, items)
: this._setActiveInDefaultMode(delta, items);
}
Expand All @@ -269,16 +269,15 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
* down the list until it finds an item that is not disabled, and it will wrap if it
* encounters either end of the list.
*/
private _setActiveInWrapMode(delta: number, items: T[]): void {
// when active item would leave menu, wrap to beginning or end
this._activeItemIndex =
(this._activeItemIndex + delta + items.length) % items.length;

// skip all disabled menu items recursively until an enabled one is reached
if (items[this._activeItemIndex].disabled) {
this._setActiveInWrapMode(delta, items);
} else {
this.setActiveItem(this._activeItemIndex);
private _setActiveInWrapMode(delta: -1 | 1, items: T[]): void {
for (let i = 1; i <= items.length; i++) {
const index = (this._activeItemIndex + (delta * i) + items.length) % items.length;
const item = items[index];

if (!item.disabled) {
this.setActiveItem(index);
return;
}
}
}

Expand All @@ -287,7 +286,7 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
* continue to move down the list until it finds an item that is not disabled. If
* it encounters either end of the list, it will stop and not wrap.
*/
private _setActiveInDefaultMode(delta: number, items: T[]): void {
private _setActiveInDefaultMode(delta: -1 | 1, items: T[]): void {
this._setActiveItemByIndex(this._activeItemIndex + delta, delta, items);
}

Expand All @@ -296,13 +295,18 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
* item is disabled, it will move in the fallbackDelta direction until it either
* finds an enabled item or encounters the end of the list.
*/
private _setActiveItemByIndex(index: number, fallbackDelta: number,
items = this._items.toArray()): void {
if (!items[index]) { return; }
private _setActiveItemByIndex(index: number, fallbackDelta: -1 | 1,
items = this._items.toArray()): void {
if (!items[index]) {
return;
}

while (items[index].disabled) {
index += fallbackDelta;
if (!items[index]) { return; }

if (!items[index]) {
return;
}
}

this.setActiveItem(index);
Expand Down

0 comments on commit 775f560

Please sign in to comment.