Bug 15903

Created attachment 16474 [details]
image of xclock that is too narrow

Created attachment 16475 [details] [review]
patch that corrects the problem

The patch does not apply as is to git master.  Could you clone master
and update the patch for it?

Thanks.

git clone git://anongit.freedesktop.org/git/xorg/app/xclock.git

If you only have http access, you can use:

git clone http://anongit.freedesktop.org/git/xorg/app/xclock.git

Heh....the patch doesn't apply because someone already fixed the problem.  The version in git doesn't have the bug.  Someone already added the missing set of braces.  I should have checked that. :-)

Sorry, I'm reopening this. The source was fixed for many cases, but it seems there's still one problem case left even in git HEAD eefa0405.

Clock.c:Initialize() calculates incorrect extents if LC_CTYPE is set to a UTF-8 locale and LC_TIME is not, e.g. LC_CTYPE=en_US.utf8 LC_TIME=C.

--8<-- I added some tracing
diff --git a/Clock.c b/Clock.c
index 8f78cab..79cf919 100644
--- a/Clock.c
+++ b/Clock.c
@@ -591,6 +591,8 @@ Initialize (Widget request, Widget new, ArgList args, Cardinal *num_args)
        w->clock.utf8 = False;
 
+       printf("no_locale=%d\n",no_locale);
        if (!no_locale) {
           char *time_locale = setlocale(LC_TIME, NULL);
+          printf("time_locale='%s'\n",time_locale);
 
           if (strstr(time_locale, "UTF-8") || strstr(time_locale, "utf8")) {
@@ -613,4 +615,5 @@ Initialize (Widget request, Widget new, ArgList args, Cardinal *num_args)
 #endif /* NO_I18N */
 
+       printf("utf8=%d\n",w->clock.utf8);
        (void) time(&time_value);
        tm = *localtime(&time_value);
-->8--

which gets me the following output:

# OK
$ LC_CTYPE=C LC_TIME=C ./xclock 
no_locale=0
time_locale='C'
utf8=0

# also OK
$ LC_CTYPE=en_US.utf8 LC_TIME=en_US.utf8 ./xclock 
no_locale=0
time_locale='en_US.utf8'
utf8=1

# this is OK too
$ LC_CTYPE=C LC_TIME=en_US.utf8 ./xclock 
no_locale=0
time_locale='en_US.utf8'
utf8=1

# but this calculates the wrong size
$ LC_CTYPE=en_US.utf8 LC_TIME=C ./xclock 
no_locale=0
time_locale='C'
utf8=0

The double negations in Clock.c made me instinctively work around the problem by setting LC_CTYPE=C for my xclock invocation, but I guess it would be nice to handle also this case in the code.

I believe the correct solution is to determine character extent calculation method based on LC_CTYPE rather than LC_TIME.

--8<-- http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap07.html
7.3.1 LC_CTYPE
The LC_CTYPE category shall define character classification, case conversion, and other character attributes.

..

7.3.5 LC_TIME
The LC_TIME category shall define the interpretation of the conversion specifications supported by the date utility and shall affect the behavior of the strftime(), wcsftime(), strptime(), XSI and nl_langinfo() functions.
-->8--

I interpret this to mean that LC_TIME determines what specifiers and formats are be output by the various functions, while LC_CTYPE determines encoding.

I'm attaching a patch which works for me, regardless of what I set LC_CTYPE to.

Created attachment 38165 [details] [review]
Patch to calculate text and window size from LC_CTYPE instead of LC_TIME

Review of attachment 38165 [details] [review]:

Looks correct to me.

Reviewed-by: Julien Cristau <jcristau@debian.org>

commit 251461d84de4220f05ffa35a3138aad1cd2e0302
Author: Peter Stuge <peter@stuge.se>
Date:   Thu Aug 26 04:14:27 2010 +0200

    Calculate text and window size from LC_CTYPE instead of LC_TIME
    
    LC_TIME only defines what will be displayed, LC_CTYPE is what defines
    the character encoding.
    
    References:
    
      Bug 15903 - LANG=en_US xclock -digital -font fixed has wrong size
      https://bugs.freedesktop.org/show_bug.cgi?id=15903
    
    Reviewed-by: Julien Cristau <jcristau@debian.org>
    Reviewed-by: Jeremy Huddleston <jeremyhu@apple.com>