This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(input): update $viewValue when cleared #14772

Open

wesleycho wants to merge 1 commit into angular:master

from wesleycho:fix/ie-input

Open

fix(input): update $viewValue when cleared #14772

wesleycho wants to merge 1 commit into angular:master from wesleycho:fix/ie-input

Conversation

@wesleycho

Copy link

Contributor

@wesleycho wesleycho commented Jun 14, 2016

Fix when user clicks clear button in an input element in IE, $viewValue not being correctly updated

This should fix #11193.

@wesleycho wesleycho force-pushed the fix/ie-input branch from 0bf2e0a to e8d890f Compare

June 14, 2016 15:30

@Narretz

Copy link

Contributor

Narretz commented Jun 14, 2016

Cool! I believe writing a test is difficult because we can't click the clear button?

@wesleycho wesleycho force-pushed the fix/ie-input branch 2 times, most recently from 2445f95 to 1da5563 Compare

June 14, 2016 15:51

@wesleycho

Copy link

Contributor Author

wesleycho commented Jun 14, 2016

Yeah - fussing around with this a bit to get the right condition currently, but I think the right fix might be attainable here.

@wesleycho wesleycho force-pushed the fix/ie-input branch from 1da5563 to 316ba23 Compare

June 14, 2016 15:58

@wesleycho

Copy link

Contributor Author

wesleycho commented Jun 14, 2016

This is a little tough - so falsy model changes will convert to the empty string, so this potentially breaks some behavior. Any ideas on good workarounds for that situation?

@wesleycho wesleycho force-pushed the fix/ie-input branch 10 times, most recently from 6273af0 to d997158 Compare

June 14, 2016 17:14

[画像:jbedard]

jbedard reviewed

Jun 14, 2016

View reviewed changes

src/ng/directive/input.js Outdated

var timeout;

var timeout, oldVal;

var viewValueUpdated = true, msieInput = msie >= 10 && msie <= 11;

if (msieInput && inputType === 'text') {

Copy link

Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only text inputs? Won't types such as number have the same issue?

Copy link

Contributor Author

@wesleycho wesleycho Jun 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you're correct - I assumed it would have the spinner, but apparently not.

@wesleycho wesleycho force-pushed the fix/ie-input branch from d997158 to 2ee9eed Compare

June 14, 2016 17:32

[画像:jbedard]

jbedard reviewed

Jun 14, 2016

View reviewed changes

src/ng/directive/input.js

function ieListener(ev) {

var val = element.val();

if (val === oldVal && !viewValueUpdated) return;

Copy link

Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that the validity changed but the value did not (similar to the value === '' && ctrl.$$hasNativeValidators check in listener)? Although I'm not seeing how that would have been handled previously either (in the !hasEvent('input') case)...

@wesleycho wesleycho force-pushed the fix/ie-input branch from 2ee9eed to ba63651 Compare

June 14, 2016 17:35

[画像:jbedard]

jbedard reviewed

Jun 14, 2016

View reviewed changes

src/ng/directive/input.js Outdated

var timeout;

var timeout, oldVal;

var viewValueUpdated = false, msieInput = msie >= 10 && msie <= 11;

if (msieInput && attr.type in IE_INPUTS_WITH_CLEARING) {

Copy link

Contributor

@jbedard jbedard Jun 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we even care about the type for setting the initial oldVal though? I'd think we'd always want to setup the oldVal if msieInput is true...

Copy link

Contributor Author

@wesleycho wesleycho Jun 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't that trigger a potentially unnecessary DOM access though? That is likely more expensive than this check I think. I originally had it just accessing the value, I'd be fine changing it back.

Copy link

Contributor

@jbedard jbedard Jun 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we keep it I think it should be part of the msieInput condition, not this setup condition.

But I'd think element.val() is cheap enough it doesn't matter.

@wesleycho

Copy link

Contributor Author

wesleycho commented Jun 14, 2016

There are some complicated scenarios I'm having trouble figuring out a good solution to.

Here is an incomplete table:

Initial view value	Initial model value	View value after view change	Model value after model change	Erroneous IE input events	Run $parsers/$validators
''	undefined	''	N/A	false	true
''	null	''	N/A	false	true
''	null	N/A	''	false	true
''	undefined	N/A	N/A	true	false

@wesleycho


 fix(input): update $viewValue when cleared

2feeed6

- Fix when user clicks clear button in an input element in IE, $viewValue not being correctly updated

@wesleycho wesleycho force-pushed the fix/ie-input branch from ba63651 to 2feeed6 Compare

June 14, 2016 17:53

@gkalpak

Copy link

Member

gkalpak commented Jun 15, 2016

Wouldn't it be easier to just add an extra listener on IEs:

element.on('??mousedown/mouseup/click??', function(event) {
 deferListener(event, this, this.value);
});

@gkalpak gkalpak added type: bug component: forms frequency: moderate browser: IE10 labels

Jun 15, 2016

@gkalpak gkalpak added browser: IE11 severity: broken expected use labels

Jun 15, 2016

@gkalpak gkalpak added this to the Backlog milestone

Jun 15, 2016

@wesleycho wesleycho mentioned this pull request

Jun 16, 2016

ng-change event is not triggered for IE11 when the element has no keydown event fired #14783

Open

@Narretz

Copy link

Contributor

Narretz commented Oct 7, 2016

@wesleycho Do you still want to work on this?

@wesleycho

Copy link

Contributor Author

wesleycho commented Oct 7, 2016

Don't have the bandwidth anymore :( .

@googlebot googlebot added the cla: yes label

Oct 7, 2016

@blemaire

Copy link

blemaire commented Jun 8, 2018

What's the status on this issue (not doubt i'm not the only one encountering this)
Thanks

@gkalpak

Copy link

Member

gkalpak commented Jun 8, 2018

It has been abandoned (afaict) 😁
If anyone wants to take it over, we will be more than happy to offer guidance and reviews, but I don't think we have the resources to take it over, given that we have less than a month of active feature development before entering LTS.

@gkalpak gkalpak mentioned this pull request

Jun 8, 2018

AngularJS binding does not update when IE's clear button is clicked #11193

Open

@Splaktar Splaktar mentioned this pull request

Aug 30, 2018

input: length property and model not updated after using IE "clear field" button angular/material#10872

Closed

fix(input): update $viewValue when cleared #14772

Are you sure you want to change the base?

fix(input): update $viewValue when cleared #14772

Uh oh!

Conversation

@wesleycho wesleycho commented Jun 14, 2016

Uh oh!

Narretz commented Jun 14, 2016

Uh oh!

wesleycho commented Jun 14, 2016

Uh oh!

wesleycho commented Jun 14, 2016

Uh oh!

@jbedard jbedard Jun 14, 2016

Choose a reason for hiding this comment

Uh oh!

@wesleycho wesleycho Jun 14, 2016

Choose a reason for hiding this comment

Uh oh!

@jbedard jbedard Jun 14, 2016

Choose a reason for hiding this comment

Uh oh!

@jbedard jbedard Jun 14, 2016

Choose a reason for hiding this comment

Uh oh!

@wesleycho wesleycho Jun 14, 2016

Choose a reason for hiding this comment

Uh oh!

@jbedard jbedard Jun 14, 2016

Choose a reason for hiding this comment

Uh oh!

wesleycho commented Jun 14, 2016

Uh oh!

gkalpak commented Jun 15, 2016

Uh oh!

Narretz commented Oct 7, 2016

Uh oh!

wesleycho commented Oct 7, 2016

Uh oh!

blemaire commented Jun 8, 2018

Uh oh!

gkalpak commented Jun 8, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants