Skip to content

Commit

Permalink
fix(material/core): avoid browser inconsistencies when parsing time
Browse files Browse the repository at this point in the history
Previously we were trying to rely on the browser to parse time, but we couldn't rely on it fully because browsers are inconsistent in how the handle time strings. We had some fallback logic to try and patch over it, but it led to some bug.

These changes switch to relying fully on our own logic for parsing times.
  • Loading branch information
crisbeto committed Oct 4, 2024
1 parent 490bcfe commit 0fb4247
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,7 @@ describe('DateFnsAdapter', () => {
expect(adapter.isValid(adapter.parseTime('24:05', 'p')!)).toBe(false);
expect(adapter.isValid(adapter.parseTime('00:61:05', 'p')!)).toBe(false);
expect(adapter.isValid(adapter.parseTime('14:52:78', 'p')!)).toBe(false);
expect(adapter.isValid(adapter.parseTime('12:10 PM11:10 PM', 'p')!)).toBe(false);
});

it('should compare times', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,7 @@ describe('LuxonDateAdapter', () => {
expect(adapter.isValid(adapter.parseTime('24:05', 't')!)).toBeFalse();
expect(adapter.isValid(adapter.parseTime('00:61:05', 'tt')!)).toBeFalse();
expect(adapter.isValid(adapter.parseTime('14:52:78', 'tt')!)).toBeFalse();
expect(adapter.isValid(adapter.parseTime('12:10 PM11:10 PM', 'tt')!)).toBeFalse();
});

it('should return null when parsing unsupported time values', () => {
Expand Down
13 changes: 3 additions & 10 deletions src/material/core/datetime/native-date-adapter.spec.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
import {LOCALE_ID} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {Platform} from '@angular/cdk/platform';
import {DEC, FEB, JAN, MAR} from '../../testing';
import {DateAdapter, MAT_DATE_LOCALE, NativeDateAdapter, NativeDateModule} from './index';

describe('NativeDateAdapter', () => {
let adapter: NativeDateAdapter;
let assertValidDate: (d: Date | null, valid: boolean) => void;
let platform: Platform;

beforeEach(() => {
TestBed.configureTestingModule({imports: [NativeDateModule]});
adapter = TestBed.inject(DateAdapter) as NativeDateAdapter;
platform = TestBed.inject(Platform);

assertValidDate = (d: Date | null, valid: boolean) => {
expect(adapter.isDateInstance(d))
Expand Down Expand Up @@ -587,13 +584,9 @@ describe('NativeDateAdapter', () => {
expect(adapter.isValid(adapter.parseTime('123')!)).toBe(false);
expect(adapter.isValid(adapter.parseTime('14:52 PM')!)).toBe(false);
expect(adapter.isValid(adapter.parseTime('24:05')!)).toBe(false);

// Firefox is a bit more forgiving of invalid times than other browsers.
// E.g. these just roll over instead of producing an invalid object.
if (!platform.FIREFOX) {
expect(adapter.isValid(adapter.parseTime('00:61:05')!)).toBe(false);
expect(adapter.isValid(adapter.parseTime('14:52:78')!)).toBe(false);
}
expect(adapter.isValid(adapter.parseTime('00:61:05')!)).toBe(false);
expect(adapter.isValid(adapter.parseTime('14:52:78')!)).toBe(false);
expect(adapter.isValid(adapter.parseTime('12:10 PM11:10 PM')!)).toBe(false);
});

it('should return null when parsing unsupported time values', () => {
Expand Down
99 changes: 45 additions & 54 deletions src/material/core/datetime/native-date-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const ISO_8601_REGEX =
* - {{hours}}.{{minutes}} AM/PM
* - {{hours}}.{{minutes}}.{{seconds}} AM/PM
*/
const TIME_REGEX = /(\d?\d)[:.](\d?\d)(?:[:.](\d?\d))?\s*(AM|PM)?/i;
const TIME_REGEX = /^(\d?\d)[:.](\d?\d)(?:[:.](\d?\d))?\s*(AM|PM)?$/i;

/** Creates an array and fills it with values. */
function range<T>(length: number, valueFunction: (index: number) => T): T[] {
Expand Down Expand Up @@ -292,67 +292,20 @@ export class NativeDateAdapter extends DateAdapter<Date> {
return null;
}

const today = this.today();
const base = this.toIso8601(today);
// Attempt to parse the value directly.
let result = this._parseTimeString(value);

// JS is able to parse colon-separated times (including AM/PM) by
// appending it to a valid date string. Generate one from today's date.
let result = Date.parse(`${base} ${value}`);

// Some locales use a dot instead of a colon as a separator, try replacing it before parsing.
if (!result && value.includes('.')) {
result = Date.parse(`${base} ${value.replace(/\./g, ':')}`);
}

// Other locales add extra characters around the time, but are otherwise parseable
// Some locales add extra characters around the time, but are otherwise parseable
// (e.g. `00:05 ч.` in bg-BG). Try replacing all non-number and non-colon characters.
if (!result) {
if (result === null) {
const withoutExtras = value.replace(/[^0-9:(AM|PM)]/gi, '').trim();

if (withoutExtras.length > 0) {
result = Date.parse(`${base} ${withoutExtras}`);
}
}

// Some browser implementations of Date aren't very flexible with the time formats.
// E.g. Safari doesn't support AM/PM or padded numbers. As a final resort, we try
// parsing some of the more common time formats ourselves.
if (!result) {
const parsed = value.toUpperCase().match(TIME_REGEX);

if (parsed) {
let hours = parseInt(parsed[1]);
const minutes = parseInt(parsed[2]);
let seconds: number | undefined = parsed[3] == null ? undefined : parseInt(parsed[3]);
const amPm = parsed[4] as 'AM' | 'PM' | undefined;

if (hours === 12) {
hours = amPm === 'AM' ? 0 : hours;
} else if (amPm === 'PM') {
hours += 12;
}

if (
inRange(hours, 0, 23) &&
inRange(minutes, 0, 59) &&
(seconds == null || inRange(seconds, 0, 59))
) {
return this.setTime(today, hours, minutes, seconds || 0);
}
}
}

if (result) {
const date = new Date(result);

// Firefox allows overflows in the time string, e.g. 25:00 gets parsed as the next day.
// Other browsers return invalid date objects in such cases so try to normalize it.
if (this.sameDate(today, date)) {
return date;
result = this._parseTimeString(withoutExtras);
}
}

return this.invalid();
return result || this.invalid();
}

override addSeconds(date: Date, amount: number): Date {
Expand Down Expand Up @@ -397,6 +350,44 @@ export class NativeDateAdapter extends DateAdapter<Date> {
d.setUTCHours(date.getHours(), date.getMinutes(), date.getSeconds(), date.getMilliseconds());
return dtf.format(d);
}

/**
* Attempts to parse a time string into a date object. Returns null if it cannot be parsed.
* @param value Time string to parse.
*/
private _parseTimeString(value: string): Date | null {
// Note: we can technically rely on the browser for the time parsing by generating
// an ISO string and appending the string to the end of it. We don't do it, because
// browsers aren't consistent in what they support. Some examples:
// - Safari doesn't support AM/PM.
// - Firefox produces a valid date object if the time string has overflows (e.g. 12:75) while
// other browsers produce an invalid date.
// - Safari doesn't allow padded numbers.
const parsed = value.toUpperCase().match(TIME_REGEX);

if (parsed) {
let hours = parseInt(parsed[1]);
const minutes = parseInt(parsed[2]);
let seconds: number | undefined = parsed[3] == null ? undefined : parseInt(parsed[3]);
const amPm = parsed[4] as 'AM' | 'PM' | undefined;

if (hours === 12) {
hours = amPm === 'AM' ? 0 : hours;
} else if (amPm === 'PM') {
hours += 12;
}

if (
inRange(hours, 0, 23) &&
inRange(minutes, 0, 59) &&
(seconds == null || inRange(seconds, 0, 59))
) {
return this.setTime(this.today(), hours, minutes, seconds || 0);
}
}

return null;
}
}

/** Checks whether a number is within a certain range. */
Expand Down

0 comments on commit 0fb4247

Please sign in to comment.