[webkit-reviews] review denied: [Bug 95830] Refactor CalendarPicker to not use global variables. : [Attachment 162215] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Keishi Hattori
<keishi at webkit.org>'s request for review:
Bug 95830: Refactor CalendarPicker to not use global variables.
https://bugs.webkit.org/show_bug.cgi?id=95830

Attachment 162215: Patch
https://bugs.webkit.org/attachment.cgi?id=162215&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=162215&action=review


> Source/WebCore/Resources/pagepopups/calendarPicker.js:78
> +    minimumDate: new Date(-62135596800000.0),
> +    maximumDate: new Date(8640000000000000.0)

Are these needed? CalendarPIcker constructor always sets values.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:298
> + * @param {?Element} element

I don't think element is nullable.  should be {!Element}.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:366
> +    var MainPadding = 6; // TODO: Fix name.

TODO: -> FIXME:

> Source/WebCore/Resources/pagepopups/calendarPicker.js:404
>   */
> -function YearMonthController() {
> +function YearMonthController(picker) {

Please add annotation for 'picker' argument.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:771
>   */
> -function DaysTable() {
> +function DaysTable(picker) {

ditto.