After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 404832 - g_date_strftime should use wcsftime if available
g_date_strftime should use wcsftime if available
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other Windows
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2007-02-06 01:51 UTC by Andreas Köhler
Modified: 2007-02-17 08:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
test case (542 bytes, text/plain)
2007-02-06 01:52 UTC, Andreas Köhler
  Details
encodings test case (317 bytes, text/plain)
2007-02-06 20:17 UTC, Andreas Köhler
  Details
Suggested patch (13.71 KB, patch)
2007-02-07 02:34 UTC, Tor Lillqvist
committed Details | Review
Corresponding patch for glib-2-12 (13.24 KB, patch)
2007-02-07 03:07 UTC, Tor Lillqvist
committed Details | Review

Description Andreas Köhler 2007-02-06 01:51:12 UTC
See attached test case.
Compiled with mingw and without any special parameters.  Set "Standard and Formats" to "French" and "Language for non-Unicode applications"(or so) to "Russian" or the other way around.  Other combinations may fail as well.

On Windows, strftime with %B will fill the buffer with the full month in the encoding specified for non-Unicode applications which may differ from the one used by g_locale_to_utf8.  This can be avoided if defined(G_WIN32_HAVE_WIDECHAR_API) and wcsftime is used.
Comment 1 Andreas Köhler 2007-02-06 01:52:14 UTC
Created attachment 81970 [details]
test case
Comment 2 Owen Taylor 2007-02-06 14:45:43 UTC
> with the full month in the encoding specified for non-Unicode applications 
> which may differ from the one used by g_locale_to_utf8

The change sounds like a possibly useful one, but I'm suprised by the
above. What are these two encodings? g_locale_to_utf8() really is supposed
to be picking up the "encoding specified for non-Unicode applications"
Comment 3 Andreas Köhler 2007-02-06 20:16:22 UTC
That is right, g_get_charset picks the encoding specified for non-Unicode applications.  But that may differ from the locale encoding... Maybe another bug?

See the attachment for an encoding test case.  E.g. it might print
setlocale: Russian_Russia.1251
g_get_charset: CP1252
Comment 4 Andreas Köhler 2007-02-06 20:17:00 UTC
Created attachment 82041 [details]
encodings test case
Comment 5 Owen Taylor 2007-02-06 20:36:45 UTC
How does this problem occur without an explicit call to setlocale() with a 
non-"" second argument? If I look at the msdn docs for setlocale() I
see: 

  setlocale( LC_ALL, "" );

    Sets the locale to the default, which is the user-default ANSI code page obtained from the operating system

Internally, g_get_charset() uses GetACP() which gives the "user-default
ANSI code page". You first example doesn't seem to have a non-default
setlocale() call. (gtk_init() calls setlocale() internally, with the
above arguments, IIRC)
Comment 6 Tor Lillqvist 2007-02-06 20:52:02 UTC
I agree that wcsftime() should be used on NT-based Windows.

Whether the situation described here is particularily common I don't know, though. What is the use case where this situation occurs? Is such a setup common among Russian speakers in France, for instance? Any idea what the "Language for non-Unicode applications" setting actually does, API-wise? The "user friendly" wording in the Control Panel applet is less than useful. Presumably this setting sets the system codepage (what GetACP() returns), but not the default locale (what GetThreadLocale() returns), which is affected by the "Standards and Formats" setting?
Comment 7 Andreas Köhler 2007-02-06 21:03:21 UTC
Just two links:
bug 403815
http://codesnipers.com/?q=node/46
Comment 8 Tor Lillqvist 2007-02-06 21:35:35 UTC
Hmm, looking at the source for wcsftime() (for instance, as included with MSVC6, or with the Platform SDK) reveals an interesting and somewhat disappointing thing: wcsftime() converts the format to a multibyte string with wcstombs(), calls strftime(), and convers the result to UTF-16 with mbstowcs(). So using wcsftime() will not get rid of strftime(), it will still be used...

As the C library's handling of locales seems to be somewhat confusing anyway, what should be done is probably then to forget both strftime() and wcstime(). Instead parse the format string ourselves and call the necessary wide character Win32 APIs, like GetLocaleInfo() and GetDateFormat(). But which LCID should be used? The one returned by GetThreadLocale(), presumably. And as we use wide character APIs, codepages fortunately do not get involved at all.
Comment 9 Tor Lillqvist 2007-02-07 02:34:46 UTC
Created attachment 82059 [details] [review]
Suggested patch

Suggested patch to trunk GLib. Unfortunately we can't use this as such in the stable branch, as we (at least pretend to) still support Win9x there. I'll cook up an other patch for the stable branch, where on Win9x the old code is used.
Comment 10 Tor Lillqvist 2007-02-07 03:07:25 UTC
Created attachment 82063 [details] [review]
Corresponding patch for glib-2-12
Comment 11 Andreas Köhler 2007-02-07 18:44:04 UTC
On a related issue, I just filed bug 405469.
Comment 12 Tor Lillqvist 2007-02-08 11:11:01 UTC
So, do you think the approach in these patches will work for you? I.e. using the thread locale, i.e. what the "Standard and Formats" setting affects, for g_date_strftime(), using just wide character Windows APIs and not strftime() or wcsftime() at all?

When somebody chooses a language with a different codepage as system codepage than that used by the default user locale, what is usually the rationale for this? Like in the French locale but Russian codepage example, would this be somebody who wants to manipulate files with russian filenames (with programs that use non-Unicode APIs)?
Comment 13 Tor Lillqvist 2007-02-13 00:14:25 UTC
Hello? Unless somebody tells me why it's a bad idea, I'll commit the patches to the trunk and stable branch.
Comment 14 Andreas Köhler 2007-02-13 08:02:32 UTC
I am sorry, I am a bit busy right now and I wanted to test before I respond.  I guess the approach will work for me, but in a few days I could tell you more definitely.
If you want to commit, go on :-)
Comment 15 Tor Lillqvist 2007-02-17 08:57:42 UTC
Fixed in trunk and glib-2-12.

2007-02-17  Tor Lillqvist  <tml@novell.com>

	* glib/gdate.c (win32_strftime_helper): New Win32-only
	function. Use the wide character Win32 API to do the work of
	strftime(): GetThreadLocale(), GetLocaleInfoW(), GetDateFormatW()
	and GetTimeFormatW().
	(g_date_strftime): On NT-based Windows use win32_strftime_helper()
	instead of strftime() to avoid codepage issues with strftime().
	Unfortunately using wcsftime() would not help either. (#404832)