From 230e22babb201361461f6de9f1d30437074b79b6 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Sun, 18 Feb 2018 01:48:20 +0100 Subject: [PATCH] fix(list-key-manager): infinite loop if all items are disabled (#9981) 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 --- .../a11y/key-manager/list-key-manager.spec.ts | 10 ++++++ src/cdk/a11y/key-manager/list-key-manager.ts | 34 +++++++++++-------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/cdk/a11y/key-manager/list-key-manager.spec.ts b/src/cdk/a11y/key-manager/list-key-manager.spec.ts index ace4a24520d..93e32ddd4d2 100644 --- a/src/cdk/a11y/key-manager/list-key-manager.spec.ts +++ b/src/cdk/a11y/key-manager/list-key-manager.spec.ts @@ -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', () => { diff --git a/src/cdk/a11y/key-manager/list-key-manager.ts b/src/cdk/a11y/key-manager/list-key-manager.ts index d11782a0d8b..c3178d4e045 100644 --- a/src/cdk/a11y/key-manager/list-key-manager.ts +++ b/src/cdk/a11y/key-manager/list-key-manager.ts @@ -259,7 +259,7 @@ export class ListKeyManager { * 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); } @@ -269,16 +269,15 @@ export class ListKeyManager { * 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; + 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]; - // 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); + if (!item.disabled) { + this.setActiveItem(index); + return; + } } } @@ -287,7 +286,7 @@ export class ListKeyManager { * 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); } @@ -296,13 +295,18 @@ export class ListKeyManager { * 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);