fix(input): update $viewValue when cleared#14772
Conversation
Cool! I believe writing a test is difficult because we can't click the clear button? |
wesleychoforce-pushed the fix/ie-input branch 2 times, most recently from 2445f95 to 1da5563Compare
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? |
wesleychoforce-pushed the fix/ie-input branch 10 times, most recently from 6273af0 to d997158Compare
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 :( . |
blemaire commented Jun 8, 2018
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.