Bug 16454

Summary: [GTK] Text input doesn't work consistently on PPC
Product: WebKit Reporter: Alp Toker <alp>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: Mac (PowerPC)   
OS: All   
Bug Depends on:    
Bug Blocks: 14120    
Attachments:
Description Flags
Fix
none
0001-Fix-keyval-text-translation-for-key-events-on-big-e.patch
ap: review+
0001-Fix-keyval-text-translation-for-key-events-on-big-e.patch ap: review+

Description Alp Toker 2007-12-15 12:35:11 PST
Big endian systems have trouble with keyboard entry in forms etc.
Comment 1 Alp Toker 2007-12-15 12:52:09 PST
Created attachment 17916 [details]
Fix
Comment 2 Alp Toker 2007-12-15 13:09:04 PST
Comment on attachment 17916 [details]
Fix

Given the implementation does this:
    memcpy(m_data, str, len * sizeof(UChar));

This patch doesn't look right. Needs further investigation.
Comment 3 Xan Lopez 2007-12-15 16:22:19 PST
Created attachment 17918 [details]
0001-Fix-keyval-text-translation-for-key-events-on-big-e.patch

Subject: [PATCH] Fix keyval->text translation for key events on big endian systems.

UChar is UTF-16, not UCS-4, so transform accordingly.
---
 WebCore/ChangeLog                    |   15 +++++++++++++++
 WebCore/platform/gtk/KeyEventGtk.cpp |   15 +++++++++++++--
 2 files changed, 28 insertions(+), 2 deletions(-)
Comment 4 Alexey Proskuryakov 2007-12-16 00:58:09 PST
Comment on attachment 17918 [details]
0001-Fix-keyval-text-translation-for-key-events-on-big-e.patch

Per gnome documentation, g_ucs4_to_utf16 signals error by returning NULL - theoretically, nwc may remain uninitialized in this case, so you shouldn't compare it to 1.

More importantly, I do not see any reason to truncate the input to "single character" - probably, the function should be renamed, and then you can just handle any length (where any is 1 or 2 :) ). Or you can keep the name, and say that two UTF-16 surrogates are still a single Unicode character.

This is a good fix as is, so r=me, but please consider making it even better!
Comment 5 Xan Lopez 2007-12-16 04:10:37 PST
Created attachment 17929 [details]
0001-Fix-keyval-text-translation-for-key-events-on-big-e.patch

Very good points, thanks!

I'm checking for uchar16 != NULL and passing nwc to the String constructor now.
Comment 6 Xan Lopez 2007-12-16 04:13:14 PST
Oh btw, one more thing. Does it still make sense to keep the function inline?
Comment 7 Alexey Proskuryakov 2007-12-16 04:27:51 PST
Comment on attachment 17929 [details]
0001-Fix-keyval-text-translation-for-key-events-on-big-e.patch

r=me

> Does it still make sense to keep the function inline?

Probably not :)
Comment 8 Alp Toker 2007-12-16 05:21:27 PST
Landed in r28769 (with tab removed from the ChangeLog and function de-inlined).