Bug 101329 - [Chromium-win] Refactor date/time format conversion code in LocaleWin
Summary: [Chromium-win] Refactor date/time format conversion code in LocaleWin
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kent Tamura
URL:
Keywords:
Depends on: 101461
Blocks:
  Show dependency tree / graph
 
Reported: 2012-11-06 02:49 PST by Kent Tamura
Modified: 2012-11-07 20:12 PST (History)
2 users (show)

See Also:

Attachments
Patch (13.86 KB, patch)
2012-11-06 03:02 PST, Kent Tamura 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2012-11-06 02:49:45 PST 
[Chromium-win] Refactor date/time format parsing code in LocaleWin
Comment 1 Kent Tamura 2012-11-06 03:02:15 PST 
Created attachment 172535 [details]
Patch
Comment 2 Kentaro Hara 2012-11-06 03:48:38 PST 
Comment on attachment 172535 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=172535&action=review

> Source/WebCore/platform/text/win/LocaleWin.cpp:242
> +                else
> +                    converted.append("EEEE");

'ddddd' is converted to 'EEEE'. Is it expected?

> Source/WebCore/platform/text/win/LocaleWin.cpp:251
> +            } else if (ch == 't')
> +                converted.append('a');

't' and 'tt' are converted to 'a', which looks ok. 'ttt' is also converted to 'a', is it expected?
Comment 3 Kent Tamura 2012-11-06 05:00:14 PST 
Comment on attachment 172535 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=172535&action=review

>> Source/WebCore/platform/text/win/LocaleWin.cpp:242
>> +                    converted.append("EEEE");
> 
> 'ddddd' is converted to 'EEEE'. Is it expected?

Yes.
ddddd is invalid according to MSDN, and users can't set arbitrary format in Control Panel. ddddd should not appear unless OS is corrupted, and any behavior would be ok here.

>> Source/WebCore/platform/text/win/LocaleWin.cpp:251
>> +                converted.append('a');
> 
> 't' and 'tt' are converted to 'a', which looks ok. 'ttt' is also converted to 'a', is it expected?

Ditto.
Comment 4 Kentaro Hara 2012-11-06 05:00:57 PST 
Comment on attachment 172535 [details]
Patch

Makes sense. Thanks for the clarification.
Comment 5 WebKit Review Bot 2012-11-06 19:34:06 PST 
Comment on attachment 172535 [details]
Patch

Clearing flags on attachment: 172535

Committed r133712: <http://trac.webkit.org/changeset/133712>
Comment 6 WebKit Review Bot 2012-11-06 19:34:10 PST 
All reviewed patches have been landed.  Closing bug.
Comment 7 WebKit Review Bot 2012-11-07 06:04:22 PST 
Re-opened since this is blocked by bug 101461
Comment 8 Kent Tamura 2012-11-07 17:20:45 PST 
(In reply to comment #7)
> Re-opened since this is blocked by bug 101461

Oh, XP-only failures.  I found my change unveiled a bug in the XP/Vista-only path of LocaleWin::shortTimeFormat, which converts a format twice.
Comment 9 Kent Tamura 2012-11-07 20:12:46 PST 
Committed r133842: <http://trac.webkit.org/changeset/133842>