Bug 97517 - REGRESSION(r129448): multiple fields time input UI doesn't use system time format settings on Chromium-Mac
Summary: REGRESSION(r129448): multiple fields time input UI doesn't use system time fo...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keishi Hattori
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-24 21:15 PDT by yosin
Modified: 2012-09-25 21:53 PDT (History)
2 users (show)

See Also:

Attachments
Patch (7.26 KB, patch)
2012-09-24 21:46 PDT, Keishi Hattori 		no flags Details | Formatted Diff | Diff
Patch (5.72 KB, patch)
2012-09-24 21:50 PDT, Keishi Hattori 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) 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 yosin 2012-09-24 21:15:59 PDT 
We should use NSLocale object returned by [NSLocale currentLocale] if browser language/@lang equals [[NSLocal currentLocale] localeIdentifier].
Comment 1 yosin 2012-09-24 21:16:51 PDT 
Imported from http://crbug.com/152080 - Input type=time can not convert time to 24hr format after OS format changed
Comment 2 Keishi Hattori 2012-09-24 21:46:08 PDT 
Created attachment 165517 [details]
Patch
Comment 3 yosin 2012-09-24 21:49:35 PDT 
Comment on attachment 165517 [details]
Patch

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

> Source/WebKit/chromium/src/WebPagePopupImpl.cpp:42
>  #include "PagePopupClient.h"

Please remove WebPagePopupImpl.cpp from this patch. This changes are unrelated.
Comment 4 Keishi Hattori 2012-09-24 21:50:12 PDT 
Created attachment 165518 [details]
Patch
Comment 5 yosin 2012-09-24 21:57:48 PDT 
Comment on attachment 165518 [details]
Patch

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

> Source/WebCore/platform/text/mac/LocaleMac.mm:55
> +    return normalizedLocale.left(separatorPosition);

Is it OK to compare only language part?

It seems that short date format for en_CA and en_US are different.

CA yy-MM-dd
GB dd/MM/yyyy

http://unicode.org/cldr/trac/browser/tags/release-21-0-1/common/main/en_CA.xml
http://unicode.org/cldr/trac/browser/tags/release-21-0-1/common/main/en_GB.xml

Should we have another bug to fix this?
Comment 6 Kent Tamura 2012-09-25 02:34:05 PDT 
Comment on attachment 165518 [details]
Patch

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

>> Source/WebCore/platform/text/mac/LocaleMac.mm:55
>> +    return normalizedLocale.left(separatorPosition);
> 
> Is it OK to compare only language part?
> 
> It seems that short date format for en_CA and en_US are different.
> 
> CA yy-MM-dd
> GB dd/MM/yyyy
> 
> http://unicode.org/cldr/trac/browser/tags/release-21-0-1/common/main/en_CA.xml
> http://unicode.org/cldr/trac/browser/tags/release-21-0-1/common/main/en_GB.xml
> 
> Should we have another bug to fix this?

Yeah, It should be a separated issue.
Comment 7 WebKit Review Bot 2012-09-25 02:39:12 PDT 
Comment on attachment 165518 [details]
Patch

Clearing flags on attachment: 165518

Committed r129480: <http://trac.webkit.org/changeset/129480>
Comment 8 WebKit Review Bot 2012-09-25 02:39:15 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 9 Darin Adler 2012-09-25 09:31:25 PDT 
Comment on attachment 165518 [details]
Patch

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

> Source/WebCore/platform/text/mac/LocaleMac.mm:66
> +    return [[NSLocale alloc] initWithLocaleIdentifier:locale];

This leaks the NSLocale object.

> Source/WebCore/platform/text/mac/LocaleMac.mm:-71
> -    : m_locale([[NSLocale alloc] initWithLocaleIdentifier:localeIdentifier])

This old incorrect code leaked the NSLocale object.

> Source/WebCore/platform/text/mac/LocaleMac.mm:102
> +    return adoptPtr(new LocaleMac([[NSLocale alloc] initWithLocaleIdentifier:localeIdentifier]));

This leaks the NSLocale object.
Comment 10 Keishi Hattori 2012-09-25 21:53:44 PDT 
(In reply to comment #8)
> All reviewed patches have been landed.  Closing bug.

(In reply to comment #9)
> (From update of attachment 165518 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=165518&action=review
> 
> > Source/WebCore/platform/text/mac/LocaleMac.mm:66
> > +    return [[NSLocale alloc] initWithLocaleIdentifier:locale];
> 
> This leaks the NSLocale object.
> 
I filed a bug Bug 97628 .