1381421 - [DateTimePicker] Handle dates earlier than 0001-01-01 and later than 275760-09-13 correctly

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, enhancement, P1)

Description

[Scott Wu [:scottwu]]

The HTML Spec[1] defines the min date to be 0001-01-01, and the largest date defined in ECMAScript[2] is 275760-09-13. Dates outside of this range should not only be unselectable, but also display properly. The spinners and arrow buttons on the calendar should also prevent user from moving beyond the boundaries.

[1] https://html.spec.whatwg.org/#valid-date-string
[2] http://ecma-international.org/ecma-262/5.1/#sec-15.9.1.1

Comment 1

[Scott Wu [:scottwu]]

Attachment: Bug 1381421 - (Part 1) Handle dates earlier than 0001-01-01 and later than 275760-09-13 correctly.

Review commit: https://reviewboard.mozilla.org/r/159746/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/159746/

Comment 2

[Scott Wu [:scottwu]]

Attachment: Bug 1381421 - (Part 2) Add browser chrome tests for the minimum and maximum dates.

Review commit: https://reviewboard.mozilla.org/r/166474/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/166474/

Comment 3

[Scott Wu [:scottwu]]

Comment on attachment 8888713 [details]
Bug 1381421 - (Part 1) Handle dates earlier than 0001-01-01 and later than 275760-09-13 correctly.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/159746/diff/1-2/

Comment 4

[Scott Wu [:scottwu]]

Comment on attachment 8888713 [details]
Bug 1381421 - (Part 1) Handle dates earlier than 0001-01-01 and later than 275760-09-13 correctly.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/159746/diff/2-3/

Comment 5

[Scott Wu [:scottwu]]

Comment on attachment 8895285 [details]
Bug 1381421 - (Part 2) Add browser chrome tests for the minimum and maximum dates.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/166474/diff/1-2/

Comment 6

[Scott Wu [:scottwu]]

This patch addresses the obvious issues when displaying 0001-01-01 and 275760-09-13. Anything smaller or larger are not displayed and unselectable. There are other things that are also changed in this patch:
- In calendar.js, renamed set() to setCalendar() for clarity, and the method takes only `year` and `month` because `day` isn't relevant for displaying the calendar month.
- For the Calendar object, functions are passed in on creation rather than as props, because they are not expected to change.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8895285 [details]
Bug 1381421 - (Part 2) Add browser chrome tests for the minimum and maximum dates.

https://reviewboard.mozilla.org/r/166474/#review175068

::: toolkit/content/tests/browser/browser_datetime_datepicker.js:253
(Diff revision 2)
> +  Assert.deepEqual(
> +    getCalendarText(),
> +    [
> +      "31", "1", "2", "3",  "4",  "5",  "6",
> +      "7",  "8", "9", "10", "11", "12", "13",
> +      "",   "",  "",  "",   "",   "",   "",

lol, I guess I shouldn't schedule anything here then. :)

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8888713 [details]
Bug 1381421 - (Part 1) Handle dates earlier than 0001-01-01 and later than 275760-09-13 correctly.

https://reviewboard.mozilla.org/r/159746/#review175324

Hey scottwu, this looks great! Thanks so much! Just two notes below, but nothing major.

::: toolkit/content/widgets/calendar.js:35
(Diff revision 3)
> -    weekHeaders: []
> +    weekHeaders: [],
> +    setSelection: options.setSelection,
> +    getDayString: options.getDayString,
> +    getWeekHeaderString: options.getWeekHeaderString
>    };
>    this.props = {};

I guess this is unused now?

::: toolkit/content/widgets/datekeeper.js:81
(Diff revision 3)
> -                           new Date(this.state.today);
> +        month: month === undefined ? today.getMonth() : month
> +      });
>      },
> +
>      /**
> -     * Set new date. The year is always treated as full year, so the short-form
> +     * Set new calendar date. The year is always treated as full year, so the

Hm, I don't think "calendar date" is the right term here.

See [this article on Calendar Date](https://en.wikipedia.org/wiki/Calendar_date) - specifically:

"A calendar date is a reference to a particular day represented within a calendar system."

So I think "calendar date" is kinda confusing, since we're choosing only the month and year, and not the day.

I don't actually know what the term is for just the month and year. Perhaps we should call this function:

`setCalendarMonth`

and change this comment to be "Set new calendar month."

Comment 9

[Scott Wu [:scottwu]]

Comment on attachment 8888713 [details]
Bug 1381421 - (Part 1) Handle dates earlier than 0001-01-01 and later than 275760-09-13 correctly.

https://reviewboard.mozilla.org/r/159746/#review175324

> I guess this is unused now?

Yes, thanks for spotting it!

> Hm, I don't think "calendar date" is the right term here.
> 
> See [this article on Calendar Date](https://en.wikipedia.org/wiki/Calendar_date) - specifically:
> 
> "A calendar date is a reference to a particular day represented within a calendar system."
> 
> So I think "calendar date" is kinda confusing, since we're choosing only the month and year, and not the day.
> 
> I don't actually know what the term is for just the month and year. Perhaps we should call this function:
> 
> `setCalendarMonth`
> 
> and change this comment to be "Set new calendar month."

I agree. `setCalendarMonth` sounds good. I shall rename it that.

Comment 10

[Scott Wu [:scottwu]]

Comment on attachment 8895285 [details]
Bug 1381421 - (Part 2) Add browser chrome tests for the minimum and maximum dates.

https://reviewboard.mozilla.org/r/166474/#review175068

> lol, I guess I shouldn't schedule anything here then. :)

Yeah might disappoint some sci-fi fans but ¯\_(ツ)_/¯

Comment 11

[Scott Wu [:scottwu]]

Comment on attachment 8888713 [details]
Bug 1381421 - (Part 1) Handle dates earlier than 0001-01-01 and later than 275760-09-13 correctly.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/159746/diff/3-4/

Comment 12

[Scott Wu [:scottwu]]

Comment on attachment 8895285 [details]
Bug 1381421 - (Part 2) Add browser chrome tests for the minimum and maximum dates.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/166474/diff/2-3/

Comment 13

[Scott Wu [:scottwu]]

Thanks again Mike! Checking it in.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4ddc77083d20
(Part 1) Handle dates earlier than 0001-01-01 and later than 275760-09-13 correctly. r=mconley
https://hg.mozilla.org/integration/autoland/rev/48bb3e020058
(Part 2) Add browser chrome tests for the minimum and maximum dates. r=mconley

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4ddc77083d20
https://hg.mozilla.org/mozilla-central/rev/48bb3e020058