Bug 16454 - [GTK] Text input doesn't work consistently on PPC
: [GTK] Text input doesn't work consistently on PPC
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: WebKit Gtk
: 528+ (Nightly build)
: Macintosh PowerPC All
: P2 Normal
Assigned To: Nobody
: Gtk
Depends on:
Blocks: 14120
  Show dependency treegraph
 
Reported: 2007-12-15 12:35 PST by Alp Toker
Modified: 2007-12-16 05:21 PST (History)
1 user (show)

See Also:


Attachments
Fix (1.35 KB, patch)
2007-12-15 12:52 PST, Alp Toker
no flags Details | Formatted Diff | Diff
0001-Fix-keyval-text-translation-for-key-events-on-big-e.patch (1.88 KB, patch)
2007-12-15 16:22 PST, Xan Lopez
ap: review+
Details | Formatted Diff | Diff
0001-Fix-keyval-text-translation-for-key-events-on-big-e.patch (1.91 KB, patch)
2007-12-16 04:10 PST, Xan Lopez
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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).