Bug 98117 - [Chromium-Win] Implement LocaleWin::dateFormat
Summary: [Chromium-Win] Implement LocaleWin::dateFormat
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: 98109
Blocks: 97997
  Show dependency treegraph
 
Reported: 2012-10-01 22:53 PDT by Kent Tamura
Modified: 2012-10-02 18:17 PDT (History)
3 users (show)

See Also:


Attachments
Patch (5.17 KB, patch)
2012-10-02 01:48 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch 2 (5.27 KB, patch)
2012-10-02 01:53 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch 3 (9.12 KB, patch)
2012-10-02 03:51 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2012-10-01 22:53:33 PDT
[Chromium-Win] Implement LocaleWin::dateFormat
Comment 1 Kent Tamura 2012-10-02 01:48:49 PDT
Created attachment 166642 [details]
Patch
Comment 2 Kent Tamura 2012-10-02 01:53:42 PDT
Created attachment 166644 [details]
Patch 2
Comment 3 Kentaro Hara 2012-10-02 02:13:22 PDT
Comment on attachment 166644 [details]
Patch 2

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

> Source/WebCore/ChangeLog:13
> +        No new tests. The function is not used yet.

Isn't there any way to add a test? At least I want a manual test.

> Source/WebCore/platform/text/LocaleWin.cpp:703
> +    for (unsigned i = 0; i < literal.length(); ++i) {
> +        if (literal[i] == '\'')
> +            buffer.append("''");
> +        else {
> +            String escaped = literal.substring(i);
> +            escaped.replace(ASCIILiteral("'"), ASCIILiteral("''"));
> +            buffer.append("'");
> +            buffer.append(escaped);
> +            buffer.append("'");
> +            return;
> +        }
> +    }

I don't fully understand the behavior:

'' => ''''''
'abc' => '''abc'''
a'bc' => 'a''bc'''

?
Comment 4 Kent Tamura 2012-10-02 03:51:27 PDT
Created attachment 166657 [details]
Patch 3

Add test
Comment 5 Kent Tamura 2012-10-02 03:58:02 PDT
Comment on attachment 166644 [details]
Patch 2

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

>> Source/WebCore/ChangeLog:13
>> +        No new tests. The function is not used yet.
> 
> Isn't there any way to add a test? At least I want a manual test.

I exposed a function of LocaleWin, and added a test to LocaleWinTest.cpp.

>> Source/WebCore/platform/text/LocaleWin.cpp:703
>> +    }
> 
> I don't fully understand the behavior:
> 
> '' => ''''''
> 'abc' => '''abc'''
> a'bc' => 'a''bc'''
> 
> ?

| '' => ''''''

This should be: '' => ''''

| 'abc' => '''abc'''
| a'bc' => 'a''bc'''

Right.

- ' is always represented as ''
- other letters are always quoted by ' and '.
- If a source string starts with ', we need special handling.
    If the source string was ' and we replaced it with '' then wrap with ' and ', the function would produce '''', which will be recognized as two quotes.
Comment 6 Kentaro Hara 2012-10-02 04:08:14 PDT
Comment on attachment 166657 [details]
Patch 3

Looks OK.
Comment 7 WebKit Review Bot 2012-10-02 18:17:04 PDT
Comment on attachment 166657 [details]
Patch 3

Clearing flags on attachment: 166657

Committed r130242: <http://trac.webkit.org/changeset/130242>
Comment 8 WebKit Review Bot 2012-10-02 18:17:07 PDT
All reviewed patches have been landed.  Closing bug.