bug#10299: Emacs doesn't handle Unicode characters in keyboard layout on MS Windows

[Joakim Hårsman]

Sorry for the late reply, I've been away on vacation.

The change to SetWindowTextW for frame titles was needed because
otherwise the frame title was cut short after the first character
(presumably because the window class name is a UCS-2 wide string and
the second byte therefore is null). This is exaclty the case they talk
about at your link.

If I change w32_set frame text to always call SetWindowText I get a
default frame title of "e". Changing DispatchMessage to use the wide
version when appropriate doesn't help either.

I'm not surprised the WM_CHAR bit twiddling was wrong since the actual
format was so badly documented, your version looks much better.

I'll try your new patch tomorrow and get back with details on how it worked,

On 28 July 2012 16:50, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Tue, 24 Jan 2012 21:42:46 +0100
>> From: Joakim Hårsman <joakim.harsman@gmail.com>
>> Cc: Andreas Schwab <schwab@linux-m68k.org>, 10299@debbugs.gnu.org
>>
>> GetMessage is a macro, many Windows API functions are, and switches
>> between GetMessageA and GetMessageW based on whether UNICODE is
>> defined. I really should explicitly use GetMessageA, I just missed it.
>>
>> I switched to Stefan's suggestion, but with explicit use of
>> GetMessageA. Note that this makes that line longer than 80 chars.
>>
>> Here's an amended patch that also moves the definition of the
>> INIT_WINDOW_CLASS macro to just before use, and makes
>> w32_set_frame_text static.
>
> Thanks.  It's been a while since you submitted this, so sorry for the
> delay.  We are now past Emacs 24.1 release, so we are free to install
> your changes.
>
> However, when I applied the patch, I bumped into several problems.
> First, I don't understand why we need this part:
>
>   +/* Set text of w32 frame with handle HWND to TEXT.
>   +
>   +   We explicitly switch between the Unicode and ANSI version of
>   +   SetWindowText because Emacs isn't compiled with UNICODE defined to
>   +   retain compatibility with Windows 95. */
>   +
>   +static void
>   +w32_set_frame_text (HWND hwnd, LPCSTR text)
>   +{
>   +  if (os_subtype == OS_NT)
>   +    SetWindowTextW (hwnd, (LPCWSTR)text);
>   +  else
>   +    SetWindowTextA (hwnd, text);
>   +}
>
>    /* Change the name of frame F to NAME.  If NAME is nil, set F's name to
>           w32_id_name.
>   @@ -1697,10 +1711,10 @@
>      if (FRAME_W32_WINDOW (f))
>        {
>          if (STRING_MULTIBYTE (name))
>   -       name = ENCODE_SYSTEM (name);
>   -
>   +        name = ENCODE_SYSTEM (name);
>   +
>          BLOCK_INPUT;
>   -      SetWindowText (FRAME_W32_WINDOW (f), SDATA (name));
>   +      w32_set_frame_text(FRAME_W32_WINDOW (f), SDATA (name));
>          UNBLOCK_INPUT;
>        }
>    }
>   @@ -1746,7 +1760,7 @@
>           name = ENCODE_SYSTEM (name);
>
>          BLOCK_INPUT;
>   -      SetWindowText (FRAME_W32_WINDOW (f), SDATA (name));
>   +      w32_set_frame_text(FRAME_W32_WINDOW (f), SDATA (name));
>          UNBLOCK_INPUT;
>        }
>    }
>
> Why do we need to use SetWindowTextW, when all we change is the way we
> request messages for Emacs?  With the patch below, which doesn't use
> this part, I see no problems at all with the titles of Emacs frames.
> Can you tell me how to reproduce the problem with the title?
>
> Moreover, passing 'SDATA (name)' to SetWindowTextW is wrong, since
> that function expects a wchar_t *, not a char *, as pointer to the
> text to display in the title.  (The compiler should warn about that,
> didn't it?)  So if we do need this part, we will need to convert the
> frame name into a wide character string.
>
> Next, this part, whose need was not entirely understood even when you
> submitted the patches:
>
>   @@ -2915,8 +2948,21 @@
>
>        case WM_SYSCHAR:
>        case WM_CHAR:
>   -      post_character_message (hwnd, msg, wParam, lParam,
>   -                             w32_get_key_modifiers (wParam, lParam));
>   +      if (wParam > 255 )
>   +        {
>   +          unsigned short lo = wParam & 0x0000FFFF;
>   +          unsigned short hi = (wParam & 0xFFFF0000) >> 8;
>   +          wParam  = hi | lo;
>   +
>   +          W32Msg wmsg;
>   +          wmsg.dwModifiers = w32_get_key_modifiers (wParam, lParam);
>   +          signal_user_input ();
>   +          my_post_msg (&wmsg, hwnd, WM_UNICHAR, wParam, lParam);
>   +
>   +        }
>   +      else
>   +        post_character_message (hwnd, msg, wParam, lParam,
>   +                                w32_get_key_modifiers (wParam, lParam));
>          break;
>
> This doesn't work in a locale where Unicode codepoints of the
> characters generated by the localized keyboard are above 255.  (I live
> in one such locale.)  In those locales, this snippet produces the
> wrong characters.  The root cause of this is that WM_CHAR's wParam is
> not really the Unicode codepoint of the character I type, it's some
> garbled value.  After looking into this, my conclusion was that your
> patches didn't do everything that's needed to get true Unicode
> character input.  This page:
>
>   
> http://social.msdn.microsoft.com/forums/en-US/windowssdk/thread/07afec87-68c1-4a56-bf46-a38a9c2232e9/
>
> suggests what needs to be done.  So I went ahead and made the few
> additional changes mentioned there, and the Unicode input started
> working for me.  With these additional changes, the funky bit-tweaking
> code is no longer needed, at least for the characters in the 0x05nn
> range of Unicode that I tried.  I hope it will work with your custom
> keyboard as well.
>
> The patches I tried are below.  I'd appreciate if you could try them
> and see if they work with your custom keyboard layouts.  If they do, I
> will install them.
>
> (Juanma, I'd appreciate if you could try this as well.)
>
> Thanks.
>
>
> === modified file 'src/w32fns.c'
> --- src/w32fns.c        2012-07-27 09:24:34 +0000
> +++ src/w32fns.c        2012-07-28 14:28:29 +0000
> @@ -1780,23 +1780,37 @@ w32_load_cursor (LPCTSTR name)
>
>  static LRESULT CALLBACK w32_wnd_proc (HWND, UINT, WPARAM, LPARAM);
>
> +#define INIT_WINDOW_CLASS(WC)                    \
> +  (WC).style = CS_HREDRAW | CS_VREDRAW;                  \
> +  (WC).lpfnWndProc = (WNDPROC) w32_wnd_proc;      \
> +  (WC).cbClsExtra = 0;                            \
> +  (WC).cbWndExtra = WND_EXTRA_BYTES;              \
> +  (WC).hInstance = hinst;                         \
> +  (WC).hIcon = LoadIcon (hinst, EMACS_CLASS);     \
> +  (WC).hCursor = w32_load_cursor (IDC_ARROW);     \
> +  (WC).hbrBackground = NULL;                      \
> +  (WC).lpszMenuName = NULL;                       \
> +
>  static BOOL
>  w32_init_class (HINSTANCE hinst)
>  {
> -  WNDCLASS wc;
>
> -  wc.style = CS_HREDRAW | CS_VREDRAW;
> -  wc.lpfnWndProc = (WNDPROC) w32_wnd_proc;
> -  wc.cbClsExtra = 0;
> -  wc.cbWndExtra = WND_EXTRA_BYTES;
> -  wc.hInstance = hinst;
> -  wc.hIcon = LoadIcon (hinst, EMACS_CLASS);
> -  wc.hCursor = w32_load_cursor (IDC_ARROW);
> -  wc.hbrBackground = NULL; /* GetStockObject (WHITE_BRUSH);  */
> -  wc.lpszMenuName = NULL;
> -  wc.lpszClassName = EMACS_CLASS;
> +  if (os_subtype == OS_NT)
> +    {
> +      WNDCLASSW  uwc;
> +      INIT_WINDOW_CLASS(uwc);
> +      uwc.lpszClassName = L"Emacs";
> +
> +      return RegisterClassW (&uwc);
> +    }
> +  else
> +    {
> +      WNDCLASS  wc;
> +      INIT_WINDOW_CLASS(wc);
> +      wc.lpszClassName = EMACS_CLASS;
>
> -  return (RegisterClass (&wc));
> +      return RegisterClassA (&wc);
> +    }
>  }
>
>  static HWND
> @@ -2246,7 +2260,7 @@ w32_msg_pump (deferred_msg * msg_buf)
>
>    msh_mousewheel = RegisterWindowMessage (MSH_MOUSEWHEEL);
>
> -  while (GetMessage (&msg, NULL, 0, 0))
> +  while ((os_subtype == OS_NT ? GetMessageW : GetMessageA) (&msg, NULL, 0, 
> 0))
>      {
>        if (msg.hwnd == NULL)
>         {
> @@ -2341,7 +2355,10 @@ w32_msg_pump (deferred_msg * msg_buf)
>         }
>        else
>         {
> -         DispatchMessage (&msg);
> +         if (os_subtype == OS_NT)
> +           DispatchMessageW (&msg);
> +         else
> +           DispatchMessageA (&msg);
>         }
>
>        /* Exit nested loop when our deferred message has completed.  */
> @@ -2918,8 +2935,18 @@ w32_wnd_proc (HWND hwnd, UINT msg, WPARA
>
>      case WM_SYSCHAR:
>      case WM_CHAR:
> -      post_character_message (hwnd, msg, wParam, lParam,
> -                             w32_get_key_modifiers (wParam, lParam));
> +      if (wParam > 255 )
> +        {
> +          W32Msg wmsg;
> +
> +          wmsg.dwModifiers = w32_get_key_modifiers (wParam, lParam);
> +          signal_user_input ();
> +          my_post_msg (&wmsg, hwnd, WM_UNICHAR, wParam, lParam);
> +
> +        }
> +      else
> +        post_character_message (hwnd, msg, wParam, lParam,
> +                                w32_get_key_modifiers (wParam, lParam));
>        break;
>
>      case WM_UNICHAR:
> @@ -3801,7 +3828,7 @@ w32_wnd_proc (HWND hwnd, UINT msg, WPARA
>         }
>
>      dflt:
> -      return DefWindowProc (hwnd, msg, wParam, lParam);
> +      return (os_subtype == OS_NT ? DefWindowProcW :  DefWindowProcA) (hwnd, 
> msg, wParam, lParam);
>      }
>
>    /* The most common default return code for handled messages is 0.  */
>