fix(input): update $viewValue when cleared #14772
Conversation
|
Cool! I believe writing a test is difficult because we can't click the clear button? |
wesleycho
force-pushed
the
fix/ie-input
branch
2 times, most recently
from
2445f95
to
1da5563
Compare
|
Yeah - fussing around with this a bit to get the right condition currently, but I think the right fix might be attainable here. |
|
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
force-pushed
the
fix/ie-input
branch
10 times, most recently
from
6273af0
to
d997158
Compare
src/ng/directive/input.js
Outdated
| var timeout; | ||
| var timeout, oldVal; | ||
| var viewValueUpdated = true, msieInput = msie >= 10 && msie <= 11; | ||
| if (msieInput && inputType === 'text') { |
There was a problem hiding this comment.
Why only text inputs? Won't types such as number have the same issue?
There was a problem hiding this comment.
Looks like you're correct - I assumed it would have the spinner, but apparently not.
| @@ -1152,10 +1158,18 @@ function baseInputType(scope, element, attr, ctrl, $sniffer, $browser) { | |||
| } | |||
| }; | |||
|
|
|||
| function ieListener(ev) { | |||
| var val = element.val(); | |||
| if (val === oldVal && !viewValueUpdated) return; | |||
There was a problem hiding this comment.
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)...
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) { |
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
There are some complicated scenarios I'm having trouble figuring out a good solution to. Here is an incomplete table:
|
- Fix when user clicks clear button in an input element in IE, $viewValue not being correctly updated
|
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);
}); |
|
@wesleycho Do you still want to work on this? |
|
Don't have the bandwidth anymore :( . |
|
What's the status on this issue (not doubt i'm not the only one encountering this) |
|
It has been abandoned (afaict) 😁 |
This should fix #11193.