Commit 23f0bf15 authored by Nick Diego Yamane's avatar Nick Diego Yamane Committed by Commit Bot

x11, ime: Fix keys decoding when GTK_IM_MODULE=xim

As of crrev.com/c/1789603, GdkEventKey::state is not being set correctly when
translating ui::KeyEvent into GdkEvents. Such field should be equivalent to
XKeyEvent::state, which contains, besides modifiers state bits, keyboard group
information. This causes regressions such as crbug.com/1021732, oddly
specifically when GTK_IM_MODULE env variable is set to 'xim'.

This CL fixes it by including 'group' value into GdkEventKey::state composed
value, as was already done in GtkEventLoopX11.

Bug: 1021732
Change-Id: I2d166ab9f88fc46fd5150f0f2d224aa85a22f9a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1902438Reviewed-by: default avatarThomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Nick Yamane <nickdiego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#713976}
parent bfda1a73
...@@ -11,30 +11,12 @@ ...@@ -11,30 +11,12 @@
#include <gtk/gtk.h> #include <gtk/gtk.h>
#include "base/memory/singleton.h" #include "base/memory/singleton.h"
#include "chrome/browser/ui/libgtkui/gtk_util.h"
#include "ui/events/platform/x11/x11_event_source.h" #include "ui/events/platform/x11/x11_event_source.h"
#include "ui/gfx/x/x11_types.h" #include "ui/gfx/x/x11_types.h"
namespace libgtkui { namespace libgtkui {
namespace {
// Xkb Events store group attribute into XKeyEvent::state bit field, along with
// other state-related info, while GdkEventKey objects have separate fields for
// that purpose, they are ::state and ::group. This function is responsible for
// recomposing them into a single bit field value when translating GdkEventKey
// into XKeyEvent. This is similar to XkbBuildCoreState(), but assumes state is
// an uint rather than an uchar.
//
// More details:
// https://gitlab.freedesktop.org/xorg/proto/xorgproto/blob/master/include/X11/extensions/XKB.h#L372
int BuildXkbStateFromGdkEvent(const GdkEventKey& keyev) {
DCHECK_EQ(0u, XkbGroupForCoreState(keyev.state));
DCHECK(XkbIsLegalGroup(keyev.group));
return keyev.state | ((keyev.group & 0x3) << 13);
}
} // namespace
// static // static
GtkEventLoopX11* GtkEventLoopX11::GetInstance() { GtkEventLoopX11* GtkEventLoopX11::GetInstance() {
return base::Singleton<GtkEventLoopX11>::get(); return base::Singleton<GtkEventLoopX11>::get();
...@@ -88,9 +70,10 @@ void GtkEventLoopX11::ProcessGdkEventKey(const GdkEventKey& gdk_event_key) { ...@@ -88,9 +70,10 @@ void GtkEventLoopX11::ProcessGdkEventKey(const GdkEventKey& gdk_event_key) {
x_event.xkey.window = GDK_WINDOW_XID(gdk_event_key.window); x_event.xkey.window = GDK_WINDOW_XID(gdk_event_key.window);
x_event.xkey.root = DefaultRootWindow(x_event.xkey.display); x_event.xkey.root = DefaultRootWindow(x_event.xkey.display);
x_event.xkey.time = gdk_event_key.time; x_event.xkey.time = gdk_event_key.time;
x_event.xkey.state = BuildXkbStateFromGdkEvent(gdk_event_key);
x_event.xkey.keycode = gdk_event_key.hardware_keycode; x_event.xkey.keycode = gdk_event_key.hardware_keycode;
x_event.xkey.same_screen = true; x_event.xkey.same_screen = true;
x_event.xkey.state =
BuildXkbStateFromGdkEvent(gdk_event_key.state, gdk_event_key.group);
// We want to process the gtk event; mapped to an X11 event immediately // We want to process the gtk event; mapped to an X11 event immediately
// otherwise if we put it back on the queue we may get items out of order. // otherwise if we put it back on the queue we may get items out of order.
......
...@@ -607,6 +607,11 @@ GdkDisplay* GetGdkDisplay() { ...@@ -607,6 +607,11 @@ GdkDisplay* GetGdkDisplay() {
return display; return display;
} }
int BuildXkbStateFromGdkEvent(unsigned int state, unsigned char group) {
DCHECK_EQ(0u, ((state >> 13) & 0x3));
return state | ((group & 0x3) << 13);
}
GdkEvent* GdkEventFromKeyEvent(const ui::KeyEvent& key_event) { GdkEvent* GdkEventFromKeyEvent(const ui::KeyEvent& key_event) {
GdkEventType event_type = GdkEventType event_type =
key_event.type() == ui::ET_KEY_PRESSED ? GDK_KEY_PRESS : GDK_KEY_RELEASE; key_event.type() == ui::ET_KEY_PRESSED ? GDK_KEY_PRESS : GDK_KEY_RELEASE;
...@@ -632,7 +637,7 @@ GdkEvent* GdkEventFromKeyEvent(const ui::KeyEvent& key_event) { ...@@ -632,7 +637,7 @@ GdkEvent* GdkEventFromKeyEvent(const ui::KeyEvent& key_event) {
gdk_event->key.time = event_time.InMilliseconds(); gdk_event->key.time = event_time.InMilliseconds();
gdk_event->key.hardware_keycode = hw_code; gdk_event->key.hardware_keycode = hw_code;
gdk_event->key.keyval = keyval; gdk_event->key.keyval = keyval;
gdk_event->key.state = state; gdk_event->key.state = BuildXkbStateFromGdkEvent(state, group);
gdk_event->key.group = group; gdk_event->key.group = group;
gdk_event->key.send_event = key_event.flags() & ui::EF_FINAL; gdk_event->key.send_event = key_event.flags() & ui::EF_FINAL;
gdk_event->key.is_modifier = state & GDK_MODIFIER_MASK; gdk_event->key.is_modifier = state & GDK_MODIFIER_MASK;
......
...@@ -177,6 +177,17 @@ std::string GetGtkSettingsStringProperty(GtkSettings* settings, ...@@ -177,6 +177,17 @@ std::string GetGtkSettingsStringProperty(GtkSettings* settings,
// Get current GdkDisplay instance // Get current GdkDisplay instance
GdkDisplay* GetGdkDisplay(); GdkDisplay* GetGdkDisplay();
// Xkb Events store group attribute into XKeyEvent::state bit field, along with
// other state-related info, while GdkEventKey objects have separate fields for
// that purpose, they are ::state and ::group. This function is responsible for
// recomposing them into a single bit field value when translating GdkEventKey
// into XKeyEvent. This is similar to XkbBuildCoreState(), but assumes state is
// an uint rather than an uchar.
//
// More details:
// https://gitlab.freedesktop.org/xorg/proto/xorgproto/blob/master/include/X11/extensions/XKB.h#L372
int BuildXkbStateFromGdkEvent(unsigned int state, unsigned char group);
// Translates |key_event| into a GdkEvent. GdkEvent::key::window is the only // Translates |key_event| into a GdkEvent. GdkEvent::key::window is the only
// field not set by this function, callers must set it, as the way for // field not set by this function, callers must set it, as the way for
// retrieving it may vary depending on the event being processed. E.g: for IME // retrieving it may vary depending on the event being processed. E.g: for IME
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment