Adjust design of the Calendar Picker. Use Chrome style buttons. Gray border color. etc.
Created attachment 183900 [details] Patch
Attachment 183900 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." 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 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 183900 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183900&action=review > Source/WebCore/Resources/pagepopups/calendarPicker.css:166 > + height: 24px; Absolute height is dangerous because -webkit-control font depends on local computer setting. > Source/WebCore/Resources/pagepopups/calendarPicker.css:175 > + line-height: 18px; ditto. > Source/WebCore/Resources/pagepopups/calendarPicker.js:898 > + disclosureTriangle.innerHTML = "<svg width=\"7\" height=\"5\"><polygon points=\"0,1 7,1 3.5,5\" style=\"fill:#000000;\" /></svg>"; nit: You can avoid ugly escaping by using single quotes in the svg string. > Source/WebCore/Resources/pagepopups/calendarPicker.js:945 > + this._left2.innerHTML = "<svg width=\"9\" height=\"7\"><polygon points=\"0,3.5 4,7 4,0\" style=\"fill:#6e6e6e;\" /><polygon points=\"5,3.5 9,7 9,0\" style=\"fill:#6e6e6e;\" /></svg>"; ditto. > Source/WebCore/Resources/pagepopups/calendarPicker.js:950 > + this._left1.innerHTML = "<svg width=\"4\" height=\"7\"><polygon points=\"0,3.5 4,7 4,0\" style=\"fill:#6e6e6e;\" /></svg>"; ditto. > Source/WebCore/Resources/pagepopups/calendarPicker.js:962 > + this._right1.innerHTML = "<svg width=\"4\" height=\"7\"><polygon points=\"0,7 0,0, 4,3.5\" style=\"fill:#6e6e6e;\" /></svg>"; ditto. > Source/WebCore/Resources/pagepopups/calendarPicker.js:967 > + this._right2.innerHTML = "<svg width=\"9\" height=\"7\"><polygon points=\"4,3.5 0,7 0,0\" style=\"fill:#6e6e6e;\" /><polygon points=\"9,3.5 5,7 5,0\" style=\"fill:#6e6e6e;\" /></svg>"; ditto. > Source/WebCore/Resources/pagepopups/calendarPicker.js:-1018 > - this._monthPopup.style.left = this._month.offsetLeft + (this._month.offsetWidth - this._monthPopup.offsetWidth) / 2 + "px"; > - this._monthPopup.style.top = this._month.offsetTop + this._month.offsetHeight + "px"; > + var bounds = this.picker.daysTableBounds(); > + this._monthPopup.style.left = bounds.x + "px"; > + this._monthPopup.style.top = bounds.y + "px"; > + this._monthPopup.style.width = bounds.width + "px"; > + this._monthPopup.style.height = bounds.height + "px"; > > this._wall.style.display = "block"; > this._wall.style.zIndex = "999"; // This should be smaller than the z-index of monthPopup. > > - var popupHeight = this._monthPopup.clientHeight; > - var fullHeight = this._monthPopupContents.clientHeight; > - if (fullHeight > popupHeight) { > - var selected = this._getSelection(); > - if (selected) { > - var bottom = selected.offsetTop + selected.clientHeight; > - if (bottom > popupHeight) > - this._monthPopup.scrollTop = bottom - popupHeight; > - } > - this._monthPopup.style.webkitPaddingEnd = getScrollbarWidth() + 'px'; > - } Should we do this change now? I think this change affects UX, and would be out of scope of this bug (appearance change). > Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css:10 > +.days-area-container:focus { > + -webkit-transition: border-color 200ms; We use 4-space indentation. > Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css:9 > + inset 0 1px 2px rgba(255, 255, 255, 0.75); Need additional indentation for this line. > Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css:21 > +:enabled:hover:-webkit-any(button, input[type='button']) { > + background-image: -webkit-linear-gradient(#f0f0f0, #f0f0f0 38%, #e0e0e0); We use 4-space indentation. > Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css:35 > +:disabled:-webkit-any(button, input[type='button']) { > + background-image: -webkit-linear-gradient(#f1f1f1, #f1f1f1 38%, #e6e6e6); ditto. > Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css:43 > +:enabled:focus:-webkit-any(button, input[type='button']) { > + -webkit-transition: border-color 200ms; ditto. > Source/WebCore/WebCore.gyp/WebCore.gyp:-1350 > - ['OS=="mac"', { > - 'include_dirs': [ > - '<(chromium_src_dir)/third_party/apple_webkit', > - ], You shouldn't remove this block.
Created attachment 184121 [details] Patch
Created attachment 184123 [details] Patch
Attachment 184123 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." 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 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 183900 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183900&action=review >> Source/WebCore/Resources/pagepopups/calendarPicker.js:-1018 >> - } > > Should we do this change now? I think this change affects UX, and would be out of scope of this bug (appearance change). Removing this whole block was a mistake. I just wanted to remove this line. 1017: this._monthPopup.style.webkitPaddingEnd = getScrollbarWidth() + 'px';
Created attachment 184128 [details] Patch
Attachment 184128 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." 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 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 184128 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=184128&action=review > Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css:10 > +.days-area-container:focus { > + -webkit-transition: border-color 200ms; wrong indentation. > Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css:13 > + border-color: rgb(77, 144, 254); should be -webkit-focus-ring-color. In Chromium Settings page, we respect system's focus ring color. > Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css:44 > + border-color: rgb(77, 144, 254); should be -webkit-focus-ring-color.
Created attachment 184153 [details] Patch
Attachment 184153 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." 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 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 184153 [details] Patch Looks good.
Comment on attachment 184153 [details] Patch Rejecting attachment 184153 [details] from commit-queue. New failing tests: platform/chromium/fast/forms/calendar-picker/week-picker-key-operations.html platform/chromium/fast/forms/calendar-picker/month-picker-key-operations.html platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations.html Full output: http://queues.webkit.org/results/16069003
Comment on attachment 184153 [details] Patch Attachment 184153 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16078064 New failing tests: platform/chromium/fast/forms/calendar-picker/week-picker-key-operations.html platform/chromium/fast/forms/calendar-picker/month-picker-key-operations.html platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations.html
Created attachment 184187 [details] Patch
Comment on attachment 184187 [details] Patch Rejecting attachment 184187 [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-03', 'validate-changelog', '--non-interactive', 184187, '--port=chromium-xvfb']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue /mnt/git/webkit-commit-queue/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/16072148
Created attachment 184188 [details] Patch
Comment on attachment 184188 [details] Patch Rejecting attachment 184188 [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-01', 'validate-changelog', '--non-interactive', 184188, '--port=chromium-xvfb']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue /mnt/git/webkit-commit-queue/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/16065547
Attachment 184188 [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-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/TestExpectations', 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/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/week-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-key-operations.html', u'ManualTests/forms/calendar-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/calendarPickerMac.css', u'Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css', u'Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css', u'Source/WebCore/WebCore.gyp/WebCore.gyp', u'Source/WebCore/rendering/RenderTheme.cpp', u'Source/WebCore/rendering/RenderTheme.h', u'Source/WebCore/rendering/RenderThemeChromiumMac.h', u'Source/WebCore/rendering/RenderThemeChromiumMac.mm', 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 19 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 184188 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=184188&action=review > ChangeLog:13 > +2013-01-23 Keishi Hattori <keishi@webkit.org> > + > + Adjust design of the Calendar Picker > + https://bugs.webkit.org/show_bug.cgi?id=107507 > + > + Reviewed by NOBODY (OOPS!). > + > + * ManualTests/forms/calendar-picker.html: > + > +2013-01-21 Keishi Hattori <keishi@webkit.org> > + > + Adjust design of the Calendar Picker > + https://bugs.webkit.org/show_bug.cgi?id=107507 Duplicated ChangeLog entries.
Created attachment 184381 [details] Patch
Comment on attachment 184381 [details] Patch Rejecting attachment 184381 [details] from commit-queue. New failing tests: platform/chromium/fast/forms/calendar-picker/week-picker-key-operations.html platform/chromium/fast/forms/calendar-picker/month-picker-key-operations.html platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations.html Full output: http://queues.webkit.org/results/16073603
Comment on attachment 184381 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=184381&action=review > LayoutTests/ChangeLog:12 > + * platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations-expected.txt: > + * platform/chromium/fast/forms/calendar-picker/month-picker-key-operations-expected.txt: > + * platform/chromium/fast/forms/calendar-picker/month-picker-key-operations.html: > + * platform/chromium/fast/forms/calendar-picker/week-picker-key-operations-expected.txt: You need to update platform/chromium-win/platform/chromium/fast/forms/calendar-picker/*-picker-key-operations.html too. We hit this platform difference several times. We might want to make separated tests for F4 key behavior.
Comment on attachment 184381 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=184381&action=review >> LayoutTests/ChangeLog:12 >> + * platform/chromium/fast/forms/calendar-picker/week-picker-key-operations-expected.txt: > > You need to update platform/chromium-win/platform/chromium/fast/forms/calendar-picker/*-picker-key-operations.html too. > > We hit this platform difference several times. We might want to make separated tests for F4 key behavior. operations.html -> operations-expected.txt
Created attachment 184653 [details] Patch
Comment on attachment 184653 [details] Patch Rejecting attachment 184653 [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-03', 'apply-attachment', '--no-update', '--non-interactive', 184653, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: ps/calendarPickerMac.css' patch: **** Can't create file /tmp/ppKjAyJ9 : No space left on device fatal: pathspec 'Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css' did not match any files Failed to git add Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css. at /mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply line 448. Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/16094015
Comment on attachment 184653 [details] Patch Clearing flags on attachment: 184653 Committed r140778: <http://trac.webkit.org/changeset/140778>
All reviewed patches have been landed. Closing bug.
Comment on attachment 184653 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=184653&action=review > Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css:1 > +.year-month-button { We should have added a copyright header. > Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css:1 > +input[type='button'], ditto.
(In reply to comment #30) > (From update of attachment 184653 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=184653&action=review > > > Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css:1 > > +.year-month-button { > > We should have added a copyright header. > > > Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css:1 > > +input[type='button'], > > ditto. Added by http://trac.webkit.org/changeset/142359