From 6136c08d4287a575f6ef593e3ecca27dfa517c01 Mon Sep 17 00:00:00 2001 From: patrick96 Date: Wed, 8 Sep 2021 18:13:21 +0200 Subject: [PATCH] tray: Properly handle missing _XEMBED_INFO The icon not supporting XEMBED is not fatal, we just treat is as always mapped. Fixes #2479 Fixes #2442 --- CHANGELOG.md | 5 +++++ include/x11/tray_client.hpp | 20 ++++++++++++++--- include/x11/tray_manager.hpp | 1 - include/x11/xembed.hpp | 37 ++++++++++++++++++++++--------- src/x11/tray_client.cpp | 32 ++++++++++++++++----------- src/x11/tray_manager.cpp | 38 ++++++++++++++++++++------------ src/x11/xembed.cpp | 42 ++++++++++++++++++++++++------------ 7 files changed, 120 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80fa3a51..380dc37e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed +- The tray mistakenly removed tray icons that did not support XEMBED + ([`#2479`](https://github.com/polybar/polybar/issues/2479), + [`#2442`](https://github.com/polybar/polybar/issues/2442)) + ## [3.5.6] - 2021-05-24 ### Build - Support building documentation on sphinx 4.0 ([`#2424`](https://github.com/polybar/polybar/issues/2424)) diff --git a/include/x11/tray_client.hpp b/include/x11/tray_client.hpp index d74c9546..eb2593fa 100644 --- a/include/x11/tray_client.hpp +++ b/include/x11/tray_client.hpp @@ -4,12 +4,12 @@ #include "common.hpp" #include "utils/concurrency.hpp" +#include "x11/xembed.hpp" POLYBAR_NS // fwd declarations class connection; -struct xembed_data; class tray_client { public: @@ -28,7 +28,10 @@ class tray_client { void mapped(bool state); xcb_window_t window() const; - xembed_data* xembed() const; + + void query_xembed(); + bool is_xembed_supported() const; + const xembed::info& get_xembed() const; void ensure_state() const; void reconfigure(int x, int y) const; @@ -38,7 +41,18 @@ class tray_client { connection& m_connection; xcb_window_t m_window{0}; - shared_ptr m_xembed; + /** + * Whether the client window supports XEMBED. + * + * A tray client can still work when it doesn't support XEMBED. + */ + bool m_xembed_supported{false}; + + /** + * _XEMBED_INFO of the client window + */ + xembed::info m_xembed; + bool m_mapped{false}; unsigned int m_width; diff --git a/include/x11/tray_manager.hpp b/include/x11/tray_manager.hpp index 1228c3a9..bf3732e4 100644 --- a/include/x11/tray_manager.hpp +++ b/include/x11/tray_manager.hpp @@ -34,7 +34,6 @@ using namespace std::chrono_literals; // fwd declarations class connection; -struct xembed_data; class background_manager; class bg_slice; diff --git a/include/x11/xembed.hpp b/include/x11/xembed.hpp index b587bbf2..51407612 100644 --- a/include/x11/xembed.hpp +++ b/include/x11/xembed.hpp @@ -21,24 +21,41 @@ POLYBAR_NS #define XEMBED_FOCUS_FIRST 1 #define XEMBED_FOCUS_LAST 1 -struct xembed_data { - unsigned long version; - unsigned long flags; - xcb_timestamp_t time; - xcb_atom_t xembed; - xcb_atom_t xembed_info; -}; +/** + * Max XEMBED version supported. + */ +#define XEMBED_MAX_VERSION UINT32_C(0) +/** + * Implementation of parts of the XEMBED spec (as much as needed to get the tray working). + * + * Ref: https://specifications.freedesktop.org/xembed-spec/xembed-spec-latest.html + */ namespace xembed { - xembed_data* query(connection& conn, xcb_window_t win, xembed_data* data); + + class info { + public: + void set(uint32_t* data); + + uint32_t get_version() const; + uint32_t get_flags() const; + + bool is_mapped() const; + + protected: + uint32_t version; + uint32_t flags; + }; + + bool query(connection& conn, xcb_window_t win, info& data); void send_message(connection& conn, xcb_window_t target, long message, long d1, long d2, long d3); void send_focus_event(connection& conn, xcb_window_t target); - void notify_embedded(connection& conn, xcb_window_t win, xcb_window_t embedder, long version); + void notify_embedded(connection& conn, xcb_window_t win, xcb_window_t embedder, uint32_t version); void notify_activated(connection& conn, xcb_window_t win); void notify_deactivated(connection& conn, xcb_window_t win); void notify_focused(connection& conn, xcb_window_t win, long focus_type); void notify_unfocused(connection& conn, xcb_window_t win); void unembed(connection& conn, xcb_window_t win, xcb_window_t root); -} +} // namespace xembed POLYBAR_NS_END diff --git a/src/x11/tray_client.cpp b/src/x11/tray_client.cpp index 12978fad..11d357ee 100644 --- a/src/x11/tray_client.cpp +++ b/src/x11/tray_client.cpp @@ -5,16 +5,11 @@ #include "utils/memory.hpp" #include "x11/connection.hpp" -#include "x11/xembed.hpp" POLYBAR_NS tray_client::tray_client(connection& conn, xcb_window_t win, unsigned int w, unsigned int h) - : m_connection(conn), m_window(win), m_width(w), m_height(h) { - m_xembed = memory_util::make_malloc_ptr(); - m_xembed->version = XEMBED_VERSION; - m_xembed->flags = XEMBED_MAPPED; -} + : m_connection(conn), m_window(win), m_width(w), m_height(h) {} tray_client::~tray_client() { xembed::unembed(m_connection, window(), m_connection.root()); @@ -60,20 +55,31 @@ xcb_window_t tray_client::window() const { return m_window; } -/** - * Get xembed data pointer - */ -xembed_data* tray_client::xembed() const { - return m_xembed.get(); +void tray_client::query_xembed() { + m_xembed_supported = xembed::query(m_connection, m_window, m_xembed); +} + +bool tray_client::is_xembed_supported() const { + return m_xembed_supported; +} + +const xembed::info& tray_client::get_xembed() const { + return m_xembed; } /** * Make sure that the window mapping state is correct */ void tray_client::ensure_state() const { - if (!mapped() && ((xembed()->flags & XEMBED_MAPPED) == XEMBED_MAPPED)) { + bool should_be_mapped = true; + + if (is_xembed_supported()) { + should_be_mapped = m_xembed.is_mapped(); + } + + if (!mapped() && should_be_mapped) { m_connection.map_window_checked(window()); - } else if (mapped() && ((xembed()->flags & XEMBED_MAPPED) != XEMBED_MAPPED)) { + } else if (mapped() && !should_be_mapped) { m_connection.unmap_window_checked(window()); } } diff --git a/src/x11/tray_manager.cpp b/src/x11/tray_manager.cpp index f8414538..77c2eb5b 100644 --- a/src/x11/tray_manager.cpp +++ b/src/x11/tray_manager.cpp @@ -21,6 +21,12 @@ #include "x11/winspec.hpp" #include "x11/xembed.hpp" +/* + * Tray implementation according to the System Tray Protocol. + * + * Ref: https://specifications.freedesktop.org/systemtray-spec/systemtray-spec-latest.html + */ + // ==================================================================================================== // // TODO: 32-bit visual @@ -740,14 +746,20 @@ void tray_manager::process_docking_request(xcb_window_t win) { auto& client = m_clients.back(); try { - m_log.trace("tray: Get client _XEMBED_INFO"); - xembed::query(m_connection, win, client->xembed()); - } catch (const std::exception& err) { + client->query_xembed(); + } catch (const xpp::x::error::window& err) { m_log.err("Failed to query _XEMBED_INFO, removing client... (%s)", err.what()); remove_client(win, true); return; } + m_log.trace("tray: xembed = %s", client->is_xembed_supported() ? "true" : "false"); + if (client->is_xembed_supported()) { + m_log.trace("tray: version = 0x%x, flags = 0x%x, XEMBED_MAPPED = %s", client->get_xembed().get_version(), + client->get_xembed().get_flags(), client->get_xembed().is_mapped() ? "true" : "false"); + } + + try { const unsigned int mask = XCB_CW_EVENT_MASK; const unsigned int values[]{XCB_EVENT_MASK_PROPERTY_CHANGE | XCB_EVENT_MASK_STRUCTURE_NOTIFY}; @@ -765,10 +777,12 @@ void tray_manager::process_docking_request(xcb_window_t win) { m_connection.reparent_window_checked( client->window(), m_tray, calculate_client_x(client->window()), calculate_client_y()); - m_log.trace("tray: Send embbeded notification to client"); - xembed::notify_embedded(m_connection, client->window(), m_tray, client->xembed()->version); + if (client->is_xembed_supported()) { + m_log.trace("tray: Send embbeded notification to client"); + xembed::notify_embedded(m_connection, client->window(), m_tray, client->get_xembed().get_version()); + } - if (client->xembed()->flags & XEMBED_MAPPED) { + if (!client->is_xembed_supported() || client->get_xembed().is_mapped()) { m_log.trace("tray: Map client"); m_connection.map_window_checked(client->window()); } @@ -1019,7 +1033,6 @@ void tray_manager::handle(const evt::property_notify& evt) { m_log.trace("tray: _XEMBED_INFO: %s", m_connection.id(evt->window)); - auto xd = client->xembed(); auto win = client->window(); if (evt->state == XCB_PROPERTY_NEW_VALUE) { @@ -1027,20 +1040,17 @@ void tray_manager::handle(const evt::property_notify& evt) { } try { - m_log.trace("tray: Get client _XEMBED_INFO"); - xembed::query(m_connection, win, xd); - } catch (const application_error& err) { - m_log.err(err.what()); - return; + client->query_xembed(); } catch (const xpp::x::error::window& err) { m_log.err("Failed to query _XEMBED_INFO, removing client... (%s)", err.what()); remove_client(win, true); return; } - m_log.trace("tray: _XEMBED_INFO[0]=%u _XEMBED_INFO[1]=%u", xd->version, xd->flags); + m_log.trace("tray: version = 0x%x, flags = 0x%x, XEMBED_MAPPED = %s", client->get_xembed().get_version(), + client->get_xembed().get_flags(), client->get_xembed().is_mapped() ? "true" : "false"); - if ((client->xembed()->flags & XEMBED_MAPPED) & XEMBED_MAPPED) { + if (client->get_xembed().is_mapped()) { reconfigure(); } } diff --git a/src/x11/xembed.cpp b/src/x11/xembed.cpp index b426d9c4..1f3f27a4 100644 --- a/src/x11/xembed.cpp +++ b/src/x11/xembed.cpp @@ -5,26 +5,40 @@ POLYBAR_NS namespace xembed { + + void info::set(uint32_t* data) { + version = data[0]; + flags = data[1]; + } + + uint32_t info::get_version() const { + return version; + } + + uint32_t info::get_flags() const { + return flags; + } + + bool info::is_mapped() const { + return (flags & XEMBED_MAPPED) == XEMBED_MAPPED; + } + /** * Query _XEMBED_INFO for the given window + * + * \return Whether valid XEMBED info was found */ - xembed_data* query(connection& conn, xcb_window_t win, xembed_data* data) { - auto info = conn.get_property(false, win, _XEMBED_INFO, XCB_GET_PROPERTY_TYPE_ANY, 0L, 2); + bool query(connection& conn, xcb_window_t win, info& data) { + auto info = conn.get_property(false, win, _XEMBED_INFO, _XEMBED_INFO, 0L, 2); if (info->value_len == 0) { - throw application_error("Invalid _XEMBED_INFO for window " + conn.id(win)); + return false; } - std::vector xembed_data{info.value().begin(), info.value().end()}; + std::vector xembed_data{info.value().begin(), info.value().end()}; + data.set(xembed_data.data()); - data->xembed = _XEMBED; - data->xembed_info = _XEMBED_INFO; - - data->time = XCB_CURRENT_TIME; - data->flags = xembed_data[1]; - data->version = xembed_data[0]; - - return data; + return true; } /** @@ -53,8 +67,8 @@ namespace xembed { /** * Acknowledge window embedding */ - void notify_embedded(connection& conn, xcb_window_t win, xcb_window_t embedder, long version) { - send_message(conn, win, XEMBED_EMBEDDED_NOTIFY, 0, embedder, version); + void notify_embedded(connection& conn, xcb_window_t win, xcb_window_t embedder, uint32_t version) { + send_message(conn, win, XEMBED_EMBEDDED_NOTIFY, 0, embedder, std::min(version, XEMBED_MAX_VERSION)); } /**