Commit e092749a authored by Hidehiko Abe's avatar Hidehiko Abe Committed by Commit Bot

wayland: Fix IME keysym.

Currently WaylandInputMethodContext::OnKeysym interprets the key
as native evdev code. However, according to the protocol spec,
it is XKB keysym.
Fix the interpretation by using XkbKeyboardLayoutEngine
as a keycode converter.

BUG=1123705
TEST=Ran on Chrome OS DUT.

Change-Id: I3d45fd94424a55860872a6625a68963db650aa28
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2437346Reviewed-by: default avatarRobert Kroeger <rjkroege@chromium.org>
Reviewed-by: default avatarJun Mukai <mukai@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813614}
parent 6da6c21d
...@@ -61,6 +61,7 @@ source_set("unittests") { ...@@ -61,6 +61,7 @@ source_set("unittests") {
"keyboard_layout_engine_unittest.cc", "keyboard_layout_engine_unittest.cc",
"xkb/xkb_keyboard_layout_engine_unittest.cc", "xkb/xkb_keyboard_layout_engine_unittest.cc",
] ]
configs += [ ":xkbcommon" ]
} }
deps = [ deps = [
......
...@@ -852,6 +852,30 @@ void XkbKeyboardLayoutEngine::SetKeymap(xkb_keymap* keymap) { ...@@ -852,6 +852,30 @@ void XkbKeyboardLayoutEngine::SetKeymap(xkb_keymap* keymap) {
num_lock_mask = flag; num_lock_mask = flag;
} }
} }
// Reconstruct keysym map.
xkb_keysym_map_.clear();
const xkb_keycode_t min_key = xkb_keymap_min_keycode(keymap);
const xkb_keycode_t max_key = xkb_keymap_max_keycode(keymap);
for (xkb_keycode_t keycode = min_key; keycode <= max_key; ++keycode) {
const xkb_layout_index_t num_layouts =
xkb_keymap_num_layouts_for_key(keymap, keycode);
for (xkb_layout_index_t layout = 0; layout < num_layouts; ++layout) {
const xkb_level_index_t num_levels =
xkb_keymap_num_levels_for_key(keymap, keycode, layout);
for (xkb_level_index_t level = 0; level < num_levels; ++level) {
const xkb_keysym_t* keysyms;
int num_syms = xkb_keymap_key_get_syms_by_level(keymap, keycode, layout,
level, &keysyms);
for (int i = 0; i < num_syms; ++i) {
// Ignore if there already an entry for the current keysym.
// Iterating keycode from min to max, so the minimum value wins.
xkb_keysym_map_.emplace(keysyms[i], keycode);
}
}
}
}
layout_index_ = 0; layout_index_ = 0;
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
// Update num lock mask. // Update num lock mask.
...@@ -893,6 +917,15 @@ int XkbKeyboardLayoutEngine::UpdateModifiers(uint32_t depressed, ...@@ -893,6 +917,15 @@ int XkbKeyboardLayoutEngine::UpdateModifiers(uint32_t depressed,
return ui_flags; return ui_flags;
} }
DomCode XkbKeyboardLayoutEngine::GetDomCodeByKeysym(uint32_t keysym) const {
auto iter = xkb_keysym_map_.find(keysym);
if (iter == xkb_keysym_map_.end()) {
VLOG(1) << "No Keycode found for the keysym: " << keysym;
return DomCode::NONE;
}
return KeycodeConverter::NativeKeycodeToDomCode(iter->second);
}
bool XkbKeyboardLayoutEngine::XkbLookup(xkb_keycode_t xkb_keycode, bool XkbKeyboardLayoutEngine::XkbLookup(xkb_keycode_t xkb_keycode,
xkb_mod_mask_t xkb_flags, xkb_mod_mask_t xkb_flags,
xkb_keysym_t* xkb_keysym, xkb_keysym_t* xkb_keysym,
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include <vector> #include <vector>
#include "base/component_export.h" #include "base/component_export.h"
#include "base/containers/flat_map.h"
#include "base/memory/free_deleter.h" #include "base/memory/free_deleter.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
...@@ -53,6 +54,8 @@ class COMPONENT_EXPORT(EVENTS_OZONE_LAYOUT) XkbKeyboardLayoutEngine ...@@ -53,6 +54,8 @@ class COMPONENT_EXPORT(EVENTS_OZONE_LAYOUT) XkbKeyboardLayoutEngine
uint32_t locked, uint32_t locked,
uint32_t group); uint32_t group);
DomCode GetDomCodeByKeysym(uint32_t keysym) const;
static void ParseLayoutName(const std::string& layout_name, static void ParseLayoutName(const std::string& layout_name,
std::string* layout_id, std::string* layout_id,
std::string* layout_variant); std::string* layout_variant);
...@@ -66,6 +69,12 @@ class COMPONENT_EXPORT(EVENTS_OZONE_LAYOUT) XkbKeyboardLayoutEngine ...@@ -66,6 +69,12 @@ class COMPONENT_EXPORT(EVENTS_OZONE_LAYOUT) XkbKeyboardLayoutEngine
}; };
std::vector<XkbFlagMapEntry> xkb_flag_map_; std::vector<XkbFlagMapEntry> xkb_flag_map_;
// Table from xkb_keysym to xkb_keycode on the current keymap.
// Note that there could be multiple keycodes mapped to the same
// keysym. In the case, the first one (smallest keycode) will be
// kept.
base::flat_map<uint32_t, uint32_t> xkb_keysym_map_;
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
// Flag mask for num lock, which is always considered enabled in ChromeOS. // Flag mask for num lock, which is always considered enabled in ChromeOS.
xkb_mod_mask_t num_lock_mod_mask_ = 0; xkb_mod_mask_t num_lock_mod_mask_ = 0;
......
...@@ -906,4 +906,42 @@ TEST_F(XkbLayoutEngineVkTest, XkbRuleNamesForLayoutName) { ...@@ -906,4 +906,42 @@ TEST_F(XkbLayoutEngineVkTest, XkbRuleNamesForLayoutName) {
} }
} }
TEST_F(XkbLayoutEngineVkTest, GetDomCodeByKeysym) {
// Set up US keyboard layout.
{
std::unique_ptr<xkb_context, ui::XkbContextDeleter> xkb_context(
xkb_context_new(XKB_CONTEXT_NO_FLAGS));
xkb_rule_names names = {
.rules = nullptr,
.model = "pc101",
.layout = "us",
.variant = "",
.options = "",
};
std::unique_ptr<xkb_keymap, ui::XkbKeymapDeleter> xkb_keymap(
xkb_keymap_new_from_names(xkb_context.get(), &names,
XKB_KEYMAP_COMPILE_NO_FLAGS));
std::unique_ptr<char, base::FreeDeleter> layout(
xkb_keymap_get_as_string(xkb_keymap.get(), XKB_KEYMAP_FORMAT_TEXT_V1));
layout_engine_->SetCurrentLayoutFromBuffer(layout.get(),
std::strlen(layout.get()));
}
constexpr struct {
uint32_t keysym;
DomCode dom_code;
} kTestCases[] = {
{65307, ui::DomCode::ESCAPE}, {65288, ui::DomCode::BACKSPACE},
{65293, ui::DomCode::ENTER}, {65289, ui::DomCode::TAB},
{65056, ui::DomCode::TAB}, // SHIFT+TAB.
{65289, ui::DomCode::TAB}, // CTRL+TAB.
};
for (const auto& test_case : kTestCases) {
SCOPED_TRACE(test_case.keysym);
EXPECT_EQ(test_case.dom_code,
layout_engine_->GetDomCodeByKeysym(test_case.keysym));
}
}
} // namespace ui } // namespace ui
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/command_line.h" #include "base/command_line.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/notreached.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "ui/base/ime/composition_text.h" #include "ui/base/ime/composition_text.h"
...@@ -22,6 +23,10 @@ ...@@ -22,6 +23,10 @@
#include "ui/ozone/platform/wayland/host/zwp_text_input_wrapper_v1.h" #include "ui/ozone/platform/wayland/host/zwp_text_input_wrapper_v1.h"
#include "ui/ozone/public/ozone_switches.h" #include "ui/ozone/public/ozone_switches.h"
#if BUILDFLAG(USE_XKBCOMMON)
#include "ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.h"
#endif
namespace ui { namespace ui {
WaylandInputMethodContext::WaylandInputMethodContext( WaylandInputMethodContext::WaylandInputMethodContext(
...@@ -49,7 +54,6 @@ void WaylandInputMethodContext::Init(bool initialize_for_testing) { ...@@ -49,7 +54,6 @@ void WaylandInputMethodContext::Init(bool initialize_for_testing) {
initialize_for_testing || initialize_for_testing ||
base::CommandLine::ForCurrentProcess()->HasSwitch( base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableWaylandIme); switches::kEnableWaylandIme);
// If text input instance is not created then all ime context operations // If text input instance is not created then all ime context operations
// are noop. This option is because in some environments someone might not // are noop. This option is because in some environments someone might not
// want to enable ime/virtual keyboard even if it's available. // want to enable ime/virtual keyboard even if it's available.
...@@ -148,19 +152,23 @@ void WaylandInputMethodContext::OnDeleteSurroundingText(int32_t index, ...@@ -148,19 +152,23 @@ void WaylandInputMethodContext::OnDeleteSurroundingText(int32_t index,
ime_delegate_->OnDeleteSurroundingText(index, length); ime_delegate_->OnDeleteSurroundingText(index, length);
} }
void WaylandInputMethodContext::OnKeysym(uint32_t key, void WaylandInputMethodContext::OnKeysym(uint32_t keysym,
uint32_t state, uint32_t state,
uint32_t modifiers) { uint32_t modifiers) {
#if BUILDFLAG(USE_XKBCOMMON)
// TODO(crbug.com/1079353): Handle modifiers.
DomCode dom_code = DomCode dom_code =
KeycodeConverter::NativeKeycodeToDomCode(EvdevCodeToNativeCode(key)); connection_->keyboard()->layout_engine()->GetDomCodeByKeysym(keysym);
if (dom_code == ui::DomCode::NONE) if (dom_code == DomCode::NONE)
return; return;
// TODO(crbug.com/1079353): Handle modifiers.
EventType type = EventType type =
state == WL_KEYBOARD_KEY_STATE_PRESSED ? ET_KEY_PRESSED : ET_KEY_RELEASED; state == WL_KEYBOARD_KEY_STATE_PRESSED ? ET_KEY_PRESSED : ET_KEY_RELEASED;
key_delegate_->OnKeyboardKeyEvent(type, dom_code, /*repeat=*/false, key_delegate_->OnKeyboardKeyEvent(type, dom_code, /*repeat=*/false,
EventTimeForNow()); EventTimeForNow());
#else
NOTIMPLEMENTED();
#endif
} }
} // namespace ui } // namespace ui
...@@ -45,7 +45,7 @@ class WaylandInputMethodContext : public LinuxInputMethodContext, ...@@ -45,7 +45,7 @@ class WaylandInputMethodContext : public LinuxInputMethodContext,
void OnPreeditString(const std::string& text, int preedit_cursor) override; void OnPreeditString(const std::string& text, int preedit_cursor) override;
void OnCommitString(const std::string& text) override; void OnCommitString(const std::string& text) override;
void OnDeleteSurroundingText(int32_t index, uint32_t length) override; void OnDeleteSurroundingText(int32_t index, uint32_t length) override;
void OnKeysym(uint32_t key, uint32_t state, uint32_t modifiers) override; void OnKeysym(uint32_t keysym, uint32_t state, uint32_t modifiers) override;
private: private:
void UpdatePreeditText(const base::string16& preedit_text); void UpdatePreeditText(const base::string16& preedit_text);
......
...@@ -45,11 +45,7 @@ WaylandKeyboard::WaylandKeyboard( ...@@ -45,11 +45,7 @@ WaylandKeyboard::WaylandKeyboard(
connection_(connection), connection_(connection),
delegate_(delegate), delegate_(delegate),
auto_repeat_handler_(this), auto_repeat_handler_(this),
#if BUILDFLAG(USE_XKBCOMMON) layout_engine_(static_cast<LayoutEngine*>(layout_engine)) {
layout_engine_(static_cast<XkbKeyboardLayoutEngine*>(layout_engine)) {
#else
layout_engine_(layout_engine) {
#endif
static const wl_keyboard_listener listener = { static const wl_keyboard_listener listener = {
&WaylandKeyboard::Keymap, &WaylandKeyboard::Enter, &WaylandKeyboard::Keymap, &WaylandKeyboard::Enter,
&WaylandKeyboard::Leave, &WaylandKeyboard::Key, &WaylandKeyboard::Leave, &WaylandKeyboard::Key,
......
...@@ -29,6 +29,14 @@ class WaylandKeyboard : public EventAutoRepeatHandler::Delegate { ...@@ -29,6 +29,14 @@ class WaylandKeyboard : public EventAutoRepeatHandler::Delegate {
public: public:
class Delegate; class Delegate;
using LayoutEngine =
#if BUILDFLAG(USE_XKBCOMMON)
XkbKeyboardLayoutEngine
#else
KeyboardLayoutEngine
#endif
;
WaylandKeyboard(wl_keyboard* keyboard, WaylandKeyboard(wl_keyboard* keyboard,
zcr_keyboard_extension_v1* keyboard_extension_v1, zcr_keyboard_extension_v1* keyboard_extension_v1,
WaylandConnection* connection, WaylandConnection* connection,
...@@ -41,6 +49,7 @@ class WaylandKeyboard : public EventAutoRepeatHandler::Delegate { ...@@ -41,6 +49,7 @@ class WaylandKeyboard : public EventAutoRepeatHandler::Delegate {
int modifiers, int modifiers,
DomKey* out_dom_key, DomKey* out_dom_key,
KeyboardCode* out_key_code); KeyboardCode* out_key_code);
LayoutEngine* layout_engine() const { return layout_engine_; }
private: private:
// wl_keyboard_listener // wl_keyboard_listener
...@@ -99,11 +108,7 @@ class WaylandKeyboard : public EventAutoRepeatHandler::Delegate { ...@@ -99,11 +108,7 @@ class WaylandKeyboard : public EventAutoRepeatHandler::Delegate {
base::OnceClosure auto_repeat_closure_; base::OnceClosure auto_repeat_closure_;
wl::Object<wl_callback> sync_callback_; wl::Object<wl_callback> sync_callback_;
#if BUILDFLAG(USE_XKBCOMMON) LayoutEngine* layout_engine_;
XkbKeyboardLayoutEngine* layout_engine_;
#else
KeyboardLayoutEngine* layout_engine_;
#endif
}; };
class WaylandKeyboard::Delegate { class WaylandKeyboard::Delegate {
......
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