Skip to content

Commit

Permalink
global-shortcuts: fix crash by ignoring duplicate shortcuts (#241)
Browse files Browse the repository at this point in the history 
* global shortcuts: fix crash by ignoring duplicate shortcuts

hyprland_global_shortcuts_manager_v1_register_shortcut will cause an
error if the (app_id, shortcut_id) combination has already been
registered. Ignore shortcuts that have already been registered.

When a shortcut is registered again it should also overwrite the
session for the keybind to make the shortcut work after restarting
an application, otherwise the key can't be used again.

* Show warning if the shortcut has already been registered

* Remove {} around short ifs
  • Loading branch information
@dec05eba
dec05eba authored Jul 23, 2024
1 parent 663be9c commit 3b8c781
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 46 deletions.
97 changes: 51 additions & 46 deletions src/portals/GlobalShortcuts.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,49 @@ CGlobalShortcutsPortal::SSession* CGlobalShortcutsPortal::getSession(sdbus::Obje
return nullptr;
}

SKeybind* CGlobalShortcutsPortal::getShortcutById(const std::string& appID, const std::string& shortcutId) {
for (auto& s : m_vSessions) {
if (s->appid != appID)
continue;

for (auto& keybind : s->keybinds) {
if (keybind->id == shortcutId)
return keybind.get();
}
}

return nullptr;
}

SKeybind* CGlobalShortcutsPortal::registerShortcut(SSession* session, const DBusShortcut& shortcut) {
std::string id = shortcut.get<0>();
std::unordered_map<std::string, sdbus::Variant> data = shortcut.get<1>();
std::string description;

for (auto& [k, v] : data) {
if (k == "description")
description = v.get<std::string>();
else
Debug::log(LOG, "[globalshortcuts] unknown shortcut data type {}", k);
}

auto* PSHORTCUT = getShortcutById(session->appid, id);
if (PSHORTCUT)
Debug::log(WARN, "[globalshortcuts] shortcut {} already registered for appid {}", id, session->appid);
else {
PSHORTCUT = session->keybinds.emplace_back(std::make_unique<SKeybind>()).get();
PSHORTCUT->shortcut =
hyprland_global_shortcuts_manager_v1_register_shortcut(m_sState.manager, id.c_str(), session->appid.c_str(), description.c_str(), "");
hyprland_global_shortcut_v1_add_listener(PSHORTCUT->shortcut, &shortcutListener, PSHORTCUT);
}

PSHORTCUT->id = std::move(id);
PSHORTCUT->description = std::move(description);
PSHORTCUT->session = session;

return PSHORTCUT;
}

void CGlobalShortcutsPortal::onCreateSession(sdbus::MethodCall& call) {
sdbus::ObjectPath requestHandle, sessionHandle;

Expand Down Expand Up @@ -61,30 +104,10 @@ void CGlobalShortcutsPortal::onCreateSession(sdbus::MethodCall& call) {
if (k == "shortcuts") {
PSESSION->registered = true;

std::vector<sdbus::Struct<std::string, std::unordered_map<std::string, sdbus::Variant>>> shortcuts;

shortcuts = v.get<std::vector<sdbus::Struct<std::string, std::unordered_map<std::string, sdbus::Variant>>>>();
std::vector<DBusShortcut> shortcuts = v.get<std::vector<DBusShortcut>>();

for (auto& s : shortcuts) {
const auto PSHORTCUT = PSESSION->keybinds.emplace_back(std::make_unique<SKeybind>()).get();

PSHORTCUT->id = s.get<0>();

std::unordered_map<std::string, sdbus::Variant> data = s.get<1>();

for (auto& [k, v] : data) {
if (k == "description") {
PSHORTCUT->description = v.get<std::string>();
} else {
Debug::log(LOG, "[globalshortcuts] unknown shortcut data type {}", k);
}
}

PSHORTCUT->shortcut =
hyprland_global_shortcuts_manager_v1_register_shortcut(m_sState.manager, PSHORTCUT->id.c_str(), PSESSION->appid.c_str(), PSHORTCUT->description.c_str(), "");
hyprland_global_shortcut_v1_add_listener(PSHORTCUT->shortcut, &shortcutListener, PSHORTCUT);

PSHORTCUT->session = PSESSION;
registerShortcut(PSESSION, s);
}

Debug::log(LOG, "[globalshortcuts] registered {} shortcuts", shortcuts.size());
Expand All @@ -105,8 +128,8 @@ void CGlobalShortcutsPortal::onBindShortcuts(sdbus::MethodCall& call) {
Debug::log(LOG, "[globalshortcuts] Bind keys:");
Debug::log(LOG, "[globalshortcuts] | {}", sessionHandle.c_str());

std::vector<sdbus::Struct<std::string, std::unordered_map<std::string, sdbus::Variant>>> shortcuts;
std::vector<sdbus::Struct<std::string, std::unordered_map<std::string, sdbus::Variant>>> shortcutsToReturn;
std::vector<DBusShortcut> shortcuts;
std::vector<DBusShortcut> shortcutsToReturn;
call >> shortcuts;

const auto PSESSION = getSession(sessionHandle);
Expand All @@ -119,25 +142,7 @@ void CGlobalShortcutsPortal::onBindShortcuts(sdbus::MethodCall& call) {
PSESSION->registered = true;

for (auto& s : shortcuts) {
const auto PSHORTCUT = PSESSION->keybinds.emplace_back(std::make_unique<SKeybind>()).get();

PSHORTCUT->id = s.get<0>();

std::unordered_map<std::string, sdbus::Variant> data = s.get<1>();

for (auto& [k, v] : data) {
if (k == "description") {
PSHORTCUT->description = v.get<std::string>();
} else {
Debug::log(LOG, "[globalshortcuts] unknown shortcut data type {}", k);
}
}

PSHORTCUT->shortcut =
hyprland_global_shortcuts_manager_v1_register_shortcut(m_sState.manager, PSHORTCUT->id.c_str(), PSESSION->appid.c_str(), PSHORTCUT->description.c_str(), "");
hyprland_global_shortcut_v1_add_listener(PSHORTCUT->shortcut, &shortcutListener, PSHORTCUT);

PSHORTCUT->session = PSESSION;
const auto* PSHORTCUT = registerShortcut(PSESSION, s);

std::unordered_map<std::string, sdbus::Variant> shortcutData;
shortcutData["description"] = PSHORTCUT->description;
Expand Down Expand Up @@ -172,7 +177,7 @@ void CGlobalShortcutsPortal::onListShortcuts(sdbus::MethodCall& call) {
return;
}

std::vector<sdbus::Struct<std::string, std::unordered_map<std::string, sdbus::Variant>>> shortcuts;
std::vector<DBusShortcut> shortcuts;

for (auto& s : PSESSION->keybinds) {
std::unordered_map<std::string, sdbus::Variant> opts;
Expand Down Expand Up @@ -211,7 +216,7 @@ CGlobalShortcutsPortal::CGlobalShortcutsPortal(hyprland_global_shortcuts_manager
void CGlobalShortcutsPortal::onActivated(SKeybind* pKeybind, uint64_t time) {
const auto PSESSION = (CGlobalShortcutsPortal::SSession*)pKeybind->session;

Debug::log(TRACE, "[gs] Session {} called activated on {}", (void*)PSESSION, pKeybind->id);
Debug::log(TRACE, "[gs] Session {} called activated on {}", PSESSION->sessionHandle.c_str(), pKeybind->id);

auto signal = m_pObject->createSignal(INTERFACE_NAME, "Activated");
signal << PSESSION->sessionHandle;
Expand All @@ -225,7 +230,7 @@ void CGlobalShortcutsPortal::onActivated(SKeybind* pKeybind, uint64_t time) {
void CGlobalShortcutsPortal::onDeactivated(SKeybind* pKeybind, uint64_t time) {
const auto PSESSION = (CGlobalShortcutsPortal::SSession*)pKeybind->session;

Debug::log(TRACE, "[gs] Session {} called deactivated on {}", (void*)PSESSION, pKeybind->id);
Debug::log(TRACE, "[gs] Session {} called deactivated on {}", PSESSION->sessionHandle.c_str(), pKeybind->id);

auto signal = m_pObject->createSignal(INTERFACE_NAME, "Deactivated");
signal << PSESSION->sessionHandle;
Expand Down
4 changes: 4 additions & 0 deletions src/portals/GlobalShortcuts.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ class CGlobalShortcutsPortal {

std::unique_ptr<sdbus::IObject> m_pObject;

using DBusShortcut = sdbus::Struct<std::string, std::unordered_map<std::string, sdbus::Variant>>;

SSession* getSession(sdbus::ObjectPath& path);
SKeybind* getShortcutById(const std::string& appID, const std::string& shortcutId);
SKeybind* registerShortcut(SSession* session, const DBusShortcut& shortcut);

const std::string INTERFACE_NAME = "org.freedesktop.impl.portal.GlobalShortcuts";
const std::string OBJECT_PATH = "/org/freedesktop/portal/desktop";
Expand Down

0 comments on commit 3b8c781

Please sign in to comment.