Bug 97517 – REGRESSION(r129448): multiple fields time input UI doesn't use system time format settings on Chromium-Mac

• Component: Forms
• Status: RESOLVED FIXED

We should use NSLocale object returned by [NSLocale currentLocale] if browser language/@lang equals [[NSLocal currentLocale] localeIdentifier].

Imported from http://crbug.com/152080 - Input type=time can not convert time to 24hr format after OS format changed

[Keishi Hattori]

Created attachment 165517 [details]
Patch

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.

[Keishi Hattori]

Created attachment 165518 [details]
Patch

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?

[Kent Tamura]

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.

[WebKit Review Bot]

Comment on attachment 165518 [details]
Patch

Clearing flags on attachment: 165518

Committed r129480: <http://trac.webkit.org/changeset/129480>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

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.

[Keishi Hattori]

(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 .