Summary: | PagePopupController.formatMonth should support short month format | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Keishi Hattori <keishi> | ||||||||||
Component: | Forms | Assignee: | 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
Keishi Hattori
2013-02-11 18:01:44 PST
Created attachment 188497 [details]
Patch
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. Created attachment 188524 [details]
Patch
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 you posted a patch to a wrong bug. Created attachment 188533 [details]
Patch
Comment on attachment 188533 [details]
Patch
ok
Comment on attachment 188533 [details] Patch Clearing flags on attachment: 188533 Committed r142988: <http://trac.webkit.org/changeset/142988> All reviewed patches have been landed. Closing bug. 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.
(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. (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. (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! Comment on attachment 189051 [details] Properly add shortMonthFormat() to the LocaleNone class landed. http://trac.webkit.org/changeset/143320 |