If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Date.toLocaleFormat is corrupted after landing bug 1346674

RESOLVED FIXED in Firefox 55

Status

()

Core
JavaScript: Internationalization API
RESOLVED FIXED
6 months ago
6 months ago

People

(Reporter: m_kato, Assigned: gandalf)

Tracking

({regression})

55 Branch
mozilla55
Unspecified
Windows
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 unaffected, firefox-esr52 unaffected, firefox53 unaffected, firefox54 unaffected, firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

6 months ago
- Env
Windows 10 with Japanese locale setting

- Step
Run the following on Scratchpad.

var d = new Date();
d.toLocaleFormat();

- Result
/*
2017”N3ŒŽ22“ú 16:53:14
*/

- Expected Result
/*
2017年3月22日 16:53:14
*/
(Reporter)

Updated

6 months ago
Blocks: 1346674
No longer blocks: 1349454
(Reporter)

Updated

6 months ago
Summary: Date.toLocaleFormat is corrupted after landing bug 1349454 → Date.toLocaleFormat is corrupted after landing bug 1346674
(Assignee)

Comment 1

6 months ago
:pike - Similar question to bug 1349454, but about dates instead of fonts.

With ICU and Intl API we use browser locale to localize the date/time. Should `toLocaleFormat` follow OS locale or browser locale?
Flags: needinfo?(l10n)

Comment 2

6 months ago
I think we're getting a japanese date format, but get the encoding wrong here.

No idea where that happens.
Flags: needinfo?(l10n)
(Assignee)

Comment 3

6 months ago
The encoding is wrong because it asks for:
http://searchfox.org/mozilla-central/rev/0079c7adf3b329bff579d3bbe6ac7ba2f6218a19/js/xpconnect/src/XPCLocale.cpp#162

which if (fortunately) only used by:
http://searchfox.org/mozilla-central/rev/0079c7adf3b329bff579d3bbe6ac7ba2f6218a19/js/src/jsdate.cpp#2705

so it'll all go away when we get rid of toLocaleFormat, but in the meantime we have a situation where the whole JS context uses app's locale, except of this one function which takes the OS locale.

Ok, I'll revert it, because we only use toLocaleFormat in Android now, which is a non-ICU scenario, so we have no other place but OS to take the datetime tokens from. :(
Comment hidden (mozreview-request)
(Reporter)

Comment 5

6 months ago
mozreview-review
Comment on attachment 8850068 [details]
Bug 1349470 - Use OS locale for unicode conversions for Date.toLocaleFormat.

https://reviewboard.mozilla.org/r/122788/#review125186

::: js/xpconnect/src/XPCLocale.cpp:177
(Diff revision 1)
> -      NS_ConvertUTF8toUTF16 localeStr(appLocale);
> +      // access to any other date/time than the OS one, so we have to also
> +      // use the OS locale for unicode conversions.
> +      // See bug 1349470 for more details.
> +      nsAutoCString osLocale;
> +      OSPreferences::GetInstance()->GetSystemLocale(osLocale);
> +      NS_ConvertUTF8toUTF16 localeStr(osLocale);

Since we don't use NS_ILOCALE_DATE, we can use NS_CopyNativeToUnicode simply instead of LocaleService, nsIPlatformCharset and EncodingUtils.

    nsAutoString result;
    NS_CopyNativeToUnicode(nsDependentCString(src), result);
    JSString* ucstr =
      JS_NewUCStringCopyN(cx, result.get(), result.Length());
    if (ucstr) {
      rval.setString(ucstr);
      return true;
    }
    
But it should be handled by another bug, so it is OK for this bug by your fix.
Attachment #8850068 - Flags: review?(m_kato) → review+

Comment 6

6 months ago
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1038d1c502be
Use OS locale for unicode conversions for Date.toLocaleFormat. r=m_kato
(Reporter)

Updated

6 months ago
Blocks: 943283
(Assignee)

Updated

6 months ago
Assignee: nobody → gandalf
Status: NEW → ASSIGNED

Comment 7

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1038d1c502be
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
status-firefox52: --- → unaffected
status-firefox53: --- → unaffected
status-firefox54: --- → unaffected
status-firefox-esr52: --- → unaffected
You need to log in before you can comment on or make changes to this bug.