Bug 109439

Summary:

Update calendar picker UI

Product:

WebKit

Reporter:

Keishi Hattori <keishi>

Component:

Forms

Assignee:

Keishi Hattori <keishi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dglazkov, pfeldman, syoichi, tkent, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

109169, 109530, 109897, 110131, 110132, 110137, 110140, 110454, 110589, 110830, 110967, 110970, 111319

Bug Blocks:

111811, 111812

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	webkit.review.bot: commit-queue-
replace CalendarPicker	none
Patch	none
Patch	none
Patch	none
Patch	none

Keishi Hattori

Reported 2013-02-11 08:03:21 PST

We want to update calendar picker UI to match the Chrome UI.

Attachments
Patch (760.46 KB, patch) 2013-02-14 21:05 PST, Keishi Hattori	no flags	Details Formatted Diff Diff
Patch (819.05 KB, patch) 2013-02-18 00:57 PST, Keishi Hattori	no flags	Details Formatted Diff Diff
Patch (819.58 KB, patch) 2013-02-18 02:58 PST, Keishi Hattori	webkit.review.bot: commit-queue-	Details Formatted Diff Diff
replace CalendarPicker (242.20 KB, patch) 2013-02-28 13:06 PST, Keishi Hattori	no flags	Details Formatted Diff Diff
Patch (747.02 KB, patch) 2013-03-07 04:32 PST, Keishi Hattori	no flags	Details Formatted Diff Diff
Patch (737.75 KB, patch) 2013-03-07 22:36 PST, Keishi Hattori	no flags	Details Formatted Diff Diff
Patch (755.75 KB, patch) 2013-03-07 23:11 PST, Keishi Hattori	no flags	Details Formatted Diff Diff
Patch (755.85 KB, patch) 2013-03-07 23:13 PST, Keishi Hattori	no flags	Details Formatted Diff Diff
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.

Keishi Hattori

Comment 1 2013-02-11 08:05:04 PST

Mock: https://googledrive.com/host/0B4aiM9jljUy8T19WQkJVMTFTd1E/

James Robinson

Comment 2 2013-02-11 09:35:15 PST

Nothing in this mock requires compositing that I can see. Why do you feel you need it?

Keishi Hattori

Comment 3 2013-02-14 21:05:22 PST

Created attachment 188474 [details] Patch

Kent Tamura

Comment 4 2013-02-14 21:24:45 PST

