Skip to content

Commit

Permalink
Merge pull request #636 from capricorn86/task/635-setting-an-invalid-…
Browse files Browse the repository at this point in the history 
…value-to-htmlselectelementselectedindex-causes-a-crash

#635@patch: Multiples fixes regarding how HTMLSelectElement.selectedI…
  • Loading branch information
@capricorn86
capricorn86 committed Oct 24, 2022
2 parents b02d1c7 + 67e8bca commit 76c09bb
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 75 deletions.
40 changes: 35 additions & 5 deletions packages/happy-dom/src/nodes/html-option-element/HTMLOptionElement.ts
View file
@@ -1,6 +1,7 @@
import HTMLElement from '../html-element/HTMLElement';
import IHTMLElement from '../html-element/IHTMLElement';
import IHTMLFormElement from '../html-form-element/IHTMLFormElement';
import IHTMLSelectElement from '../html-select-element/IHTMLSelectElement';
import IHTMLOptionElement from './IHTMLOptionElement';

/**
Expand Down Expand Up @@ -58,7 +59,20 @@ export default class HTMLOptionElement extends HTMLElement implements IHTMLOptio
* @returns Selected.
*/
public get selected(): boolean {
return this.getAttributeNS(null, 'selected') !== null;
const parentNode = <IHTMLSelectElement>this.parentNode;

if (parentNode?.tagName === 'SELECT') {
let index = -1;
for (let i = 0; i < parentNode.options.length; i++) {
if (parentNode.options[i] === this) {
index = i;
break;
}
}
return index !== -1 && parentNode.options.selectedIndex === index;
}

return false;
}

