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

fix(input[date]): support years with more than 4 digits #13905

Closed
wants to merge 8 commits into from
Closed

fix(input[date]): support years with more than 4 digits #13905

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cbe6fbd
fix(input[date]): support years with more than 4 digits
LeeAdcock Jan 31, 2016
2ee3afc
test(input[date]): improve test descriptions, removed erroneous conso…
LeeAdcock Jan 31, 2016
8574cc0
fix(input[date]): resolving jshint finding
LeeAdcock Jan 31, 2016
a3408ed
test(input[date]): removing unnecessary space, per jscs
LeeAdcock Jan 31, 2016
4c1955c
fix(input[date]): iso date parsing, increase flexibility for timezone…
LeeAdcock Jan 31, 2016
6a0bf4f
perf(input[date]): iso date matching regex should be non-capturing
LeeAdcock Feb 1, 2016
335fc48
fix(input[date]): iso date parsing, Edge timezone offset requires col…
LeeAdcock Feb 2, 2016
a4acfb5
test(input[date]): removing test of non-functional regex implementation
LeeAdcock Feb 10, 2016
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
12 changes: 6 additions & 6 deletions src/ng/directive/input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
ngModelMinErr: false,
*/

// Regex code is obtained from SO: https://stackoverflow.com/questions/3143070/javascript-regex-iso-datetime#answer-3143231
var ISO_DATE_REGEXP = /\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d\.\d+([+-][0-2]\d:[0-5]\d|Z)/;
// Regex code was initially obtained from SO prior to modification: https://stackoverflow.com/questions/3143070/javascript-regex-iso-datetime#answer-3143231
var ISO_DATE_REGEXP = /^\d{4,}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d\.\d+(?:[+-][0-2]\d:[0-5]\d|Z)$/;
// See valid URLs in RFC3987 (http://tools.ietf.org/html/rfc3987)
// Note: We are being more lenient, because browsers are too.
// 1. Scheme
Expand All @@ -26,10 +26,10 @@ var ISO_DATE_REGEXP = /\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d\.\d+([+-][0-
var URL_REGEXP = /^[a-z][a-z\d.+-]*:\/*(?:[^:@]+(?::[^@]+)?@)?(?:[^\s:/?#]+|\[[a-f\d:]+\])(?::\d+)?(?:\/[^?#]*)?(?:\?[^#]*)?(?:#.*)?$/i;
var EMAIL_REGEXP = /^[a-z0-9!#$%&'*+\/=?^_`{|}~.-]+@[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)*$/i;
var NUMBER_REGEXP = /^\s*(\-|\+)?(\d+|(\d*(\.\d*)))([eE][+-]?\d+)?\s*$/;
var DATE_REGEXP = /^(\d{4})-(\d{2})-(\d{2})$/;
var DATETIMELOCAL_REGEXP = /^(\d{4})-(\d\d)-(\d\d)T(\d\d):(\d\d)(?::(\d\d)(\.\d{1,3})?)?$/;
var WEEK_REGEXP = /^(\d{4})-W(\d\d)$/;
var MONTH_REGEXP = /^(\d{4})-(\d\d)$/;
var DATE_REGEXP = /^(\d{4,})-(\d{2})-(\d{2})$/;
var DATETIMELOCAL_REGEXP = /^(\d{4,})-(\d\d)-(\d\d)T(\d\d):(\d\d)(?::(\d\d)(\.\d{1,3})?)?$/;
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 inputType = {
Expand Down
167 changes: 167 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 @@ -657,6 +657,18 @@ describe('input', function() {
expect($rootScope.form.alias.$error.month).toBeTruthy();
});

it('should allow four or more digits in year', function() {
var inputElm = helper.compileInput('<input type="month" ng-model="value" ng-model-options="{timezone: \'UTC\'}"/>');

helper.changeInputValueTo('10123-03');
expect(+$rootScope.value).toBe(Date.UTC(10123, 2, 1, 0, 0, 0));

$rootScope.$apply(function() {
$rootScope.value = new Date(Date.UTC(20456, 3, 1, 0, 0, 0));
});
expect(inputElm.val()).toBe('20456-04');
});


it('should only change the month of a bound date', function() {
var inputElm = helper.compileInput('<input type="month" ng-model="value" ng-model-options="{timezone: \'UTC\'}" />');
Expand Down Expand Up @@ -856,6 +868,17 @@ describe('input', function() {
expect(inputElm).toBeValid();
});

it('should allow four or more digits in year', function() {
var inputElm = helper.compileInput('<input type="week" ng-model="value" ng-model-options="{timezone: \'UTC\'}"/>');

helper.changeInputValueTo('10123-W03');
expect(+$rootScope.value).toBe(Date.UTC(10123, 0, 21));

$rootScope.$apply(function() {
$rootScope.value = new Date(Date.UTC(20456, 0, 28));
});
expect(inputElm.val()).toBe('20456-W04');
});

it('should use UTC if specified in the options', function() {
var inputElm = helper.compileInput('<input type="week" ng-model="value" ng-model-options="{timezone: \'UTC\'}" />');
Expand Down Expand Up @@ -1141,6 +1164,18 @@ describe('input', function() {
expect(+$rootScope.value).toBe(+new Date(2000, 0, 1, 1, 2, 0));
});

it('should allow four or more digits in year', function() {
var inputElm = helper.compileInput('<input type="datetime-local" ng-model="value" />');

helper.changeInputValueTo('10123-01-01T01:02');
expect(+$rootScope.value).toBe(+new Date(10123, 0, 1, 1, 2, 0));

$rootScope.$apply(function() {
$rootScope.value = new Date(20456, 1, 1, 1, 2, 0);
});
expect(inputElm.val()).toBe('20456-02-01T01:02:00.000');
}
);

it('should label parse errors as `datetimelocal`', function() {
var inputElm = helper.compileInput('<input type="datetime-local" ng-model="val" name="alias" />', {
Expand Down Expand Up @@ -1734,6 +1769,19 @@ describe('input', function() {
}
);

it('should allow four or more digits in year', function() {
var inputElm = helper.compileInput('<input type="date" ng-model="value" ng-model-options="{timezone: \'UTC\'}" />');

helper.changeInputValueTo('10123-01-01');
expect(+$rootScope.value).toBe(Date.UTC(10123, 0, 1, 0, 0, 0));

$rootScope.$apply(function() {
$rootScope.value = new Date(Date.UTC(20456, 1, 1, 0, 0, 0));
});
expect(inputElm.val()).toBe('20456-02-01');
}
);


it('should label parse errors as `date`', function() {
var inputElm = helper.compileInput('<input type="date" ng-model="val" name="alias" />', {
Expand Down Expand Up @@ -1941,6 +1989,125 @@ describe('input', function() {

expect(inputElm).toBeValid();
});

describe('ISO_DATE_REGEXP', function() {
var dates = [
// Validate date
['00:00:00.0000+01:01', false], // date must be specified
['2010.06.15T00:00:00.0000+01:01', false], // date must use dash seperator
['x2010-06-15T00:00:00.0000+01:01', false], // invalid leading characters

// Validate year
['2010-06-15T00:00:00.0000+01:01', true], // year has four or more digits
['20100-06-15T00:00:00.0000+01:01', true], // year has four or more digits
['-06-15T00:00:00.0000+01:01', false], // year has too few digits
['2-06-15T00:00:00.0000+01:01', false], // year has too few digits
['20-06-15T00:00:00.0000+01:01', false], // year has too few digits
['201-06-15T00:00:00.0000+01:01', false], // year has too few digits

// Validate month
['2010-01-15T00:00:00.0000+01:01', true], // month has two digits
['2010--15T00:00:00.0000+01:01', false], // month has too few digits
['2010-0-15T00:00:00.0000+01:01', false], // month has too few digits
['2010-1-15T00:00:00.0000+01:01', false], // month has too few digits
['2010-111-15T00:00:00.0000+01:01', false], // month has too many digits
['2010-22-15T00:00:00.0000+01:01', false], // month is too large

// Validate day
['2010-01-01T00:00:00.0000+01:01', true], // day has two digits
['2010-01-T00:00:00.0000+01:01', false], // day has too few digits
['2010-01-1T00:00:00.0000+01:01', false], // day has too few digits
['2010-01-200T00:00:00.0000+01:01', false], // day has too many digits
['2010-01-41T00:00:00.0000+01:01', false], // day is too large

// Validate time
['2010-01-01', false], // time must be specified
['2010-01-0101:00:00.0000+01:01', false], // missing date time seperator
['2010-01-01V01:00:00.0000+01:01', false], // invalid date time seperator
['2010-01-01T01-00-00.0000+01:01', false], // time must use colon seperator

// Validate hour
['2010-01-01T01:00:00.0000+01:01', true], // hour has two digits
['2010-01-01T-01:00:00.0000+01:01', false], // hour must be positive
['2010-01-01T:00:00.0000+01:01', false], // hour has too few digits
['2010-01-01T1:00:00.0000+01:01', false], // hour has too few digits
['2010-01-01T220:00:00.0000+01:01', false], // hour has too many digits
['2010-01-01T32:00:00.0000+01:01', false], // hour is too large

// Validate minutes
['2010-01-01T01:00:00.0000+01:01', true], // minute has two digits
['2010-01-01T01:-00:00.0000+01:01', false], // minute must be positive
['2010-01-01T01::00.0000+01:01', false], // minute has too few digits
['2010-01-01T01:0:00.0000+01:01', false], // minute has too few digits
['2010-01-01T01:100:00.0000+01:01', false], // minute has too many digits
['2010-01-01T01:60:00.0000+01:01', false], // minute is too large

// Validate seconds
['2010-01-01T01:00:00.0000+01:01', true], // second has two digits
['2010-01-01T01:00:-00.0000+01:01', false], // second must be positive
['2010-01-01T01:00:.0000+01:01', false], // second has too few digits
['2010-01-01T01:00:0.0000+01:01', false], // second has too few digits
['2010-01-01T01:00:100.0000+01:01', false], // second has too many digits
['2010-01-01T01:00:60.0000+01:01', false], // second is too large

// Validate milliseconds
['2010-01-01T01:00:00+01:01', false], // millisecond must be specified
['2010-01-01T01:00:00.-0000+01:01', false], // millisecond must be positive
['2010-01-01T01:00:00:0000+01:01', false], // millisecond must use period seperator
['2010-01-01T01:00:00.+01:01', false], // millisecond has too few digits

// Validate timezone
['2010-06-15T00:00:00.0000', false], // timezone must be specified

// Validate timezone offset
['2010-06-15T00:00:00.0000+01:01', true], // timezone offset can be positive hours and minutes
['2010-06-15T00:00:00.0000-01:01', true], // timezone offset can be negative hours and minutes
['2010-06-15T00:00:00.0000~01:01', false], // timezone has postive/negative indicator
['2010-06-15T00:00:00.000001:01', false], // timezone has postive/negative indicator
['2010-06-15T00:00:00.0000+00:01Z', false], // timezone invalid trailing characters
['2010-06-15T00:00:00.0000+00:01 ', false], // timezone invalid trailing characters
Copy link
Member

Choose a reason for hiding this comment

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

Actually, according to the standard, the timezone offset can have one of three forms:

+/-HH:mm, +/-HHmm, +/-HH

(Feel free to add suppory for those as well 😃)


// Validate timezone hour offset
['2010-06-15T00:00:00.0000+:01', false], // timezone hour offset has too few digits
['2010-06-15T00:00:00.0000+0:01', false], // timezone hour offset has too few digits
['2010-06-15T00:00:00.0000+211:01', false], // timezone hour offset too many digits
['2010-06-15T00:00:00.0000+31:01', false], // timezone hour offset value too large

// Validate timezone minute offset
['2010-06-15T00:00:00.0000+00:-01', false], // timezone minute offset must be positive
['2010-06-15T00:00:00.0000+00.01', false], // timezone minute offset must use colon seperator
['2010-06-15T00:00:00.0000+0101', false], // timezone minute offset must use colon seperator
['2010-06-15T00:00:00.0000+010', false], // timezone minute offset must use colon seperator
['2010-06-15T00:00:00.0000+00', false], // timezone minute offset has too few digits
['2010-06-15T00:00:00.0000+00:', false], // timezone minute offset has too few digits
['2010-06-15T00:00:00.0000+00:0', false], // timezone minute offset has too few digits
['2010-06-15T00:00:00.0000+00:211', false], // timezone minute offset has too many digits
['2010-06-15T00:00:00.0000+01010', false], // timezone minute offset has too many digits
['2010-06-15T00:00:00.0000+00:61', false], // timezone minute offset is too large

// Validate timezone UTC
['2010-06-15T00:00:00.0000Z', true], // UTC timezone can be indicated with Z
['2010-06-15T00:00:00.0000K', false], // UTC timezone indicator is invalid
['2010-06-15T00:00:00.0000 Z', false], // UTC timezone indicator has extra space
['2010-06-15T00:00:00.0000ZZ', false], // UTC timezone indicator invalid trailing characters
['2010-06-15T00:00:00.0000Z ', false] // UTC timezone indicator invalid trailing characters
];

they('should validate date: $prop', dates, function(item) {
var date = item[0];
var valid = item[1];

/* global ISO_DATE_REGEXP: false */
expect(ISO_DATE_REGEXP.test(date)).toBe(valid);
});

it('should be non-capturing', function() {
Copy link
Member

Choose a reason for hiding this comment

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

This test is not needed. Capturing vs Non-capturing is just an implementation detail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to make this change, but would point out that if having the regex be non-capturing is important for performance, we should have a test that validates it. The regex was originally capturing, but that mistake wasn't noticed until this unit test was written. It's not unlikely that the next person that modifies it after us will unintentionally make it capturing again, unless there is a test enforces this style of implementation for performance gains.

Copy link
Member

Choose a reason for hiding this comment

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

This is still an implementation detail. E.g. if we decide to add support for all timezone formats, this will change: we'll need some capturing groups. Unless it is identified as a performance issue (which it won't - it's more of a theoretical performance gain, unlikely to be noticable), it remains an implementation detail.

So, while it's nice to avoid capturing groups if you don't need them, having a test for it will only make it more difficult to add functionality later. Unit tests should assert the functionality/behavior, not the performance characteristics, imo.

// Non-capturing to increase performance, has no functional impact
var result = ISO_DATE_REGEXP.exec('2010-06-15T00:00:00.0000+01:01');
expect(result[0]).toBe('2010-06-15T00:00:00.0000+01:01');
expect(result.length).toBe(1);
});
});
});


Expand Down