From bb7e840fac2de2f6f1f08309bf6397ae9df58b00 Mon Sep 17 00:00:00 2001 From: dhellgartner <116464099+dhellgartner@users.noreply.github.com> Date: Thu, 24 Oct 2024 22:22:47 +0200 Subject: [PATCH] Issue 2117: Fix sl-select throws exception if a sl-input in the same form receives a new value with autofill in MS Edge (#2221) * Issue 2117: Created a test to reproduce the issue Note: Test is currently fainling before the actual fix * Issue 2117: Check for the existenc of the key * Bug is caused by an incorrect event triggered by Edge * The event is missing the key property * To fix the bug, check for the existence of the property before checking the length property * Issue 2117: Reset delay in test --------- Co-authored-by: Dominikus Hellgartner --- src/components/select/select.component.ts | 2 +- src/components/select/select.test.ts | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/components/select/select.component.ts b/src/components/select/select.component.ts index 6dd5f21a..8698c761 100644 --- a/src/components/select/select.component.ts +++ b/src/components/select/select.component.ts @@ -373,7 +373,7 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon } // All other "printable" keys trigger type to select - if (event.key.length === 1 || event.key === 'Backspace') { + if ((event.key && event.key.length === 1) || event.key === 'Backspace') { const allOptions = this.getAllOptions(); // Don't block important key combos like CMD+R diff --git a/src/components/select/select.test.ts b/src/components/select/select.test.ts index a1967ec1..c23d9821 100644 --- a/src/components/select/select.test.ts +++ b/src/components/select/select.test.ts @@ -206,6 +206,29 @@ describe('', () => { expect(handler).to.be.calledTwice; }); + + // this can happen in on ms-edge autofilling an associated input element in the same form + // https://github.com/shoelace-style/shoelace/issues/2117 + it('should not throw on incomplete events', async () => { + const el = await fixture(html` + + Option 1 + + `); + + const event = new KeyboardEvent('keydown'); + Object.defineProperty(event, 'target', { writable: false, value: el }); + Object.defineProperty(event, 'key', { writable: false, value: undefined }); + + /** + * If Edge does autofill, it creates a broken KeyboardEvent + * which is missing the key value. + * Using the normal dispatch mechanism does not allow to do this + * Thus passing the event directly to the private method for testing + * + * @ts-expect-error */ + el.handleDocumentKeyDown(event); + }); }); it('should open the listbox when any letter key is pressed with sl-select is on focus', async () => {