/**
Expand All @@ -67,10 +81,26 @@ export default class HTMLOptionElement extends HTMLElement implements IHTMLOptio
* @param selected Selected.
*/
public set selected(selected: boolean) {
if (!selected) {
this.removeAttributeNS(null, 'selected');
} else {
this.setAttributeNS(null, 'selected', '');
const parentNode = <IHTMLSelectElement>this.parentNode;
if (parentNode?.tagName === 'SELECT') {
if (selected) {
let index = -1;

for (let i = 0; i < parentNode.options.length; i++) {
if (parentNode.options[i] === this) {
index = i;
break;
}
}

if (index !== -1) {
parentNode.options.selectedIndex = index;
}
} else if (parentNode.options.length) {
parentNode.options.selectedIndex = 0;
} else {
parentNode.options.selectedIndex = -1;
}
}
}

Expand Down
20 changes: 7 additions & 13 deletions packages/happy-dom/src/nodes/html-option-element/HTMLOptionsCollection.ts
View file
Expand Up @@ -2,7 +2,6 @@ import DOMException from '../../exception/DOMException';
import HTMLCollection from '../element/HTMLCollection';
import IHTMLOptGroupElement from '../html-opt-group-element/IHTMLOptGroupElement';
import IHTMLSelectElement from '../html-select-element/IHTMLSelectElement';
import HTMLOptionElement from './HTMLOptionElement';
import IHTMLOptionElement from './IHTMLOptionElement';
import IHTMLOptionsCollection from './IHTMLOptionsCollection';

Expand All @@ -17,6 +16,7 @@ export default class HTMLOptionsCollection
implements IHTMLOptionsCollection
{
private _selectElement: IHTMLSelectElement;
private _selectedIndex = -1;

/**
*
Expand All @@ -34,14 +34,7 @@ export default class HTMLOptionsCollection
* @returns SelectedIndex.
*/
public get selectedIndex(): number {
for (let i = 0; i < this.length; i++) {
const item = this[i];
if (item instanceof HTMLOptionElement && item.selected) {
return i;
}
}

return -1;
return this._selectedIndex;
}

/**
Expand All @@ -50,10 +43,11 @@ export default class HTMLOptionsCollection
* @param selectedIndex SelectedIndex.
*/
public set selectedIndex(selectedIndex: number) {
for (let i = 0; i < this.length; i++) {
const item = this[i];
if (item instanceof HTMLOptionElement) {
this[i].selected = i === selectedIndex;
if (typeof selectedIndex === 'number' && !isNaN(selectedIndex)) {
if (selectedIndex >= 0 && selectedIndex < this.length) {
this._selectedIndex = selectedIndex;
} else {
this._selectedIndex = -1;
}
}
}
Expand Down
26 changes: 17 additions & 9 deletions packages/happy-dom/src/nodes/html-select-element/HTMLSelectElement.ts
View file
@@ -1,5 +1,3 @@
import DOMException from '../../exception/DOMException';
import DOMExceptionNameEnum from '../../exception/DOMExceptionNameEnum';
import HTMLElement from '../html-element/HTMLElement';
import IHTMLElement from '../html-element/IHTMLElement';
import IHTMLFormElement from '../html-form-element/IHTMLFormElement';
Expand Down Expand Up @@ -204,13 +202,6 @@ export default class HTMLSelectElement extends HTMLElement implements IHTMLSelec
* @param value Value.
*/
public set selectedIndex(value: number) {
if (value > this.options.length - 1 || value < -1) {
throw new DOMException(
'Select elements selected index must be valid',
DOMExceptionNameEnum.indexSizeError
);
}

this.options.selectedIndex = value;
}

Expand Down Expand Up @@ -296,6 +287,10 @@ export default class HTMLSelectElement extends HTMLElement implements IHTMLSelec

if (element.tagName === 'OPTION' || element.tagName === 'OPTGROUP') {
this.options.push(<IHTMLOptionElement | IHTMLOptGroupElement>element);

if (this.options.length === 1) {
this.options.selectedIndex = 0;
}
}

this._updateIndexProperties(previousLength, this.options.length);
Expand Down Expand Up @@ -337,6 +332,10 @@ export default class HTMLSelectElement extends HTMLElement implements IHTMLSelec
} else {
this.options.push(<IHTMLOptionElement | IHTMLOptGroupElement>newElement);
}

if (this.options.length === 1) {
this.options.selectedIndex = 0;
}
}

this._updateIndexProperties(previousLength, this.options.length);
Expand All @@ -355,9 +354,18 @@ export default class HTMLSelectElement extends HTMLElement implements IHTMLSelec

if (element.tagName === 'OPTION' || element.tagName === 'OPTION') {
const index = this.options.indexOf(<IHTMLOptionElement | IHTMLOptGroupElement>node);

if (index !== -1) {
this.options.splice(index, 1);
}

if (this.options.selectedIndex >= this.options.length) {
this.options.selectedIndex = this.options.length - 1;
}

if (!this.options.length) {
this.options.selectedIndex = -1;
}
}

this._updateIndexProperties(previousLength, this.options.length);
Expand Down
91 changes: 75 additions & 16 deletions packages/happy-dom/test/nodes/html-option-element/HTMLOptionElement.test.ts
View file
@@ -1,16 +1,17 @@
import Window from '../../../src/window/Window';
import Document from '../../../src/nodes/document/Document';
import HTMLOptionElement from '../../../src/nodes/html-option-element/HTMLOptionElement';
import IHTMLOptionElement from '../../../src/nodes/html-option-element/IHTMLOptionElement';
import IHTMLSelectElement from '../../../src/nodes/html-select-element/IHTMLSelectElement';

describe('HTMLOptionElement', () => {
let window: Window;
let document: Document;
let element: HTMLOptionElement;
let element: IHTMLOptionElement;

beforeEach(() => {
window = new Window();
document = window.document;
element = <HTMLOptionElement>document.createElement('option');
element = <IHTMLOptionElement>document.createElement('option');
});

describe('Object.prototype.toString', () => {
Expand All @@ -33,20 +34,78 @@ describe('HTMLOptionElement', () => {
});
});

for (const property of ['disabled', 'selected']) {
describe(`get ${property}()`, () => {
it('Returns attribute value.', () => {
expect(element[property]).toBe(false);
element.setAttribute(property, '');
expect(element[property]).toBe(true);
});
describe('get disabled()', () => {
it('Returns the attribute "disabled".', () => {
element.setAttribute('disabled', '');
expect(element.disabled).toBe(true);
});
});

describe(`set ${property}()`, () => {
it('Sets attribute value.', () => {
element[property] = true;
expect(element.getAttribute(property)).toBe('');
});
describe('set disabled()', () => {
it('Sets the attribute "disabled".', () => {
element.disabled = true;
expect(element.getAttribute('disabled')).toBe('');
});
}
});

describe('get selected()', () => {
it('Returns the selected state of the option.', () => {
const select = <IHTMLSelectElement>document.createElement('select');
const option1 = <IHTMLOptionElement>document.createElement('option');
const option2 = <IHTMLOptionElement>document.createElement('option');

expect(option1.selected).toBe(false);
expect(option2.selected).toBe(false);

select.appendChild(option1);
select.appendChild(option2);

expect(option1.selected).toBe(true);
expect(option2.selected).toBe(false);
expect(option1.getAttribute('selected')).toBe(null);
expect(option2.getAttribute('selected')).toBe(null);

select.options.selectedIndex = 1;

expect(option1.selected).toBe(false);
expect(option2.selected).toBe(true);
expect(option1.getAttribute('selected')).toBe(null);
expect(option2.getAttribute('selected')).toBe(null);

select.options.selectedIndex = -1;

expect(option1.selected).toBe(false);
expect(option2.selected).toBe(false);
});
});

describe('set selected()', () => {
it('Sets the selected state of the option.', () => {
const select = <IHTMLSelectElement>document.createElement('select');
const option1 = <IHTMLOptionElement>document.createElement('option');
const option2 = <IHTMLOptionElement>document.createElement('option');

expect(option1.selected).toBe(false);
expect(option2.selected).toBe(false);

option1.selected = true;

expect(select.selectedIndex).toBe(-1);

select.appendChild(option1);
select.appendChild(option2);

option1.selected = true;

expect(select.selectedIndex).toBe(0);

option2.selected = true;

expect(select.selectedIndex).toBe(1);

option2.selected = false;

expect(select.selectedIndex).toBe(0);
});
});
});
59 changes: 29 additions & 30 deletions packages/happy-dom/test/nodes/html-option-element/HTMLOptionsCollection.test.ts
View file
Expand Up @@ -3,7 +3,7 @@ import IWindow from '../../../src/window/IWindow';
import IDocument from '../../../src/nodes/document/IDocument';
import HTMLSelectElement from '../../../src/nodes/html-select-element/HTMLSelectElement';
import HTMLOptionElement from '../../../src/nodes/html-option-element/HTMLOptionElement';
import { DOMException } from '../../../src';
import DOMException from '../../../src/exception/DOMException';

describe('HTMLOptionsCollection', () => {
let window: IWindow;
Expand All @@ -18,57 +18,51 @@ describe('HTMLOptionsCollection', () => {
jest.restoreAllMocks();
});

describe('get selectedindex()', () => {
it('Returns the index of the first option element in the list of options in tree order that has its selectedness set to true.', () => {
const select = <HTMLSelectElement>document.createElement('select');
const option1 = <HTMLOptionElement>document.createElement('option');
const option2 = <HTMLOptionElement>document.createElement('option');
option1.selected = true;
option1.value = 'option1';
option2.value = 'option2';
select.appendChild(option1);
select.appendChild(option2);

expect(select.options.selectedIndex).toBe(0);
});

describe('get selectedIndex()', () => {
it('Returns -1 if there are no options.', () => {
const select = <HTMLSelectElement>document.createElement('select');
expect(select.options.selectedIndex).toBe(-1);
});

it('Returns -1 if no option is selected.', () => {
it('Returns 0 by default.', () => {
const select = <HTMLSelectElement>document.createElement('select');
const option1 = <HTMLOptionElement>document.createElement('option');
const option2 = <HTMLOptionElement>document.createElement('option');

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

select.appendChild(option1);
select.appendChild(option2);

expect(select.options.selectedIndex).toBe(-1);
expect(select.options.selectedIndex).toBe(0);
});
});

describe('set selectedindex()', () => {
describe('set selectedIndex()', () => {
it('Updates option.selected', () => {
const select = <HTMLSelectElement>document.createElement('select');
select.appendChild(document.createElement('option'));
select.appendChild(document.createElement('option'));
document.body.appendChild(select);
const option1 = <HTMLOptionElement>document.createElement('option');
const option2 = <HTMLOptionElement>document.createElement('option');

expect(option1.selected).toBe(false);
expect(option2.selected).toBe(false);

select.appendChild(option1);
select.appendChild(option2);

expect((<HTMLOptionElement>select.options[0]).selected).toBe(false);
expect((<HTMLOptionElement>select.options[1]).selected).toBe(false);
expect(option1.selected).toBe(true);
expect(option2.selected).toBe(false);

select.options.selectedIndex = 1;

expect((<HTMLOptionElement>select.options[0]).selected).toBe(false);
expect((<HTMLOptionElement>select.options[1]).selected).toBe(true);
expect(option1.selected).toBe(false);
expect(option2.selected).toBe(true);

select.options.selectedIndex = -1;

expect((<HTMLOptionElement>select.options[0]).selected).toBe(false);
expect((<HTMLOptionElement>select.options[1]).selected).toBe(false);
expect(option1.selected).toBe(false);
expect(option2.selected).toBe(false);
});
});

Expand Down Expand Up @@ -134,16 +128,21 @@ describe('HTMLOptionsCollection', () => {
const select = <HTMLSelectElement>document.createElement('select');
const option = document.createElement('option');
const option2 = document.createElement('option');

expect(select.options.selectedIndex).toBe(-1);

select.appendChild(option);
select.appendChild(option2);
document.body.appendChild(select);
expect(select.options.selectedIndex).toBe(-1);

expect(select.options.selectedIndex).toBe(0);

select.options.selectedIndex = 1;
expect(select.options.selectedIndex).toBe(1);

// No option is selected after removing the selected option
select.options.remove(1);
expect(select.options.selectedIndex).toBe(0);

select.options.remove(0);
expect(select.options.selectedIndex).toBe(-1);
});
});
Expand Down

0 comments on commit 76c09bb

Please sign in to comment.