Show the format indicator in a date field
Created attachment 137066 [details] WIP
I'll split the patch into multiple.
Created attachment 137851 [details] Patch
Comment on attachment 137851 [details] Patch I hope this to be tested in non-pixel way somehow. For example, we can access the dumped render treee using Internals.elementRenderTreeAsText(), which allows us to inspect the render tree programatically. Or maybe we can simply use add a reftest.
Comment on attachment 137851 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=137851&action=review > Source/WebCore/platform/text/LocalizedDate.h:49 > +// Localized date format string such as "month/day/year". > +String dateFormatText(); Instead of adding a comment, can we simply name this function localizedDateFormatString? > Source/WebCore/platform/text/LocalizedDateICU.cpp:138 > + if (lastChar == 'y' || lastChar == 'Y') { Can we add a comment saying that these Y, M, d, etc... follow ICU convention? Or maybe we can define inline helper functions like isICUYearSymbol (We probably prefer the latter approach). > Source/WebCore/platform/text/LocalizedDateICU.cpp:140 > + builder.append(text.isEmpty() ? "year" : text); Maybe we should capitalize these? > LayoutTests/fast/forms/date/date-appearance.html:21 > +<div><input type="date" value=""></div> Can we use a ref-test here?
Created attachment 138315 [details] Patch 2
Comment on attachment 137851 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=137851&action=review >> Source/WebCore/platform/text/LocalizedDate.h:49 >> +String dateFormatText(); > > Instead of adding a comment, can we simply name this function localizedDateFormatString? ok, I renamed it. >> Source/WebCore/platform/text/LocalizedDateICU.cpp:138 >> + if (lastChar == 'y' || lastChar == 'Y') { > > Can we add a comment saying that these Y, M, d, etc... follow ICU convention? > Or maybe we can define inline helper functions like isICUYearSymbol (We probably prefer the latter approach). I inroduced isICUFooSymbol functions. >> Source/WebCore/platform/text/LocalizedDateICU.cpp:140 >> + builder.append(text.isEmpty() ? "year" : text); > > Maybe we should capitalize these? Done. >> LayoutTests/fast/forms/date/date-appearance.html:21 >> +<div><input type="date" value=""></div> > > Can we use a ref-test here? I made a text test using window.internals.visiblePlaceholder().
Comment on attachment 138315 [details] Patch 2 Looks better now.
Comment on attachment 138315 [details] Patch 2 Rejecting attachment 138315 [details] from commit-queue. New failing tests: fast/forms/date/date-appearance.html Full output: http://queues.webkit.org/results/12528070
Created attachment 138455 [details] Archive of layout-test-results from ec2-cq-02 The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: ec2-cq-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Created attachment 138491 [details] Patch for landing
Comment on attachment 138491 [details] Patch for landing Clearing flags on attachment: 138491 Committed r114999: <http://trac.webkit.org/changeset/114999>
All reviewed patches have been landed. Closing bug.