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

fix(input): re-validate when partially editing date-family inputs #13886

Closed
wants to merge 1 commit into from
Closed

fix(input): re-validate when partially editing date-family inputs #13886

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions src/ng/directive/input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ var WEEK_REGEXP = /^(\d{4})-W(\d\d)$/;
var MONTH_REGEXP = /^(\d{4})-(\d\d)$/;
var TIME_REGEXP = /^(\d\d):(\d\d)(?::(\d\d)(\.\d{1,3})?)?$/;

var PARTIAL_VALIDATION_EVENTS = 'keydown wheel mousedown';
var PARTIAL_VALIDATION_TYPES = createMap();
forEach('date,datetime-local,month,time,week'.split(','), function(type) {
PARTIAL_VALIDATION_TYPES[type] = true;
});

var inputType = {

/**
Expand Down Expand Up @@ -1118,6 +1124,8 @@ function baseInputType(scope, element, attr, ctrl, $sniffer, $browser) {
});
}

var timeout;

var listener = function(ev) {
if (timeout) {
$browser.defer.cancel(timeout);
Expand Down Expand Up @@ -1147,8 +1155,6 @@ function baseInputType(scope, element, attr, ctrl, $sniffer, $browser) {
if ($sniffer.hasEvent('input')) {
element.on('input', listener);
} else {
var timeout;

var deferListener = function(ev, input, origValue) {
if (!timeout) {
timeout = $browser.defer(function() {
Expand Down Expand Up @@ -1180,6 +1186,26 @@ function baseInputType(scope, element, attr, ctrl, $sniffer, $browser) {
// or form autocomplete on newer browser, we need "change" event to catch it
element.on('change', listener);

// Some native input types (date-family) have the ability to change validity without
// firing any input/change events.
// For these event types, when native validators are present and the browser supports the type,
// check for validity changes on various DOM events.
if (PARTIAL_VALIDATION_TYPES[type] && ctrl.$$hasNativeValidators && type === attr.type) {
element.on(PARTIAL_VALIDATION_EVENTS, function(ev) {
if (!timeout) {
Copy link
Member

Choose a reason for hiding this comment

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

The fact that we are reusing the same timeout with the else brach above, is not good I think.
Since the callbacks are conditionally calling listener, there might be a timeout registered, so we don't register another one, but that other timeout's callback may end up not calling listener (while our callback would).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've had that concern as well for a while now with keydown vs cut/paste events in the IE case (no input event).

But in this case, with current browser date-like input support, I don't think there will be any issues. I don't think there is any browser that will not have the input event but will have native date-like inputs.

I do think this should change at some point though, even if just for the IE case...

Copy link
Member

Choose a reason for hiding this comment

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

True. Having separate timeouts would still be "cleaner", but even the shared timeout isn't indeed expected to create any issues.

var validity = this[VALIDITY_STATE_PROPERTY];
var origBadInput = validity.badInput;
var origTypeMismatch = validity.typeMismatch;
timeout = $browser.defer(function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the timeout for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When a keydown event fires the input has not yet changed. This might not be necessary for the other PARTIAL_VALIDATION_EVENTS though. The other option is using keyup but then changes won't be detected if the key is held for a while. Basically the same logic used for the no-input-event case (IE).

timeout = null;
if (validity.badInput !== origBadInput || validity.typeMismatch !== origTypeMismatch) {
listener(ev);
}
});
}
});
}

ctrl.$render = function() {
// Workaround for Firefox validation #12102.
var value = ctrl.$isEmpty(ctrl.$viewValue) ? '' : ctrl.$viewValue;
Expand Down
77 changes: 77 additions & 0 deletions test/ng/directive/inputSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1943,6 +1943,83 @@ describe('input', function() {
});
});

['month', 'week', 'time', 'date', 'datetime-local'].forEach(function(inputType) {
if (jqLite('<input type="' + inputType + '">').prop('type') !== inputType) {
return;
}

describe(inputType, function() {
they('should re-validate and dirty when partially editing the input value ($prop event)',
['keydown', 'wheel', 'mousedown'],
function(validationEvent) {
var mockValidity = {valid: true, badInput: false};
var inputElm = helper.compileInput('<input type="' + inputType + '" ng-model="val" name="alias" />', mockValidity);

expect(inputElm).toBeValid();
expect($rootScope.form.alias.$pristine).toBeTruthy();

inputElm.triggerHandler({type: validationEvent});
mockValidity.valid = false;
mockValidity.badInput = true;
$browser.defer.flush();
expect(inputElm).toBeInvalid();
expect($rootScope.form.alias.$pristine).toBeFalsy();
}
);

they('should do nothing when $prop event fired but validity does not change',
['keydown', 'wheel', 'mousedown'],
function(validationEvent) {
var mockValidity = {valid: true, badInput: false};
var inputElm = helper.compileInput('<input type="' + inputType + '" ng-model="val" name="alias" />', mockValidity);

expect(inputElm).toBeValid();
expect($rootScope.form.alias.$pristine).toBeTruthy();

inputElm.triggerHandler({type: validationEvent});
$browser.defer.flush();
expect(inputElm).toBeValid();
expect($rootScope.form.alias.$pristine).toBeTruthy();
}
);

they('should re-validate dirty when already $invalid and partially editing the input value ($prop event)',
['keydown', 'wheel', 'mousedown'],
function(validationEvent) {
var mockValidity = {valid: false, valueMissing: true, badInput: false};
var inputElm = helper.compileInput('<input type="' + inputType + '" required ng-model="val" name="alias" />', mockValidity);

expect(inputElm).toBeInvalid();
expect($rootScope.form.alias.$pristine).toBeTruthy();

inputElm.triggerHandler({type: validationEvent});
mockValidity.valid = false;
mockValidity.valueMissing = true;
mockValidity.badInput = true;
$browser.defer.flush();
expect(inputElm).toBeInvalid();
expect($rootScope.form.alias.$pristine).toBeFalsy();
}
);

they('should do nothing when already $invalid and $prop event fired but validity does not change',
['keydown', 'wheel', 'mousedown'],
function(validationEvent) {
var mockValidity = {valid: false, valueMissing: true, badInput: false};
var inputElm = helper.compileInput('<input type="' + inputType + '" required ng-model="val" name="alias" />', mockValidity);

expect(inputElm).toBeInvalid();
expect($rootScope.form.alias.$pristine).toBeTruthy();

inputElm.triggerHandler({type: validationEvent});
$browser.defer.flush();
expect(inputElm).toBeInvalid();
expect($rootScope.form.alias.$pristine).toBeTruthy();
}
);
});
});


describe('number', function() {

Expand Down