Bug 109530

Summary: PagePopupController.formatMonth should support short month format
Product: WebKit Reporter: Keishi Hattori <keishi>
Component: FormsAssignee: Keishi Hattori <keishi>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 109439    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Properly add shortMonthFormat() to the LocaleNone class none

Keishi Hattori
Reported 2013-02-11 18:01:44 PST
We need to support short month format for use in the calendar picker.
Attachments
Patch (14.35 KB, patch)
2013-02-15 00:28 PST, Keishi Hattori 		no flags
DetailsFormatted DiffDiff
Patch (17.52 KB, patch)
2013-02-15 03:19 PST, Keishi Hattori 		no flags
DetailsFormatted DiffDiff
Patch (14.56 KB, patch)
2013-02-15 04:12 PST, Keishi Hattori 		no flags
DetailsFormatted DiffDiff
Properly add shortMonthFormat() to the LocaleNone class (1.43 KB, patch)
2013-02-19 04:25 PST, Alberto Garcia 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Keishi Hattori
Comment 1 2013-02-15 00:28:44 PST
Created attachment 188497 [details] Patch
Kent Tamura
Comment 2 2013-02-15 00:44:13 PST
Comment on attachment 188497 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=188497&action=review > Source/WebCore/page/PagePopupController.idl:37 > - [Conditional=CALENDAR_PICKER] DOMString formatMonth(in long year, in long zeroBaseMonth); > + [Conditional=CALENDAR_PICKER] DOMString formatMonth(in long year, in long zeroBaseMonth, in boolean useShortMonth); We had better introduce a separated function like formatShortMonth(year, month). > Source/WebCore/platform/text/PlatformLocale.cpp:357 > + builder.build(formatType == FormatTypeShort ? shortMonthFormat() : monthFormat()); nit: We had better explain this behavior on the formatDateTime declaration in PlatformLocale.h. > Source/WebCore/platform/text/PlatformLocale.h:71 > // Returns a year-month format in Unicode TR35 LDML. > virtual String monthFormat() = 0; > > + // Returns a year-month format in Unicode TR35 LDML. > + virtual String shortMonthFormat() = 0; We should mention difference between them. > Source/WebCore/platform/text/mac/LocaleMac.mm:213 > + // Gets a format for "MMMM" because Windows API always provides formats for > + // "MMMM" in some locales. > + m_shortMonthFormat = [NSDateFormatter dateFormatFromTemplate:@"yyyyMMM" options:0 locale:m_locale.get()]; incorrect comment.
Keishi Hattori
Comment 3 2013-02-15 03:19:35 PST
Created attachment 188524 [details] Patch
Kent Tamura
Comment 4 2013-02-15 03:26:34 PST
Comment on attachment 188524 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=188524&action=review ok > Source/WebCore/page/PagePopupController.idl:36 > + void setValue(in DOMString value); nit: in is unnecessary
Kent Tamura
Comment 5 2013-02-15 03:28:00 PST
you posted a patch to a wrong bug.
Keishi Hattori
Comment 6 2013-02-15 04:12:45 PST
Created attachment 188533 [details] Patch
Kent Tamura
Comment 7 2013-02-15 04:58:02 PST
Comment on attachment 188533 [details] Patch ok
WebKit Review Bot
Comment 8 2013-02-15 06:06:47 PST
Comment on attachment 188533 [details] Patch Clearing flags on attachment: 188533 Committed r142988: <http://trac.webkit.org/changeset/142988>
WebKit Review Bot
Comment 9 2013-02-15 06:06:51 PST
All reviewed patches have been landed. Closing bug.
Alberto Garcia
Comment 10 2013-02-19 04:25:33 PST
Created attachment 189051 [details] Properly add shortMonthFormat() to the LocaleNone class The shortMonthFormat() was not correctly added to the LocaleNone class, here's the fix.
Keishi Hattori
Comment 11 2013-02-19 05:01:30 PST
(In reply to comment #10) > Created an attachment (id=189051) [details] > Properly add shortMonthFormat() to the LocaleNone class > > The shortMonthFormat() was not correctly added to the LocaleNone class, here's the fix. Oops, sorry. I'm not a reviewer but every bug should have only one patch. If this is a build fix I can land it as unreviewed.
Alberto Garcia
Comment 12 2013-02-19 05:07:49 PST
(In reply to comment #11) > (In reply to comment #10) > > Created an attachment (id=189051) [details] [details] > > Properly add shortMonthFormat() to the LocaleNone class > > > > The shortMonthFormat() was not correctly added to the LocaleNone class, here's the fix. > > Oops, sorry. I'm not a reviewer but every bug should have only one patch. If this is a build fix I can land it as unreviewed. It's a build fix, and I can create a new bug if you want.
Keishi Hattori
Comment 13 2013-02-19 05:12:23 PST
(In reply to comment #12) > (In reply to comment #11) > > (In reply to comment #10) > > > Created an attachment (id=189051) [details] [details] [details] > > > Properly add shortMonthFormat() to the LocaleNone class > > > > > > The shortMonthFormat() was not correctly added to the LocaleNone class, here's the fix. > > > > Oops, sorry. I'm not a reviewer but every bug should have only one patch. If this is a build fix I can land it as unreviewed. > > It's a build fix, and I can create a new bug if you want. In that case I'll land this right now. Thanks for making it!
Keishi Hattori
Comment 14 2013-02-19 05:18:36 PST
Comment on attachment 189051 [details] Properly add shortMonthFormat() to the LocaleNone class landed. http://trac.webkit.org/changeset/143320
Note You need to log in before you can comment on or make changes to this bug.