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

ReadDateTimePattern() on Windows uses wrong connector

RESOLVED DUPLICATE of bug 1355308

Status

()

Core
Internationalization
P3
normal
RESOLVED DUPLICATE of bug 1355308
6 months ago
6 months ago

People

(Reporter: Jorg K (GMT+2), Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

6 months ago
+++ This bug was initially created as a clone of Bug #1351427 +++

See bug 1351427 comment #27.

When en-US is passed to ReadDateTimePattern() on Windows, it ends up calling GetDateTimeConnectorPattern() with an empty string and gets |{1} {0}| returned instead of |{1}, {0}| which is the correct connector for en-US.

I believe there's something wrong with the way the locale is converted to a Windows locale:
  WCHAR buffer[LOCALE_NAME_MAX_LENGTH];
  LPWSTR localeName = GetWindowsLocaleFor(aLocale, buffer);
  GetDateTimeConnectorPattern(NS_ConvertUTF16toUTF8(localeName), aRetVal)
Flags: needinfo?(gandalf)
(Reporter)

Comment 1

6 months ago
I've been looking at this code:
http://searchfox.org/mozilla-central/rev/624d25b2980cf5f83452b040e6d664460f9b7ec5/intl/locale/windows/OSPreferences_win.cpp#75
GetWindowsLocaleFor().

The locale passed in is the app locale, so en-US in my case. Then you retrieve the system locale, en-GB in my case, which is the wrong locale to use, see bug 1351427 comment #67.

If the languages match, which they would, you return LOCALE_NAME_USER_DEFAULT, which according to
https://msdn.microsoft.com/en-us/library/windows/desktop/dd373815(v=vs.85).aspx
is: Name of the current user locale, matching the preference set in the regional and language options portion of Control Panel.

I'm not even sure that LOCALE_NAME_USER_DEFAULT contains a proper usable string. In any case, you retrieve the connector based on that locale.

I think this logic is wrong. You compare the app locale to the system locale, if the languages match, you use the user/format locale.

Why all those hoops? Why not use the connector based on 'aLocale' as passed in to ReadDateTimePattern()?
> Why all those hoops? Why not use the connector based on 'aLocale' as passed in to ReadDateTimePattern()?

Because if your UI locale is "en-GB", and your system locale is "en-US" we wanted to get the connector pattern for the locale we use to format date/time, not the UI locale.

I think that the problem is that Windows does a shortcut. Instead of assigning LOCALE_NAME_USER_DEFAULT a value of the locale, it just keeps it empty.

Then, when you call GetLocaleInfoEx(LOCALE_NAME_USER_DEFAULT); it just passes empty string as the first param and the call interprets it as "use the current locale".
That works for GetLocaleInfoEx, but doesn't work for retrieving the connector (which we have to retrieve from ICU because Windows doesn't provide it).

I think the solution here is to retrieve the locale used for date/time formatting as we're discussing in bug 1351427 comment 68
Flags: needinfo?(gandalf)
(Reporter)

Comment 3

6 months ago
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #2)
> > Why all those hoops? Why not use the connector based on 'aLocale' as passed in to ReadDateTimePattern()?
> Because if your UI locale is "en-GB", and your system locale is "en-US" we
> wanted to get the connector pattern for the locale we use to format
> date/time, not the UI locale.
As 3 AM is approaching, this gets more confusing.
Let's stay with my example: App locale en-US, always for developers. Windows system locale irrelevant.
Windows user/format locale: en-GB. OK the languages match. Now you want to use the connector of en-GB and not the one of en-US. Well, then my code in bug 1351427 is wrong since it retrieves the connector way up in the format routine and that uses the app locale. Too late now to think through it more.

> I think that the problem is that Windows does a shortcut. ...
Yes.
> Too late now to think through it more.

Hahaha, ok. I suggest we first land the big patches that move us to OSPreferences' backed dateformatting to fix the visible regressions and we discuss the connector later since it's a much less visible :)

But long story short - I believe the connector should use the same locale as the date/time formatters.
(Reporter)

Comment 5

6 months ago
Will be fixed by bug 1355308.

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #4)
> But long story short - I believe the connector should use the same locale as
> the date/time formatters.
Agreed.
Status: NEW → RESOLVED
Last Resolved: 6 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1355308
You need to log in before you can comment on or make changes to this bug.