Bug 16454 – [GTK] Text input doesn't work consistently on PPC

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Alp Toker]

Big endian systems have trouble with keyboard entry in forms etc.

[Alp Toker]

Created attachment 17916 [details]
Fix

[Alp Toker]

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.

[Xan Lopez]

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(-)

[Alexey Proskuryakov]

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!

[Xan Lopez]

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.

[Xan Lopez]

Oh btw, one more thing. Does it still make sense to keep the function inline?

[Alexey Proskuryakov]

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 :)

[Alp Toker]

Landed in r28769 (with tab removed from the ChangeLog and function de-inlined).