From 628ab03b50493b0ae03e760475eeefc35e90e78d Mon Sep 17 00:00:00 2001 From: BiFangKNT <1320414964@qq.com> Date: Wed, 14 Oct 2026 02:52:05 +0800 Subject: [PATCH] fix(linux): honor tray primary action in KDE Derive ItemIsMenu from primary action availability and synchronize callback presence when the action changes. Emit NewTitle alongside PropertiesChanged so Plasma refreshes its cached activation mode. Protect click callback access across native and JNI threads. Add D-Bus regression coverage for activation, host cache refresh, and context menus, and run it in CI. Verified with automated tests and manual testing on Fedora KDE. --- .github/workflows/build-natives.yaml | 4 + .github/workflows/pr-build-check.yml | 3 + .../lib/linux/LinuxTrayManager.kt | 31 +-- .../lib/linux/LinuxPrimaryActionTest.kt | 91 +++++++++ src/native/linux/jni_bridge.c | 18 +- src/native/linux/run_primary_action_test.sh | 12 ++ src/native/linux/sni.c | 20 +- src/native/linux/sni.h | 2 + src/native/linux/test_sni_primary_action.c | 188 ++++++++++++++++++ 9 files changed, 352 insertions(+), 17 deletions(-) create mode 100644 src/jvmTest/kotlin/dev/nucleusframework/composenativetray/lib/linux/LinuxPrimaryActionTest.kt create mode 100644 src/native/linux/run_primary_action_test.sh create mode 100644 src/native/linux/test_sni_primary_action.c diff --git a/.github/workflows/build-natives.yaml b/.github/workflows/build-natives.yaml index 59fb6581..9e18e26c 100644 --- a/.github/workflows/build-natives.yaml +++ b/.github/workflows/build-natives.yaml @@ -82,6 +82,10 @@ jobs: working-directory: src/native/linux run: bash run_pixmap_test.sh + - name: Primary action controls ItemIsMenu + working-directory: src/native/linux + run: bash run_primary_action_test.sh + - name: JNI strings are standard UTF-8 (issue #441) working-directory: src/native/shared run: bash run_jni_utf8_test.sh diff --git a/.github/workflows/pr-build-check.yml b/.github/workflows/pr-build-check.yml index 3e9dd46d..63c1b17b 100644 --- a/.github/workflows/pr-build-check.yml +++ b/.github/workflows/pr-build-check.yml @@ -76,3 +76,6 @@ jobs: - name: Build run: ./gradlew build --continue + + - name: Linux primary action updates reach the tray host + run: dbus-run-session -- env COMPOSE_TRAY_DBUS_TEST=1 ./gradlew :jvmTest --tests '*LinuxPrimaryActionTest' --rerun-tasks diff --git a/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/lib/linux/LinuxTrayManager.kt b/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/lib/linux/LinuxTrayManager.kt index d8b6c56f..1ae78baf 100644 --- a/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/lib/linux/LinuxTrayManager.kt +++ b/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/lib/linux/LinuxTrayManager.kt @@ -58,6 +58,22 @@ internal class LinuxTrayManager( // Native handle private var trayHandle: Long = 0L + private val clickCallback = + JniRunnable { + val action = lock.withLock { onLeftClick } ?: return@JniRunnable + try { + val xy = IntArray(2) + native.nativeGetLastClickXY(trayHandle, xy) + TrayClickTracker.recordClick(xy[0], xy[1]) + } catch (_: Throwable) { + } + action() + } + + private fun syncClickCallback() { + native.nativeSetClickCallback(trayHandle, if (onLeftClick != null) clickCallback else null) + } + // Thread / lifecycle private var shutdownHook: Thread? = null private var loopThread: Thread? = null @@ -110,7 +126,9 @@ internal class LinuxTrayManager( tooltipChanged = (tooltip != newTooltip) iconPath = newIconPath tooltip = newTooltip + val clickPresenceChanged = (onLeftClick == null) != (newOnLeftClick == null) onLeftClick = newOnLeftClick + if (clickPresenceChanged && trayHandle != 0L) syncClickCallback() onMenuOpened = newOnMenuOpened if (newMenuItems != null) { menuItems.clear() @@ -164,18 +182,7 @@ internal class LinuxTrayManager( runCatching { native.nativeSetTitle(trayHandle, tooltip) } // Set click callback - native.nativeSetClickCallback( - trayHandle, - JniRunnable { - try { - val xy = IntArray(2) - native.nativeGetLastClickXY(trayHandle, xy) - TrayClickTracker.recordClick(xy[0], xy[1]) - } catch (_: Throwable) { - } - onLeftClick?.invoke() - }, - ) + lock.withLock { syncClickCallback() } // Set menu-opened callback native.nativeSetMenuOpenedCallback( diff --git a/src/jvmTest/kotlin/dev/nucleusframework/composenativetray/lib/linux/LinuxPrimaryActionTest.kt b/src/jvmTest/kotlin/dev/nucleusframework/composenativetray/lib/linux/LinuxPrimaryActionTest.kt new file mode 100644 index 00000000..06504079 --- /dev/null +++ b/src/jvmTest/kotlin/dev/nucleusframework/composenativetray/lib/linux/LinuxPrimaryActionTest.kt @@ -0,0 +1,91 @@ +package dev.nucleusframework.composenativetray.lib.linux + +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Run on Linux with busctl installed: + * `dbus-run-session -- env COMPOSE_TRAY_DBUS_TEST=1 ./gradlew :jvmTest --tests '*LinuxPrimaryActionTest'` + */ +class LinuxPrimaryActionTest { + @Test + fun `manager synchronizes primary action presence and replacement`() { + if (System.getenv("COMPOSE_TRAY_DBUS_TEST") != "1") return + val calls = AtomicInteger() + val tray = LinuxTrayManager("primary-action-test", "", "test") + tray.startTray() + try { + awaitProperty("b true") + activate() + assertEquals(0, calls.get()) + + tray.update("", "test", { calls.incrementAndGet() }, null) + awaitProperty("b false") + activate() + awaitCalls(calls, 1) + + tray.update("", "test", { calls.addAndGet(10) }, null) + activate() + awaitCalls(calls, 11) + + tray.update("", "test", null, null) + awaitProperty("b true") + activate() + assertEquals(11, calls.get()) + } finally { + tray.stopTray() + } + + val initialActionTray = + LinuxTrayManager("initial-action-test", "", "test", { calls.incrementAndGet() }) + initialActionTray.startTray() + try { + awaitProperty("b false") + activate() + awaitCalls(calls, 12) + } finally { + initialActionTray.stopTray() + } + } + + private fun awaitCalls( + calls: AtomicInteger, + expected: Int, + ) { + repeat(200) { + if (calls.get() == expected) return + Thread.sleep(10) + } + assertEquals(expected, calls.get()) + } + + private fun awaitProperty(expected: String) { + var output = "" + repeat(200) { + output = busctl("get-property", service, path, iface, "ItemIsMenu") + if (output == expected) return + Thread.sleep(10) + } + assertEquals(expected, output) + } + + private fun activate() { + assertEquals("", busctl("call", service, path, iface, "Activate", "ii", "12", "34")) + // Activate's D-Bus reply can arrive before the queued JVM callback. + Thread.sleep(50) + } + + private fun busctl(vararg args: String): String { + val process = + ProcessBuilder(listOf("busctl", "--user", "--timeout=2") + args).redirectErrorStream(true).start() + assertTrue(process.waitFor(5, TimeUnit.SECONDS), "busctl timed out") + return process.inputStream.bufferedReader().readText().trim() + } + + private val service = "org.kde.StatusNotifierItem-${ProcessHandle.current().pid()}-1" + private val path = "/StatusNotifierItem" + private val iface = "org.kde.StatusNotifierItem" +} diff --git a/src/native/linux/jni_bridge.c b/src/native/linux/jni_bridge.c index 5c813cfb..a9e40aeb 100644 --- a/src/native/linux/jni_bridge.c +++ b/src/native/linux/jni_bridge.c @@ -16,6 +16,7 @@ #include #include #include +#include #include "jni_utf8.h" #include "sni.h" @@ -53,6 +54,7 @@ typedef struct CallbackEntry { } CallbackEntry; static CallbackEntry *g_clickCallback = NULL; +static pthread_mutex_t g_clickCallbackLock = PTHREAD_MUTEX_INITIALIZER; static CallbackEntry *g_rclickCallback = NULL; static CallbackEntry *g_menuCallbacks = NULL; static CallbackEntry *g_menuOpenedCallback = NULL; @@ -132,8 +134,16 @@ static void invokeRunnable(jobject runnable) { static void click_trampoline(int32_t x, int32_t y, void *userdata) { (void)x; (void)y; uintptr_t key = (uintptr_t)userdata; - jobject runnable = findCallback(g_clickCallback, key); - if (runnable) invokeRunnable(runnable); + JNIEnv *env = getJNIEnv(); + if (!env) return; + pthread_mutex_lock(&g_clickCallbackLock); + jobject globalRef = findCallback(g_clickCallback, key); + jobject runnable = globalRef ? (*env)->NewLocalRef(env, globalRef) : NULL; + pthread_mutex_unlock(&g_clickCallbackLock); + if (runnable) { + invokeRunnable(runnable); + (*env)->DeleteLocalRef(env, runnable); + } } static void rclick_trampoline(int32_t x, int32_t y, void *userdata) { @@ -215,7 +225,9 @@ Java_dev_nucleusframework_composenativetray_lib_linux_LinuxNativeBridge_nativeDe /* Clean up all callbacks for this tray */ uintptr_t key = (uintptr_t)tray; + pthread_mutex_lock(&g_clickCallbackLock); storeCallback(&g_clickCallback, key, env, NULL); + pthread_mutex_unlock(&g_clickCallbackLock); storeCallback(&g_rclickCallback, key, env, NULL); storeCallback(&g_menuOpenedCallback, key, env, NULL); /* Menu callbacks are keyed by item id, clear all */ @@ -274,7 +286,9 @@ Java_dev_nucleusframework_composenativetray_lib_linux_LinuxNativeBridge_nativeSe sni_tray *tray = (sni_tray *)(uintptr_t)handle; if (!tray) return; uintptr_t key = (uintptr_t)tray; + pthread_mutex_lock(&g_clickCallbackLock); storeCallback(&g_clickCallback, key, env, callback); + pthread_mutex_unlock(&g_clickCallbackLock); sni_tray_set_click_callback(tray, callback ? click_trampoline : NULL, (void *)key); diff --git a/src/native/linux/run_primary_action_test.sh b/src/native/linux/run_primary_action_test.sh new file mode 100644 index 00000000..7cb64a6e --- /dev/null +++ b/src/native/linux/run_primary_action_test.sh @@ -0,0 +1,12 @@ +#!/bin/bash +# Use a private session bus so the test does not register with the user's desktop. +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +BIN="$(mktemp)" +trap 'rm -f "$BIN"' EXIT + +gcc -O2 -g -Wall -Wextra -Werror -Wno-unused-parameter \ + $(pkg-config --cflags libsystemd) \ + "$SCRIPT_DIR/sni.c" "$SCRIPT_DIR/test_sni_primary_action.c" \ + $(pkg-config --libs libsystemd) -lpthread -lm -ldl -o "$BIN" +dbus-run-session -- "$BIN" diff --git a/src/native/linux/sni.c b/src/native/linux/sni.c index edfd7084..8b8c553b 100644 --- a/src/native/linux/sni.c +++ b/src/native/linux/sni.c @@ -246,9 +246,14 @@ static void flush_callbacks(sni_tray *tray) { for (int i = 0; i < tray->pending_count; i++) { pending_cb *c = &tray->pending[i]; switch (c->type) { - case PCB_CLICK: - if (tray->on_click) tray->on_click(c->x, c->y, tray->on_click_data); + case PCB_CLICK: { + bus_lock(tray); + sni_click_cb callback = tray->on_click; + void *userdata = tray->on_click_data; + bus_unlock(tray); + if (callback) callback(c->x, c->y, userdata); break; + } case PCB_RCLICK: if (tray->on_rclick) tray->on_rclick(c->x, c->y, tray->on_rclick_data); break; @@ -510,7 +515,7 @@ static int sni_get_property(sd_bus *bus, const char *path, const char *interface if (strcmp(property, "Menu") == 0) return sd_bus_message_append(reply, "o", tray->current_menu_path); if (strcmp(property, "ItemIsMenu") == 0) - return sd_bus_message_append(reply, "b", 1); + return sd_bus_message_append(reply, "b", tray->on_click == NULL); if (strcmp(property, "IconName") == 0) return sd_bus_message_append(reply, "s", ""); if (strcmp(property, "IconPixmap") == 0) @@ -1285,8 +1290,17 @@ void sni_tray_set_tooltip(sni_tray *tray, const char *tooltip) { void sni_tray_set_click_callback(sni_tray *tray, sni_click_cb cb, void *userdata) { if (!tray) return; + bus_lock(tray); + int changed = (tray->on_click == NULL) != (cb == NULL); tray->on_click = cb; tray->on_click_data = userdata; + if (changed) { + emit_sni_properties_changed(tray, "ItemIsMenu"); + /* Plasma refreshes its cached ItemIsMenu via SNI signals, rather than + * PropertiesChanged. NewTitle triggers GetAll without rebuilding the menu. */ + emit_new_title(tray); + } + bus_unlock(tray); } void sni_tray_set_rclick_callback(sni_tray *tray, sni_click_cb cb, void *userdata) { diff --git a/src/native/linux/sni.h b/src/native/linux/sni.h index adc8a555..8f83fb04 100644 --- a/src/native/linux/sni.h +++ b/src/native/linux/sni.h @@ -67,6 +67,8 @@ void sni_tray_set_tooltip(sni_tray *tray, const char *tooltip); /* ── Click callbacks ───────────────────────────────────────────────── */ +/* A non-NULL primary action sets ItemIsMenu=false; NULL restores menu-only + * activation. Changes are announced to the tray host while running. */ void sni_tray_set_click_callback(sni_tray *tray, sni_click_cb cb, void *userdata); void sni_tray_set_rclick_callback(sni_tray *tray, sni_click_cb cb, void *userdata); void sni_tray_set_menu_callback(sni_tray *tray, sni_menu_item_cb cb, void *userdata); diff --git a/src/native/linux/test_sni_primary_action.c b/src/native/linux/test_sni_primary_action.c new file mode 100644 index 00000000..7f1b380f --- /dev/null +++ b/src/native/linux/test_sni_primary_action.c @@ -0,0 +1,188 @@ +/* Exercise the public SNI protocol on a private session bus. */ +#include "sni.h" +#include +#include +#include +#include +#include +#include +#include + +static const char *path = "/StatusNotifierItem"; +static const char *iface = "org.kde.StatusNotifierItem"; +static atomic_int clicks, right_clicks; +static int changes, last_item_is_menu; +static int refresh_pending, refreshes, cached_item_is_menu; +static char service[128]; + +/* Model Plasma's host cache: refresh GetAll on NewTitle, ignoring + * PropertiesChanged for routing clicks. See StatusNotifierItemSource in: + * https://github.com/KDE/plasma-workspace/blob/v6.7.5/applets/systemtray/statusnotifieritemsource.cpp */ +static void refresh_host_cache(sd_bus *bus) { + sd_bus_message *reply = NULL; + assert(sd_bus_call_method(bus, service, path, "org.freedesktop.DBus.Properties", + "GetAll", NULL, &reply, "s", iface) >= 0); + assert(sd_bus_message_enter_container(reply, 'a', "{sv}") > 0); + int found = 0; + while (sd_bus_message_enter_container(reply, 'e', "sv") > 0) { + const char *property; + assert(sd_bus_message_read(reply, "s", &property) > 0); + if (strcmp(property, "ItemIsMenu") == 0) { + assert(sd_bus_message_enter_container(reply, 'v', "b") > 0); + assert(sd_bus_message_read(reply, "b", &cached_item_is_menu) > 0); + assert(sd_bus_message_exit_container(reply) >= 0); + found = 1; + } else { + assert(sd_bus_message_skip(reply, "v") >= 0); + } + assert(sd_bus_message_exit_container(reply) >= 0); + } + assert(found); + sd_bus_message_unref(reply); +} + +static int new_title(sd_bus_message *message, void *data, sd_bus_error *error) { + (void)message; (void)data; (void)error; + refresh_pending = 1; + refreshes++; + return 0; +} + +static void *run(void *tray) { + assert(sni_tray_run(tray) == 0); + return NULL; +} + +static void click(int32_t x, int32_t y, void *data) { + assert(x == 12 && y == 34); + atomic_fetch_add(&clicks, 1); + /* User callbacks must be able to update the tray without deadlocking. */ + if (data) sni_tray_set_click_callback(data, NULL, NULL); +} + +static void right_click(int32_t x, int32_t y, void *data) { + (void)data; + assert(x == 12 && y == 34); + atomic_fetch_add(&right_clicks, 1); +} + +static int property_changed(sd_bus_message *message, void *data, sd_bus_error *error) { + (void)data; (void)error; + const char *interface, *property; + assert(sd_bus_message_read(message, "s", &interface) > 0); + assert(sd_bus_message_enter_container(message, 'a', "{sv}") > 0); + while (sd_bus_message_enter_container(message, 'e', "sv") > 0) { + assert(sd_bus_message_read(message, "s", &property) > 0); + if (strcmp(interface, iface) == 0 && strcmp(property, "ItemIsMenu") == 0) { + assert(sd_bus_message_enter_container(message, 'v', "b") > 0); + assert(sd_bus_message_read(message, "b", &last_item_is_menu) > 0); + assert(sd_bus_message_exit_container(message) >= 0); + changes++; + } else { + assert(sd_bus_message_skip(message, "v") >= 0); + } + assert(sd_bus_message_exit_container(message) >= 0); + } + return 0; +} + +static void pump(sd_bus *bus, int milliseconds) { + for (int i = 0; i < milliseconds; i++) { + int result; + while ((result = sd_bus_process(bus, NULL)) > 0) {} + assert(result >= 0); + if (refresh_pending) { + refresh_pending = 0; + refresh_host_cache(bus); + } + usleep(1000); + } +} + +static void expect_property(sd_bus *bus, int expected) { + int value = -1, result = -1; + /* Registration happens asynchronously on the tray thread. */ + for (int i = 0; i < 200 && result < 0; i++) { + result = sd_bus_get_property_trivial(bus, service, path, iface, + "ItemIsMenu", NULL, 'b', &value); + if (result < 0) usleep(10000); + } + assert(result >= 0 && value == expected); +} + +static void call(sd_bus *bus, const char *method) { + assert(sd_bus_call_method(bus, service, path, iface, method, NULL, NULL, + "ii", 12, 34) >= 0); + pump(bus, 100); +} + +static void host_click(sd_bus *bus) { + call(bus, cached_item_is_menu ? "ContextMenu" : "Activate"); +} + +int main(void) { + sd_bus *bus = NULL; + sd_bus_slot *slot = NULL; + sd_bus_slot *title_slot = NULL; + assert(sd_bus_open_user(&bus) >= 0); + snprintf(service, sizeof(service), "org.kde.StatusNotifierItem-%d-1", getpid()); + assert(sd_bus_match_signal(bus, &slot, service, path, + "org.freedesktop.DBus.Properties", "PropertiesChanged", + property_changed, NULL) >= 0); + assert(sd_bus_match_signal(bus, &title_slot, service, path, iface, + "NewTitle", new_title, NULL) >= 0); + sni_tray *tray = sni_tray_create(NULL, 0, "primary action test"); + assert(tray); + sni_tray_set_rclick_callback(tray, right_click, NULL); + pthread_t thread; + assert(pthread_create(&thread, NULL, run, tray) == 0); + expect_property(bus, 1); + refresh_host_cache(bus); + call(bus, "Activate"); + assert(atomic_load(&clicks) == 0); + + sni_tray_set_click_callback(tray, click, NULL); + expect_property(bus, 0); + pump(bus, 100); + assert(changes == 1 && last_item_is_menu == 0); + assert(refreshes == 1 && cached_item_is_menu == 0); + host_click(bus); + assert(atomic_load(&clicks) == 1); + call(bus, "ContextMenu"); + assert(atomic_load(&right_clicks) == 1); + + /* Replacing a present callback must not announce a property transition. */ + sni_tray_set_click_callback(tray, click, tray); + pump(bus, 100); + assert(changes == 1 && refreshes == 1); + host_click(bus); + assert(atomic_load(&clicks) == 2); + expect_property(bus, 1); + assert(changes == 2 && last_item_is_menu == 1); + assert(refreshes == 2 && cached_item_is_menu == 1); + call(bus, "Activate"); + assert(atomic_load(&clicks) == 2); + host_click(bus); + assert(atomic_load(&right_clicks) == 2); + + sni_tray_quit(tray); + pthread_join(thread, NULL); + sni_tray_destroy(tray); + + /* A primary action installed before registration starts with ItemIsMenu=false. */ + tray = sni_tray_create(NULL, 0, "initial primary action"); + assert(tray); + sni_tray_set_click_callback(tray, click, NULL); + assert(pthread_create(&thread, NULL, run, tray) == 0); + expect_property(bus, 0); + call(bus, "Activate"); + assert(atomic_load(&clicks) == 3); + sni_tray_quit(tray); + pthread_join(thread, NULL); + sni_tray_destroy(tray); + sd_bus_slot_unref(slot); + sd_bus_slot_unref(title_slot); + sd_bus_unref(bus); + puts("Primary action properties, host cache refresh, activation and context menu passed."); + return 0; +}