Bug 110140 – Add list view for new calendar picker

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Keishi Hattori]

Adding list view for new calendar picker

[Keishi Hattori]

Created attachment 188916 [details]
Patch

[WebKit Review Bot]

Comment on attachment 188916 [details]
Patch

Attachment 188916 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16690061

New failing tests:
platform/chromium/fast/forms/suggestion-picker/datetimelocal-suggestion-picker-appearance-rtl.html
platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-step-attribute.html
platform/chromium/fast/forms/page-popup/page-popup-adjust-rect.html
platform/chromium/fast/forms/calendar-picker/month-picker-key-operations.html
inspector/audits/audits-panel-functional.html
platform/chromium/fast/forms/calendar-picker/calendar-picker-datetimelocal.html
platform/chromium/fast/forms/calendar-picker/calendar-picker-with-step.html
platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-appearance.html
fast/loader/text-document-wrapping.html
fast/text/international/hindi-spacing.html
platform/chromium/fast/forms/suggestion-picker/datetimelocal-suggestion-picker-key-operations.html
platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-appearance-with-scroll-bar.html
platform/chromium/fast/forms/suggestion-picker/datetimelocal-suggestion-picker-appearance-with-scroll-bar.html
platform/chromium/fast/forms/calendar-picker/date-picker-events.html
platform/chromium/fast/forms/calendar-picker/calendar-picker-pre-100-year.html
platform/chromium/fast/forms/calendar-picker/week-picker-mouse-operations.html
platform/chromium/fast/forms/suggestion-picker/datetimelocal-suggestion-picker-appearance.html
platform/chromium/fast/forms/calendar-picker/month-picker-with-step.html
platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-key-operations.html
platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-min-max-attribute.html
platform/chromium/fast/forms/suggestion-picker/datetimelocal-suggestion-picker-appearance-locale-hebrew.html
platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-appearance-rtl.html
platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations.html
platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-reset-value-after-reload.html
platform/chromium/fast/forms/calendar-picker/calendar-picker-type-change-onchange.html
platform/chromium/fast/forms/calendar-picker/month-picker-mouse-operations.html
platform/chromium/fast/forms/calendar-picker/datetimelocal-picker-events.html
platform/chromium/fast/forms/calendar-picker/calendar-picker-f4-key.html
platform/chromium/fast/forms/calendar-picker/week-picker-key-operations.html
fast/loader/javascript-url-in-object.html

[Kent Tamura]

Comment on attachment 188916 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=188916&action=review

> Source/WebCore/Resources/pagepopups/calendarPicker.js:1097
> +    this.row = null;
> +    this._width = 0;
> +    this._position = 0;

Data members should have JsDoc comments in the constructor.

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

Array of what?

> Source/WebCore/Resources/pagepopups/calendarPicker.js:1116
> +    if (this._recycleBin().length < limit)
> +        this._recycleBin().push(this);

this._recycleBin() should be stored to a local variable.  Calling it twice makes unnecessary constraint.

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

should mention it's in pixel unit.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:1135
> + * @param {!number} width

ditto.

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

ditto.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:1152
> + * @param {!number} y

ditto.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:1187
> +    this._width = 0;
> +    this._cells = {};
> +    
> +    this.selectedRow = ListView.NoSelection;
> +
> +    this.scrollView = new ScrollView();

Data members should have JsDoc comments in the constructor.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:1196
> +    this._needsUpdateCells = false;

ditto.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:1201
> +ListView.NoSelection = -1;

Should this be public?

> Source/WebCore/Resources/pagepopups/calendarPicker.js:1218
> +        AnimationManager.shared.on("animationFrameWillFinish", this.onAnimationFrameWillFinish);
> +    else
> +        AnimationManager.shared.removeListener("animationFrameWillFinish", this.onAnimationFrameWillFinish);

Use AnimationManager.EventTypeAnimationFrameWillFinish

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

mention it's in pixel unit.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:1364
> + * @param {!number} height

ditto.

[Keishi Hattori]

> > Source/WebCore/Resources/pagepopups/calendarPicker.js:1201
> > +ListView.NoSelection = -1;
> 
> Should this be public?
It could be private, but I think it should be public in case we want to check if a selection exists.

[Keishi Hattori]

Created attachment 190004 [details]
Patch

[Kent Tamura]

Comment on attachment 190004 [details]
Patch

ok

[WebKit Review Bot]

Comment on attachment 190004 [details]
Patch

Rejecting attachment 190004 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 190004, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:

fatal: read error: Connection reset by peer
Died at Tools/Scripts/update-webkit line 151.

Failed to run "['Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 2

Updating OpenSource
fatal: read error: Connection reset by peer
Died at Tools/Scripts/update-webkit line 151.

Failed to run "['Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 2
Updating OpenSource
fatal: read error: Connection reset by peer
Died at Tools/Scripts/update-webkit line 151.

Full output: http://queues.webkit.org/results/16746147

[WebKit Review Bot]

Comment on attachment 190004 [details]
Patch

Clearing flags on attachment: 190004

Committed r143901: <http://trac.webkit.org/changeset/143901>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.