From 78d8914b80ad2172cf2ef12447eac7e671fdf305 Mon Sep 17 00:00:00 2001 From: David Ortner Date: Thu, 14 Nov 2024 01:46:41 +0100 Subject: [PATCH] fix: [#1603] HTMLSelectElement should not dispatch "change" event when changing value or index * fix: [#1603] HTMLSelectElement should not dispatch "change" event when changing value or index * chore: [#1603] Tries to fix integration test * chore: [#1603] Fixes failing integration test --- packages/happy-dom/src/ClassMethodBinder.ts | 3 +- .../html-select-element/HTMLSelectElement.ts | 15 -------- .../HTMLSelectElement.test.ts | 37 +++---------------- .../test/utilities/TestFunctions.js | 2 +- 4 files changed, 8 insertions(+), 49 deletions(-) diff --git a/packages/happy-dom/src/ClassMethodBinder.ts b/packages/happy-dom/src/ClassMethodBinder.ts index 32f63633..d9072f11 100644 --- a/packages/happy-dom/src/ClassMethodBinder.ts +++ b/packages/happy-dom/src/ClassMethodBinder.ts @@ -23,7 +23,8 @@ export default class ClassMethodBinder { * @param name Method name. */ public bind(name: string | symbol): void { - if (this.cache.has(name)) { + // We should never bind the Symbol.iterator method as it can cause problems with Array.from() + if (this.cache.has(name) || name === Symbol.iterator) { return; } diff --git a/packages/happy-dom/src/nodes/html-select-element/HTMLSelectElement.ts b/packages/happy-dom/src/nodes/html-select-element/HTMLSelectElement.ts index e3944140..bd64cb5f 100644 --- a/packages/happy-dom/src/nodes/html-select-element/HTMLSelectElement.ts +++ b/packages/happy-dom/src/nodes/html-select-element/HTMLSelectElement.ts @@ -385,7 +385,6 @@ export default class HTMLSelectElement extends HTMLElement { */ public set value(value: string) { const options = QuerySelector.querySelectorAll(this, 'option')[PropertySymbol.items]; - const previousSelectedIndex = this[PropertySymbol.selectedIndex]; this[PropertySymbol.selectedIndex] = -1; @@ -399,10 +398,6 @@ export default class HTMLSelectElement extends HTMLElement { option[PropertySymbol.selectedness] = false; } } - - if (previousSelectedIndex !== this[PropertySymbol.selectedIndex]) { - this.dispatchEvent(new Event('change', { bubbles: true, cancelable: true })); - } } /** @@ -427,7 +422,6 @@ export default class HTMLSelectElement extends HTMLElement { } const options = QuerySelector.querySelectorAll(this, 'option')[PropertySymbol.items]; - const previousSelectedIndex = this[PropertySymbol.selectedIndex]; this[PropertySymbol.selectedIndex] = -1; @@ -443,10 +437,6 @@ export default class HTMLSelectElement extends HTMLElement { this[PropertySymbol.selectedIndex] = selectedIndex; } } - - if (previousSelectedIndex !== this[PropertySymbol.selectedIndex]) { - this.dispatchEvent(new Event('change', { bubbles: true, cancelable: true })); - } } /** @@ -669,7 +659,6 @@ export default class HTMLSelectElement extends HTMLElement { const isMultiple = this.hasAttribute('multiple'); const options = QuerySelector.querySelectorAll(this, 'option')[PropertySymbol.items]; const selected: HTMLOptionElement[] = []; - const previousSelectedIndex = this[PropertySymbol.selectedIndex]; if (selectedOption) { this[PropertySymbol.selectedIndex] = -1; @@ -728,10 +717,6 @@ export default class HTMLSelectElement extends HTMLElement { } } } - - if (previousSelectedIndex !== this[PropertySymbol.selectedIndex]) { - this.dispatchEvent(new Event('change', { bubbles: true, cancelable: true })); - } } /** diff --git a/packages/happy-dom/test/nodes/html-select-element/HTMLSelectElement.test.ts b/packages/happy-dom/test/nodes/html-select-element/HTMLSelectElement.test.ts index fff4f137..3c26379f 100644 --- a/packages/happy-dom/test/nodes/html-select-element/HTMLSelectElement.test.ts +++ b/packages/happy-dom/test/nodes/html-select-element/HTMLSelectElement.test.ts @@ -95,7 +95,7 @@ describe('HTMLSelectElement', () => { expect(element.options.selectedIndex).toBe(0); }); - it('Dispatches "change" event.', () => { + it('Should not dispatch "change" event', () => { const option1 = document.createElement('option'); const option2 = document.createElement('option'); option1.value = 'option1'; @@ -106,13 +106,7 @@ describe('HTMLSelectElement', () => { let dispatchedEvent: Event | null = null; element.addEventListener('change', (event: Event) => (dispatchedEvent = event)); - element.value = 'option2'; - - expect(((dispatchedEvent)).type).toBe('change'); - - dispatchedEvent = null; - - element.value = 'option2'; + element.value = 'option1'; expect(dispatchedEvent).toBeNull(); }); @@ -282,7 +276,7 @@ describe('HTMLSelectElement', () => { expect(element.options.selectedIndex).toBe(-1); }); - it('Dispatched "change" event.', () => { + it('Should not dispatch "change" event', () => { const option1 = document.createElement('option'); const option2 = document.createElement('option'); @@ -294,12 +288,6 @@ describe('HTMLSelectElement', () => { element.selectedIndex = 1; - expect(((dispatchedEvent)).type).toBe('change'); - - dispatchedEvent = null; - - element.selectedIndex = 1; - expect(dispatchedEvent).toBeNull(); }); }); @@ -485,7 +473,7 @@ describe('HTMLSelectElement', () => { expect(element.item(2) === option3).toBe(true); }); - it('Dispatches "change" event.', () => { + it('Should not dispatch "change" event', () => { const option1 = document.createElement('option'); const option2 = document.createElement('option'); const option3 = document.createElement('option'); @@ -498,22 +486,7 @@ describe('HTMLSelectElement', () => { element.appendChild(option2); element.appendChild(option3); - expect(((dispatchedEvent)).type).toBe('change'); - expect(element.selectedIndex).toBe(0); - - dispatchedEvent = null; - - option3.selected = true; - - expect(((dispatchedEvent)).type).toBe('change'); - expect(element.selectedIndex).toBe(2); - - dispatchedEvent = null; - - option3.remove(); - - expect(((dispatchedEvent)).type).toBe('change'); - expect(element.selectedIndex).toBe(0); + expect(dispatchedEvent).toBeNull(); }); it('Sets "parentNode" of child elements to the proxy and not the original element.', () => { diff --git a/packages/integration-test/test/utilities/TestFunctions.js b/packages/integration-test/test/utilities/TestFunctions.js index cb4a35ee..88a90499 100644 --- a/packages/integration-test/test/utilities/TestFunctions.js +++ b/packages/integration-test/test/utilities/TestFunctions.js @@ -43,7 +43,7 @@ export function run(description, callback) { hasError = true; hasTimedout = true; resolve(); - }, 60000); + }, 90000); result .then(() => { if (!hasTimedout) {