From e36d5b201abc64025cfef286ddeae9c81c735275 Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Wed, 17 Jun 2020 06:50:43 -0400 Subject: [PATCH] fix(forms): correct usage of `selectedOptions` (#37620) Previously, `registerOnChange` used `hasOwnProperty` to identify if the property is supported. However, this does not work as the `selectedOptions` property is an inherited property. This commit fixes this by verifying the property on the prototype instead. Closes #37433 PR Close #37620 --- .../select_multiple_control_value_accessor.ts | 2 +- .../forms/test/value_accessor_integration_spec.ts | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/forms/src/directives/select_multiple_control_value_accessor.ts b/packages/forms/src/directives/select_multiple_control_value_accessor.ts index c7201bdece..ebee781504 100644 --- a/packages/forms/src/directives/select_multiple_control_value_accessor.ts +++ b/packages/forms/src/directives/select_multiple_control_value_accessor.ts @@ -156,7 +156,7 @@ export class SelectMultipleControlValueAccessor implements ControlValueAccessor registerOnChange(fn: (value: any) => any): void { this.onChange = (_: any) => { const selected: Array = []; - if (_.hasOwnProperty('selectedOptions')) { + if (_.selectedOptions !== undefined) { const options: HTMLCollection = _.selectedOptions; for (let i = 0; i < options.length; i++) { const opt: any = options.item(i); diff --git a/packages/forms/test/value_accessor_integration_spec.ts b/packages/forms/test/value_accessor_integration_spec.ts index e4116804bd..f6cce04653 100644 --- a/packages/forms/test/value_accessor_integration_spec.ts +++ b/packages/forms/test/value_accessor_integration_spec.ts @@ -499,6 +499,18 @@ import {dispatchEvent} from '@angular/platform-browser/testing/src/browser_util' } }; + it('verify that native `selectedOptions` field is used while detecting the list of selected options', + fakeAsync(() => { + if (isNode || !HTMLSelectElement.prototype.hasOwnProperty('selectedOptions')) return; + const spy = spyOnProperty(HTMLSelectElement.prototype, 'selectedOptions', 'get') + .and.callThrough(); + setSelectedCities([]); + + selectOptionViaUI('1: Object'); + assertOptionElementSelectedState([false, true, false]); + expect(spy.calls.count()).toBe(2); + })); + it('should reflect state of model after option selected and new options subsequently added', fakeAsync(() => { if (isNode) return;