Skip to content

Commit 230e22b

Browse files
devversionjelbourn
authored andcommitted
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
1 parent 9914312 commit 230e22b

File tree

2 files changed

+30
-16
lines changed

2 files changed

+30
-16
lines changed

src/cdk/a11y/key-manager/list-key-manager.spec.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,16 @@ describe('Key managers', () => {
467467
expect(keyManager.setActiveItem).toHaveBeenCalledWith(0);
468468
});
469469

470+
// This test should pass if all items are disabled and the down arrow key got pressed.
471+
// If the test setup crashes or this test times out, this test can be considered as failed.
472+
it('should not get into an infinite loop if all items are disabled', () => {
473+
keyManager.withWrap();
474+
keyManager.setActiveItem(0);
475+
476+
itemList.items.forEach(item => item.disabled = true);
477+
478+
keyManager.onKeydown(fakeKeyEvents.downArrow);
479+
});
470480
});
471481

472482
describe('typeahead mode', () => {

src/cdk/a11y/key-manager/list-key-manager.ts

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
259259
* currently active item and the new active item. It will calculate differently
260260
* depending on whether wrap mode is turned on.
261261
*/
262-
private _setActiveItemByDelta(delta: number, items = this._items.toArray()): void {
262+
private _setActiveItemByDelta(delta: -1 | 1, items = this._items.toArray()): void {
263263
this._wrap ? this._setActiveInWrapMode(delta, items)
264264
: this._setActiveInDefaultMode(delta, items);
265265
}
@@ -269,16 +269,15 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
269269
* down the list until it finds an item that is not disabled, and it will wrap if it
270270
* encounters either end of the list.
271271
*/
272-
private _setActiveInWrapMode(delta: number, items: T[]): void {
273-
// when active item would leave menu, wrap to beginning or end
274-
this._activeItemIndex =
275-
(this._activeItemIndex + delta + items.length) % items.length;
276-
277-
// skip all disabled menu items recursively until an enabled one is reached
278-
if (items[this._activeItemIndex].disabled) {
279-
this._setActiveInWrapMode(delta, items);
280-
} else {
281-
this.setActiveItem(this._activeItemIndex);
272+
private _setActiveInWrapMode(delta: -1 | 1, items: T[]): void {
273+
for (let i = 1; i <= items.length; i++) {
274+
const index = (this._activeItemIndex + (delta * i) + items.length) % items.length;
275+
const item = items[index];
276+
277+
if (!item.disabled) {
278+
this.setActiveItem(index);
279+
return;
280+
}
282281
}
283282
}
284283

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

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

303304
while (items[index].disabled) {
304305
index += fallbackDelta;
305-
if (!items[index]) { return; }
306+
307+
if (!items[index]) {
308+
return;
309+
}
306310
}
307311

308312
this.setActiveItem(index);

0 commit comments

Comments
 (0)