github/electron
1
1
Fork 0
mirror of https://github.com/electron/electron.git synced 2026-04-10 03:01:51 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix: remove use of xkb_keymap_key_get_mods_for_level (#34155)

Browse Source
This commit is contained in:
Cheng Zhao
2022-05-12 03:48:29 +09:00
committed by GitHub
parent 0971add78a
commit 2ecc68621f
2 changed files with 266 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
patches/chromium/.patches
View File

@@ -118,3 +118,4 @@ make_gtk_getlibgtk_public.patch
cherry-pick-cf64617c1cc5.patch
cherry-pick-e2b8856012e0.patch
cherry-pick-6b66a45021a0.patch
fix_xkb_keysym_reverse_look_up_for_lacros.patch

265
patches/chromium/fix_xkb_keysym_reverse_look_up_for_lacros.patch Normal file
View File

@@ -0,0 +1,265 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Hidehiko Abe <hidehiko@chromium.org>
Date: Wed, 2 Mar 2022 16:17:43 +0000
Subject: Fix xkb_keysym reverse look up for Lacros.
In crrev.com/c/3422444, we introduced
xkb_keymap_key_get_mods_for_level in order to construct a reverse map.
However, unlike its name, and unlike its document, it returns a "mask"
of modifiers, rather than the modifiers to trigger the level.
As a result, the CL introduced a regression on some key events,
such as SHIFT+SPACE.
This CL fixes the issue by giving up creating a simple reverse map.
Instead, on keymap setting, it creates a reverse map from keysym
to a list of (keycode/layout) pairs. And, on look up, we iterate
all the candidates, and find the first one (min keycode entry).
In order to runtime look up, we assume the current key layout,
as the wayland keysym event does not provide any layout info.
diff --git a/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc b/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc
index d8b0594442b2708a03532312b62c4c3481da3b23..d30045bce1af98f68d19e3b01a7a06f67682af16 100644
--- a/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc
+++ b/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc
@@ -31,14 +31,6 @@
#include "ui/events/keycodes/keyboard_code_conversion.h"
#include "ui/events/keycodes/keyboard_code_conversion_xkb.h"
-// xkb_keymap_key_get_mods_for_level is relatively new (introduced in ver 1.0,
-// Sep 6, 2020), thus it is not available on some platform, such as Ubuntu
-// 18.04, which we still supports.
-// Thus declare the function as weak here, so we can check the availability on
-// runtime.
-extern "C" __attribute__((weak)) decltype(
- xkb_keymap_key_get_mods_for_level) xkb_keymap_key_get_mods_for_level;
-
namespace ui {
namespace {
@@ -901,17 +893,7 @@ void XkbKeyboardLayoutEngine::SetKeymap(xkb_keymap* keymap) {
}
// Reconstruct keysym map.
- std::vector<std::pair<XkbKeysymMapKey, uint32_t>> keysym_map;
- auto AddEntries = [&keysym_map](base::span<const xkb_keysym_t> keysyms,
- base::span<const xkb_mod_mask_t> masks,
- xkb_keycode_t keycode) {
- if (keysyms.empty() || masks.empty())
- return;
- for (xkb_keysym_t keysym : keysyms) {
- for (xkb_mod_mask_t mask : masks)
- keysym_map.emplace_back(XkbKeysymMapKey(keysym, mask), keycode);
- }
- };
+ std::vector<XkbKeysymMapEntry> keysym_map;
const xkb_keycode_t min_key = xkb_keymap_min_keycode(keymap);
const xkb_keycode_t max_key = xkb_keymap_max_keycode(keymap);
@@ -925,34 +907,33 @@ void XkbKeyboardLayoutEngine::SetKeymap(xkb_keymap* keymap) {
const xkb_keysym_t* keysyms;
int num_syms = xkb_keymap_key_get_syms_by_level(keymap, keycode, layout,
level, &keysyms);
- if (xkb_keymap_key_get_mods_for_level) {
- xkb_mod_mask_t masks[100]; // Large enough buffer.
- int num_mods = xkb_keymap_key_get_mods_for_level(
- keymap, keycode, layout, level, masks, std::size(masks));
- AddEntries(base::make_span(keysyms, num_syms),
- base::make_span(masks, num_mods), keycode);
- } else {
- // If not, unfortunately, there's no convenient/efficient way
- // to take the possible masks. Thus, use mask 0 always.
- constexpr xkb_mod_mask_t kMask[] = {0};
- AddEntries(base::make_span(keysyms, num_syms), kMask, keycode);
- }
+ for (int i = 0; i < num_syms; ++i)
+ keysym_map.emplace_back(
+ XkbKeysymMapEntry{keysyms[i], keycode, layout});
}
}
}
- // Sort here. If there are multiple entries for a (keysym, mask) pair,
- // min keycode wins.
- std::sort(keysym_map.begin(), keysym_map.end());
+ // Then sort and unique here. On tie break, smaller keycode comes first.
+ std::sort(
+ keysym_map.begin(), keysym_map.end(),
+ [](const XkbKeysymMapEntry& entry1, const XkbKeysymMapEntry& entry2) {
+ return std::tie(entry1.xkb_keysym, entry1.xkb_keycode,
+ entry1.xkb_layout) < std::tie(entry2.xkb_keysym,
+ entry2.xkb_keycode,
+ entry2.xkb_layout);
+ });
keysym_map.erase(
- std::unique(keysym_map.begin(), keysym_map.end(),
- [](const std::pair<XkbKeysymMapKey, uint32_t>& entry1,
- const std::pair<XkbKeysymMapKey, uint32_t>& entry2) {
- return entry1.first == entry2.first;
- }),
+ std::unique(
+ keysym_map.begin(), keysym_map.end(),
+ [](const XkbKeysymMapEntry& entry1, const XkbKeysymMapEntry& entry2) {
+ return std::tie(entry1.xkb_keysym, entry1.xkb_keycode,
+ entry1.xkb_layout) == std::tie(entry2.xkb_keysym,
+ entry2.xkb_keycode,
+ entry2.xkb_layout);
+ }),
keysym_map.end());
- xkb_keysym_map_ = base::flat_map<XkbKeysymMapKey, uint32_t>(
- base::sorted_unique, std::move(keysym_map));
+ xkb_keysym_map_ = std::move(keysym_map);
layout_index_ = 0;
#if BUILDFLAG(IS_CHROMEOS_ASH)
@@ -1000,18 +981,38 @@ int XkbKeyboardLayoutEngine::UpdateModifiers(uint32_t depressed,
DomCode XkbKeyboardLayoutEngine::GetDomCodeByKeysym(uint32_t keysym,
uint32_t modifiers) const {
- // If xkb_keymap_key_get_mods_for_level is not available, all entries are
- // stored with modifiers mask is 0.
- if (!xkb_keymap_key_get_mods_for_level)
- modifiers = 0;
-
- auto iter = xkb_keysym_map_.find(XkbKeysymMapKey(keysym, modifiers));
- if (iter == xkb_keysym_map_.end()) {
- VLOG(1) << "No Keycode found for the keysym: " << keysym
- << ", modifiers: " << modifiers;
- return DomCode::NONE;
+ // Look up all candidates.
+ auto range = std::equal_range(
+ xkb_keysym_map_.begin(), xkb_keysym_map_.end(), XkbKeysymMapEntry{keysym},
+ [](const XkbKeysymMapEntry& entry1, const XkbKeysymMapEntry& entry2) {
+ return entry1.xkb_keysym < entry2.xkb_keysym;
+ });
+ if (range.first != range.second) {
+ // Note: value is already in the lexicographical order, so smaller keycode
+ // comes first.
+ for (std::unique_ptr<xkb_state, XkbStateDeleter> xkb_state(
+ xkb_state_new(xkb_state_get_keymap(xkb_state_.get())));
+ range.first != range.second; ++range.first) {
+ xkb_keycode_t xkb_keycode = range.first->xkb_keycode;
+ xkb_layout_index_t xkb_layout = range.first->xkb_layout;
+ // The argument does not have any info about the layout, so we assume the
+ // current layout here.
+ if (xkb_layout != layout_index_)
+ continue;
+ xkb_state_update_mask(xkb_state.get(), modifiers, 0, 0, 0, 0, xkb_layout);
+ const xkb_keysym_t* out_keysyms;
+ int num_syms =
+ xkb_state_key_get_syms(xkb_state.get(), xkb_keycode, &out_keysyms);
+ for (int i = 0; i < num_syms; ++i) {
+ if (out_keysyms[i] == keysym)
+ return KeycodeConverter::NativeKeycodeToDomCode(xkb_keycode);
+ }
+ }
}
- return KeycodeConverter::NativeKeycodeToDomCode(iter->second);
+
+ VLOG(1) << "No Keycode found for the keysym: " << keysym
+ << ", modifiers: " << modifiers;
+ return DomCode::NONE;
}
bool XkbKeyboardLayoutEngine::XkbLookup(xkb_keycode_t xkb_keycode,
diff --git a/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.h b/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.h
index 4fcbce540e4f4dc419f13be9a45fd97c52f8d2b5..3ed6a030d393a927663e1486efef09cf6add589c 100644
--- a/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.h
+++ b/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.h
@@ -74,14 +74,19 @@ class COMPONENT_EXPORT(EVENTS_OZONE_LAYOUT) XkbKeyboardLayoutEngine
};
std::vector<XkbFlagMapEntry> xkb_flag_map_;
- // Table from (xkb_keysym, xkb_modifier) to xkb_keycode for 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.
- // The first element is keysym value. The second element is a bit-mask of
- // modifiers.
- using XkbKeysymMapKey = std::pair<uint32_t, uint32_t>;
- base::flat_map<XkbKeysymMapKey, uint32_t> xkb_keysym_map_;
+ // The data to reverse look up xkb_keycode/xkb_layout from xkb_keysym.
+ // The data is sorted in the (xkb_keysym, xkb_keycode, xkb_layout) dictionary
+ // order. Note that there can be multiple keycode/layout for a keysym, so
+ // this is a multi map.
+ // We can binary search on this vector by keysym as the key, and iterate from
+ // the begin to the end of the range linearly. Then, on tie break, smaller
+ // keycode wins.
+ struct XkbKeysymMapEntry {
+ xkb_keysym_t xkb_keysym;
+ xkb_keycode_t xkb_keycode;
+ xkb_layout_index_t xkb_layout;
+ };
+ std::vector<XkbKeysymMapEntry> xkb_keysym_map_;
#if BUILDFLAG(IS_CHROMEOS_ASH)
// Flag mask for num lock, which is always considered enabled in ChromeOS.
diff --git a/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine_unittest.cc b/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine_unittest.cc
index 3f9223e6c2aad19309d8d3a9367b58a310023d21..18ea942e5b785e037e3fbbd10d1ff4393cf068d0 100644
--- a/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine_unittest.cc
+++ b/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine_unittest.cc
@@ -18,9 +18,6 @@
#include "ui/events/keycodes/keyboard_code_conversion.h"
#include "ui/events/ozone/layout/scoped_keyboard_layout_engine.h"
-extern "C" __attribute__((weak)) decltype(
- xkb_keymap_key_get_mods_for_level) xkb_keymap_key_get_mods_for_level;
-
namespace ui {
namespace {
@@ -946,35 +943,31 @@ TEST_F(XkbLayoutEngineVkTest, GetDomCodeByKeysym) {
constexpr struct {
uint32_t keysym;
uint32_t modifiers;
- DomCode dom_code;
+ DomCode expected_dom_code;
} kTestCases[] = {
- {65307, 0, ui::DomCode::ESCAPE}, {65288, 0, ui::DomCode::BACKSPACE},
- {65293, 0, ui::DomCode::ENTER}, {65289, 0, ui::DomCode::TAB},
+ {65307, 0, ui::DomCode::ESCAPE},
+ {65288, 0, ui::DomCode::BACKSPACE},
+ {65293, 0, ui::DomCode::ENTER},
+ {65289, 0, ui::DomCode::TAB},
{65056, kShiftMask, ui::DomCode::TAB},
+
+ // Test conflict keysym case. We use '<' as a testing example.
+ // On pc101 layout, intl_backslash is expected without SHIFT modifier.
+ {60, 0, ui::DomCode::INTL_BACKSLASH},
+ // And, if SHIFT is pressed, comma key is expected.
+ {60, kShiftMask, ui::DomCode::COMMA},
+
+ // Test for space key. The keysym mapping has only one keycode entry.
+ // It expects all modifiers are ignored. Used SHIFT as testing example.
+ {32, 0, ui::DomCode::SPACE},
+ {32, kShiftMask, ui::DomCode::SPACE},
};
for (const auto& test_case : kTestCases) {
- SCOPED_TRACE(test_case.keysym);
- EXPECT_EQ(test_case.dom_code, layout_engine_->GetDomCodeByKeysym(
- test_case.keysym, test_case.modifiers));
- }
-
- // Test conflict keysym case. We use '<' as a testing sample.
- constexpr uint32_t kLessThanCode = 60;
- if (xkb_keymap_key_get_mods_for_level) {
- // If there's no modifier, on pc101 us layout, intl_backslash is expected.
- EXPECT_EQ(ui::DomCode::INTL_BACKSLASH,
- layout_engine_->GetDomCodeByKeysym(kLessThanCode, 0));
- // If there's shift modifier, comma key is expected.
- EXPECT_EQ(ui::DomCode::COMMA,
- layout_engine_->GetDomCodeByKeysym(kLessThanCode, kShiftMask));
- } else {
- // If xkb_keymap_key_get_mods_for_level is unavailable, fallback to older
- // implementation, which ignores modifiers.
- EXPECT_EQ(ui::DomCode::COMMA,
- layout_engine_->GetDomCodeByKeysym(kLessThanCode, 0));
- EXPECT_EQ(ui::DomCode::COMMA,
- layout_engine_->GetDomCodeByKeysym(kLessThanCode, kShiftMask));
+ EXPECT_EQ(test_case.expected_dom_code,
+ layout_engine_->GetDomCodeByKeysym(test_case.keysym,
+ test_case.modifiers))
+ << "input: " << test_case.keysym << ", " << test_case.modifiers;
}
}