Comment on attachment 188474 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=188474&action=review > Source/WebCore/ChangeLog:8 > + Reviewed by NOBODY (OOPS!). > + > + No new tests. Covered by calendar picker appearance tests. Please write what are user-visible changes briefly > Source/WebCore/Resources/pagepopups/calendarPicker.css:-2 > -/* > - * Copyright (C) 2012 Google Inc. All rights reserved. Don't remove the copyright header > Source/WebCore/Resources/pagepopups/calendarPicker.js:-3 > -/* > - * Copyright (C) 2012 Google Inc. All rights reserved. Ditto. > Source/WebCore/platform/text/mac/LocaleMac.mm:212 > + // Gets a format for "MMMM" because Windows API always provides formats for > + // "MMMM" in some locales. bogus comment > LayoutTests/platform/chromium/TestExpectations:4123 > +webkit.org/b/109439 platform/chromium/fast/forms/calendar-picker/calendar-picker-mouse-operations.html [ Failure Pass ] > +webkit.org/b/109439 platform/chromium/fast/forms/calendar-picker/calendar-picker-mouse-operations.html [ Failure Pass ] > +webkit.org/b/109439 platform/chromium/fast/forms/calendar-picker/calendar-picker-mouse-operations.html [ Failure Pass ] > +webkit.org/b/109439 platform/chromium/fast/forms/calendar-picker/calendar-picker-mouse-operations.html [ Failure Pass ] > +webkit.org/b/109439 platform/chromium/fast/forms/calendar-picker/calendar-picker-mouse-operations.html [ Failure Pass ] What do this duplicated lines mean? > LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-with-step-expected.txt:9 > -PASS availableDatesInCurrentMonth().join(",") is ["2011-05-08", "2011-05-17", "2011-05-26"].join(",") > +FAIL availableDatesInCurrentMonth().join(",") should be 2011-05-08,2011-05-17,2011-05-26. Was . > Step when min is set. > -PASS availableDatesInCurrentMonth().join(",") is ["2011-05-01", "2011-05-10", "2011-05-19", "2011-05-28"].join(",") > +FAIL availableDatesInCurrentMonth().join(",") should be 2011-05-01,2011-05-10,2011-05-19,2011-05-28. Was . FAIL > LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-mouse-operations-expected.txt:15 > -PASS selectedMonth() is "2000-02" > +FAIL highlightedValue() should be 2000-02. Was 2000-01. FAIL > LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-with-step-expected.txt:11 > -PASS availableDatesInCurrentMonth()[0] is "2011-04-01" > -PASS availableDatesInCurrentMonth()[29] is "2011-04-30" > +FAIL availableDatesInCurrentMonth()[0] should be 2011-04-01 (of type string). Was undefined (of type undefined). > +FAIL availableDatesInCurrentMonth()[29] should be 2011-04-30 (of type string). Was undefined (of type undefined). > Step when min is set. > -PASS availableDatesInCurrentMonth()[0] is "2012-02-01" > -PASS availableDatesInCurrentMonth()[28] is "2012-02-29" > +FAIL availableDatesInCurrentMonth()[0] should be 2012-02-01 (of type string). Was undefined (of type undefined). > +FAIL availableDatesInCurrentMonth()[28] should be 2012-02-29 (of type string). Was undefined (of type undefined). FAIL

Kent Tamura

Comment 5 2013-02-14 21:27:40 PST

The patch is too large. Please split it as possible. e.g. - PopupController interface change - Locale::shortMonthFormat addition - ...

Keishi Hattori

Comment 6 2013-02-18 00:57:50 PST

Created attachment 188811 [details] Patch

WebKit Review Bot

Comment 7 2013-02-18 01:18:05 PST

Attachment 188811 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-ar-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-required-ar-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-required-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-ru-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-step-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/month-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/month-picker-appearance-step-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-step-expected.png', u'LayoutTests/platform/chromium-win/platform/chromium/fast/forms/calendar-picker/month-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium-win/platform/chromium/fast/forms/calendar-picker/week-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-date-types-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-date-types.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime-with-step-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime-with-step.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetimelocal-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetimelocal.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-mouse-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-mouse-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-pre-100-year-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-pre-100-year.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-with-step-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-with-step.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/date-picker-events-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/date-picker-events.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/datetimelocal-picker-events-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/datetimelocal-picker-events.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-mouse-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-mouse-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-with-step-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-with-step.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/resources/calendar-picker-common.js', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-mouse-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-mouse-operations.html', u'ManualTests/forms/calendar-picker.html', u'ManualTests/forms/date-suggestion-picker.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/Resources/pagepopups/calendarPicker.css', u'Source/WebCore/Resources/pagepopups/calendarPicker.js', u'Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css', u'Source/WebCore/Resources/pagepopups/pickerCommon.js', u'Source/WebKit/chromium/ChangeLog', u'Source/WebKit/chromium/src/DateTimeChooserImpl.cpp']" exit_code: 1 LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-expected.png:0: Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5] Total errors found: 1 in 43 files If any of these errors are false positives, please file a bug against check-webkit-style.

Keishi Hattori

Comment 8 2013-02-18 02:58:47 PST

Created attachment 188839 [details] Patch

WebKit Review Bot

Comment 9 2013-02-18 03:03:52 PST

Attachment 188839 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-ar-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-required-ar-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-required-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-ru-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-step-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/month-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/month-picker-appearance-step-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-step-expected.png', u'LayoutTests/platform/chromium-win/platform/chromium/fast/forms/calendar-picker/month-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium-win/platform/chromium/fast/forms/calendar-picker/week-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-date-types-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-date-types.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime-with-step-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime-with-step.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetimelocal-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetimelocal.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-mouse-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-mouse-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-pre-100-year-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-pre-100-year.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-with-step-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-with-step.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/date-picker-events-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/date-picker-events.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/datetimelocal-picker-events-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/datetimelocal-picker-events.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-mouse-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-mouse-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-with-step-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-with-step.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/resources/calendar-picker-common.js', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-mouse-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-mouse-operations.html', u'ManualTests/forms/calendar-picker.html', u'ManualTests/forms/date-suggestion-picker.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/Resources/pagepopups/calendarPicker.css', u'Source/WebCore/Resources/pagepopups/calendarPicker.js', u'Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css', u'Source/WebCore/Resources/pagepopups/pickerCommon.js', u'Source/WebKit/chromium/ChangeLog', u'Source/WebKit/chromium/src/DateTimeChooserImpl.cpp']" exit_code: 1 LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-expected.png:0: Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5] Total errors found: 1 in 43 files If any of these errors are false positives, please file a bug against check-webkit-style.

WebKit Review Bot

Comment 10 2013-02-18 03:56:00 PST

Comment on attachment 188839 [details] Patch Attachment 188839 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16616087 New failing tests: platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-key-operations.html platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-key-operations.html platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-key-operations.html

Kent Tamura

Comment 11 2013-02-18 04:33:09 PST

The patch is still too large to review. The following classes seem to be generic and independent from calendar picker. Would you make some new patches for them please? - Observable - AnimationManager - Animator - View - ScrubbyScrollBar - ScrollView - ListCell - ListView

Keishi Hattori

Comment 12 2013-02-18 21:30:00 PST

Hi pfeldman@ tkent@ who usually does the reviews is on a one month leave. Because it's mostly javascript could you please do the reviews? Thanks. I've split the patch and uploaded the first few. Bug 110131 Bug 110132 Bug 110137 Bug 110140

Keishi Hattori

Comment 13 2013-02-28 13:06:36 PST

Created attachment 190789 [details] replace CalendarPicker

Kent Tamura

Comment 14 2013-02-28 19:43:21 PST

Comment on attachment 190789 [details] replace CalendarPicker We should add "Clear" button to the multiple fields UI before this patch because we'll remove "Clear" button in the calendar picker, right?

Keishi Hattori

Comment 15 2013-03-07 04:32:43 PST

Created attachment 191969 [details] Patch

WebKit Review Bot

Comment 16 2013-03-07 04:37:20 PST

Attachment 191969 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-ar-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-required-ar-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-required-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-ru-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-step-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/month-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/month-picker-appearance-step-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-step-expected.png', u'LayoutTests/platform/chromium-win/fast/forms/calendar-picker/date-open-picker-with-f4-key-expected.txt', u'LayoutTests/platform/chromium-win/fast/forms/calendar-picker/date-open-picker-with-f4-key.html', u'LayoutTests/platform/chromium-win/fast/forms/calendar-picker/datetimelocal-open-picker-with-f4-key-expected.txt', u'LayoutTests/platform/chromium-win/fast/forms/calendar-picker/datetimelocal-open-picker-with-f4-key.html', u'LayoutTests/platform/chromium-win/fast/forms/calendar-picker/month-open-picker-with-f4-key-expected.txt', u'LayoutTests/platform/chromium-win/fast/forms/calendar-picker/month-open-picker-with-f4-key.html', u'LayoutTests/platform/chromium-win/fast/forms/calendar-picker/week-open-picker-with-f4-key-expected.txt', u'LayoutTests/platform/chromium-win/fast/forms/calendar-picker/week-open-picker-with-f4-key.html', u'LayoutTests/platform/chromium-win/platform/chromium/fast/forms/calendar-picker/month-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium-win/platform/chromium/fast/forms/calendar-picker/week-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium-win/platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium-win/platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime-with-step-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime-with-step.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetimelocal-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetimelocal.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-mouse-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-mouse-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-pre-100-year-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-pre-100-year.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-with-step-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-with-step.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/date-picker-events-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/date-picker-events.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/datetimelocal-picker-events-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/datetimelocal-picker-events.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-mouse-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-mouse-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-with-step-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-with-step.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/resources/calendar-picker-common.js', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-mouse-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-mouse-operations.html', u'LayoutTests/platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-key-operations.html', u'ManualTests/forms/calendar-picker.html', u'ManualTests/forms/date-suggestion-picker.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/Resources/pagepopups/calendarPicker.css', u'Source/WebCore/Resources/pagepopups/calendarPicker.js', u'Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css', u'Source/WebKit/chromium/ChangeLog', u'Source/WebKit/chromium/src/DateTimeChooserImpl.cpp']" exit_code: 1 LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-expected.png:0: Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5] Total errors found: 1 in 57 files If any of these errors are false positives, please file a bug against check-webkit-style.

WebKit Review Bot

Comment 17 2013-03-07 05:31:54 PST

Comment on attachment 191969 [details] Patch Attachment 191969 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17010224 New failing tests: platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-key-operations.html

Kent Tamura

Comment 18 2013-03-07 19:11:54 PST

Comment on attachment 191969 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191969&action=review > Source/WebCore/Resources/pagepopups/calendarPicker.css:49 > + -webkit-transform: translate(0, 0); Why is this needed? > Source/WebCore/Resources/pagepopups/calendarPicker.css:70 > + vertical-align:middle; nit: need a space after : > Source/WebCore/Resources/pagepopups/calendarPicker.js:3284 > + this.setCurrentMonth(Month.createFromDate(new Date()), false); Is this needed? setCurrentMonth is always called again later in this function. > Source/WebCore/Resources/pagepopups/calendarPicker.js:3553 > +CalendarPicker.prototype._moveHighlight = function(dateRange) { should have JsDoc comment > Source/WebCore/Resources/pagepopups/calendarPicker.js:3642 > + switch (key) { > + case "U+001B": // Esc key. We don't add indentation for "case" > LayoutTests/ChangeLog:7 > + We should have a test to check the appearance of the month popup. r- because of this and the EWS failure. > LayoutTests/platform/chromium-win/fast/forms/calendar-picker/date-open-picker-with-f4-key-expected.txt:1 > +Tests if pressing F4 opens the calendar picker. Ideally, *-open-picker-with-f4-key* changes should be done before this patch to minimize the patch size.

Kent Tamura

Comment 19 2013-03-07 19:14:28 PST

Comment on attachment 191969 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191969&action=review > Source/WebKit/chromium/src/DateTimeChooserImpl.cpp:138 > addProperty("clearLabel", Platform::current()->queryLocalizedString(WebLocalizedString::CalendarClear), writer); We can remove some properties and some localized strings. It's ok to remove them in another patch.

Keishi Hattori

Comment 20 2013-03-07 22:36:17 PST

Created attachment 192148 [details] Patch

Keishi Hattori

Comment 21 2013-03-07 23:11:34 PST

Created attachment 192155 [details] Patch

Keishi Hattori

Comment 22 2013-03-07 23:11:58 PST

Comment on attachment 191969 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191969&action=review >> Source/WebCore/Resources/pagepopups/calendarPicker.css:49 >> + -webkit-transform: translate(0, 0); > > Why is this needed? Removed. >> Source/WebCore/Resources/pagepopups/calendarPicker.css:70 >> + vertical-align:middle; > > nit: need a space after : Removed this line. Vertical centering is done with line-height. >> Source/WebCore/Resources/pagepopups/calendarPicker.js:3284 >> + this.setCurrentMonth(Month.createFromDate(new Date()), false); > > Is this needed? setCurrentMonth is always called again later in this function. No. Removed. >> Source/WebCore/Resources/pagepopups/calendarPicker.js:3553 >> +CalendarPicker.prototype._moveHighlight = function(dateRange) { > > should have JsDoc comment Added. >> Source/WebCore/Resources/pagepopups/calendarPicker.js:3642 >> + case "U+001B": // Esc key. > > We don't add indentation for "case" Done. >> Source/WebKit/chromium/src/DateTimeChooserImpl.cpp:138 >> addProperty("clearLabel", Platform::current()->queryLocalizedString(WebLocalizedString::CalendarClear), writer); > > We can remove some properties and some localized strings. It's ok to remove them in another patch. Created Bug 111812. >> LayoutTests/ChangeLog:7 >> + > > We should have a test to check the appearance of the month popup. > r- because of this and the EWS failure. Added calendar-picker-appearance-month-popup.html >> LayoutTests/platform/chromium-win/fast/forms/calendar-picker/date-open-picker-with-f4-key-expected.txt:1 >> +Tests if pressing F4 opens the calendar picker. > > Ideally, *-open-picker-with-f4-key* changes should be done before this patch to minimize the patch size. Created Bug 111811. Removed tests.

Keishi Hattori

Comment 23 2013-03-07 23:13:51 PST

Created attachment 192156 [details] Patch

WebKit Review Bot

Comment 24 2013-03-07 23:18:07 PST

Attachment 192156 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-ar-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-month-popup-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-required-ar-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-required-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-ru-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-step-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/month-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/month-picker-appearance-step-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-step-expected.png', u'LayoutTests/platform/chromium-win/platform/chromium/fast/forms/calendar-picker/month-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium-win/platform/chromium/fast/forms/calendar-picker/week-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium-win/platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium-win/platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium-win/platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-month-popup-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-month-popup.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime-with-step-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime-with-step.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetime.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetimelocal-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-datetimelocal.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-mouse-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-mouse-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-pre-100-year-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-pre-100-year.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-with-step-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-with-step.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/date-picker-events-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/date-picker-events.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/datetimelocal-picker-events-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/datetimelocal-picker-events.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-mouse-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-mouse-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-with-step-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-with-step.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/resources/calendar-picker-common.js', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-mouse-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-mouse-operations.html', u'LayoutTests/platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-key-operations.html', u'ManualTests/forms/calendar-picker.html', u'ManualTests/forms/date-suggestion-picker.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/Resources/pagepopups/calendarPicker.css', u'Source/WebCore/Resources/pagepopups/calendarPicker.js', u'Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css', u'Source/WebKit/chromium/ChangeLog', u'Source/WebKit/chromium/src/DateTimeChooserImpl.cpp']" exit_code: 1 LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-expected.png:0: Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5] Total errors found: 1 in 52 files If any of these errors are false positives, please file a bug against check-webkit-style.

Kent Tamura

Comment 25 2013-03-07 23:26:19 PST

Comment on attachment 192156 [details] Patch ok. After landing this, please address the touch-friendly layout issue ASAP.

WebKit Review Bot

Comment 26 2013-03-07 23:39:14 PST

Comment on attachment 192156 [details] Patch Clearing flags on attachment: 192156 Committed r145186: <http://trac.webkit.org/changeset/145186>

WebKit Review Bot

Comment 27 2013-03-07 23:39:20 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.