[webkit-reviews] review denied: [Bug 110970] Add calendar table view for the new calendar picker : [Attachment 190493] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Feb 27 06:43:04 PST 2013


Kent Tamura (ooo until Mar 15) <tkent at chromium.org> has denied Keishi Hattori
<keishi at webkit.org>'s request for review:
Bug 110970: Add calendar table view for the new calendar picker
https://bugs.webkit.org/show_bug.cgi?id=110970

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

------- Additional Comments from Kent Tamura (ooo until Mar 15)
<tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=190493&action=review


> Source/WebCore/Resources/pagepopups/calendarPicker.js:1901
> +DayCell.Height = 20;

Is it enough for touch input devices?  We use 28px in the pointer:coarse case
for now.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:2069
> +	   if (global.params.locale === "ja") {

global.params.locale should be getLanguage().
Or, this should check if the region part is "jp" because the rule of blue
Saturdays and red Sundays is a common sense in Japan, not Japanese language.
(But Google Chrome might provide no "jp" part.)

> Source/WebCore/Resources/pagepopups/calendarPicker.js:2204
> +    // Disable mouse wheel scrolling.

why?
We prefer "why" comments to "what" comments.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:2329
> + * @return {!Object}

should explain the content of Object.


More information about the webkit-reviews mailing list