Bug 110140

Created attachment 188916 [details] Patch

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

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.

> > 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.

Created attachment 190004 [details] Patch

Comment on attachment 190004 [details] Patch ok

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

Comment on attachment 190004 [details] Patch Clearing flags on attachment: 190004 Committed r143901: <http://trac.webkit.org/changeset/143901>

All reviewed patches have been landed. Closing bug.