Code in the Clock apps makes a lot of assumptions on hard-coded strings and item order. Bug 932270 made AM|PM localizable, which breaks basically everything in this app. Utils.is12hFormat is fine, since it uses %p from dateTimeFormat_%X to determine if format is 12 or 24 hours. https://github.com/mozilla-b2g/gaia/blob/master/apps/clock/js/utils.js#L221 But then Utils.parseTime breaks everything https://github.com/mozilla-b2g/gaia/blob/master/apps/clock/js/utils.js#L250 First using ":" to split hours and minutes, then assuming %p is after %I:%M, finally using the position of "M" to determine if it's 12 or 24 hours. Another problem is in Utils.format https://github.com/mozilla-b2g/gaia/blob/master/apps/clock/js/utils.js#L379

Note that similarly to bug 932356, this app should be able to display AM/PM before hours and minutes, according to dateTimeFormat_%X

For reference. AM/PM http://www.unicode.org/cldr/charts/latest/by_type/date_&_time.fields.html#6b2fa3ccf14e33a8 Separator http://www.unicode.org/cldr/charts/latest/by_type/date_&_time.gregorian.html#7637c409f3271b97 Instead of ":", we should probably read the character between %H/%I and %M, and fallback to ":" in the worst case (broken date format). We should probably use shortTimeFormat instead of dateTimeFormat_%X, since we're talking about time.

Thanks for this sharp bug report: parsing the time formats is fine, parsing the formatted time strings is not. (In reply to Francesco Lodolo [:flod] from comment #0) > Bug 932270 made AM|PM localizable, which breaks basically everything in this > app. Just a word in case this comes to triage: bug 932270 will break this app if localizers choose to use a non-[AM|PM] string for their locale. So this is a corner case but yes, this should be fixed properly — e.g. to properly support Chinese. > But then Utils.parseTime breaks everything > https://github.com/mozilla-b2g/gaia/blob/master/apps/clock/js/utils.js#L250 True. IIRC we used to have the same issue with the homescreen clock for Brazilian (the pt-BR locale used 'h' as a separator instead of ':'). I thought this issue had been solved, but maybe the “fix” has been to use a colon separator for Brazilian…

(In reply to Fabien Cazenave [:kaze] from comment #3) > True. IIRC we used to have the same issue with the homescreen clock for > Brazilian (the pt-BR locale used 'h' as a separator instead of ':'). I > thought this issue had been solved, but maybe the “fix” has been to use a > colon separator for Brazilian… I didn't see this one (wasn't really following Gaia bugs at the time): bug 824706. Portuguese definitely has peculiar time formats, which makes me think that Clock could be already failing there https://hg.mozilla.org/gaia-l10n/pt-BR/file/default/shared/date/date.properties#l196 For reference, zh-TW https://hg.mozilla.org/gaia-l10n/zh-TW/file/f0b97f49c661/shared/date/date.properties#l197 zh-CN is different, I wonder if that's just an error in the localization (i.e. not sure how much testing they're doing) https://hg.mozilla.org/gaia-l10n/zh-CN/file/183a412f6d06/shared/date/date.properties#l197

(In reply to Francesco Lodolo [:flod] from comment #4) > Portuguese definitely has peculiar time formats, which makes me think that > Clock could be already failing there It currently works for pt-BR simply because it's ignoring the localized format and using %H:%M or %I:%M https://github.com/mozilla-b2g/gaia/blob/master/apps/clock/js/utils.js#L229 So it will currently break only if AM/PM is changed to something else. So these are the current problems: * Utils.getLocaleTime should use the real localized format to display time * Utils.parseTime should not use ":" or "AM"/"PM" to split values

This patch gets rid of all of the AM-PM-specific logic, deferring to l10n's `shortTimeFormat` in all cases. To address the needs of HTML display showing the `%p` in a smaller format, it modifies the formatString to directly add the HTML tag before formatting.

Comment on attachment 8407129 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18342 Sorry but I'm very far from being a reviewer for code :-) From a quick test, it doesn't seem to work correctly for zh-TW, where the period should be displayed before the time.

Comment on attachment 8407129 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18342 After looking at the code I couldn't understand why it was failing, in fact it works fine. Not sure why my first test failed, probably the patch didn't apply at all.

Comment on attachment 8407129 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18342 Ah, glad you figured it out, I was going to be stumped :-)

Comment on attachment 8407129 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18342 few nits on GH, but overall the patch looks good to me. reduced complexity and also removed some unused code in the process, which is always nice.

master: https://github.com/mozilla-b2g/gaia/commit/ad9a61c0043f344a6f1ccbb05588caf0cd478228