Skip to content

Commit

Permalink
fix: [#1603] HTMLSelectElement should not dispatch "change" event whe…
Browse files Browse the repository at this point in the history
…n 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
  • Loading branch information
capricorn86 authored Nov 14, 2024
1 parent 23f8673 commit 78d8914
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 49 deletions.
3 changes: 2 additions & 1 deletion packages/happy-dom/src/ClassMethodBinder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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 }));
}
}

/**
Expand All @@ -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;

Expand All @@ -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 }));
}
}

/**
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -728,10 +717,6 @@ export default class HTMLSelectElement extends HTMLElement {
}
}
}

if (previousSelectedIndex !== this[PropertySymbol.selectedIndex]) {
this.dispatchEvent(new Event('change', { bubbles: true, cancelable: true }));
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('HTMLSelectElement', () => {
expect(element.options.selectedIndex).toBe(0);
});

it('Dispatches "change" event.', () => {
it('Should not dispatch "change" event', () => {
const option1 = <HTMLOptionElement>document.createElement('option');
const option2 = <HTMLOptionElement>document.createElement('option');
option1.value = 'option1';
Expand All @@ -106,13 +106,7 @@ describe('HTMLSelectElement', () => {
let dispatchedEvent: Event | null = null;
element.addEventListener('change', (event: Event) => (dispatchedEvent = event));

element.value = 'option2';

expect((<Event>(<unknown>dispatchedEvent)).type).toBe('change');

dispatchedEvent = null;

element.value = 'option2';
element.value = 'option1';

expect(dispatchedEvent).toBeNull();
});
Expand Down Expand Up @@ -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');

Expand All @@ -294,12 +288,6 @@ describe('HTMLSelectElement', () => {

element.selectedIndex = 1;

expect((<Event>(<unknown>dispatchedEvent)).type).toBe('change');

dispatchedEvent = null;

element.selectedIndex = 1;

expect(dispatchedEvent).toBeNull();
});
});
Expand Down Expand Up @@ -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 = <HTMLOptionElement>document.createElement('option');
const option2 = <HTMLOptionElement>document.createElement('option');
const option3 = <HTMLOptionElement>document.createElement('option');
Expand All @@ -498,22 +486,7 @@ describe('HTMLSelectElement', () => {
element.appendChild(option2);
element.appendChild(option3);

expect((<Event>(<unknown>dispatchedEvent)).type).toBe('change');
expect(element.selectedIndex).toBe(0);

dispatchedEvent = null;

option3.selected = true;

expect((<Event>(<unknown>dispatchedEvent)).type).toBe('change');
expect(element.selectedIndex).toBe(2);

dispatchedEvent = null;

option3.remove();

expect((<Event>(<unknown>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.', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/integration-test/test/utilities/TestFunctions.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export function run(description, callback) {
hasError = true;
hasTimedout = true;
resolve();
}, 60000);
}, 90000);
result
.then(() => {
if (!hasTimedout) {
Expand Down

0 comments on commit 78d8914

Please sign in to comment.