Summary: | [GTK] Text input doesn't work consistently on PPC | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alp Toker <alp> | ||||||||
Component: | WebKitGTK | Assignee: | 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
Alp Toker
2007-12-15 12:35:11 PST
Created attachment 17916 [details]
Fix
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.
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 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!
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.
Oh btw, one more thing. Does it still make sense to keep the function inline? 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 :) |