Merge pull request #381 from bnema/fix/global-shortcut-action-ownership

Fix GTK shortcut reload crash

commit: 48cc1e2169a1ac09813cd399c7e3f8a1ed96ba85
author: bnema · 2mo ago · 2026-08-14 02:19 UTC
committer: GitHub · 2mo ago · 2026-08-14 02:19 UTC
merge of: dd6d7a0 + f940181 · diff against the first parent
signature: GPG UnverifiedB5690EEEBB952194 The key is unknown

2 files · +76 -13

M internal/ui/input/global_shortcuts.go +10 -10


@@ -31,7 +31,6 @@ type GlobalShortcutHandler struct {
31	31		shortcutRefs map[string]globalShortcutRegistration
32	32		lastDispatchAt map[Action]time.Time
33	33		heldShortcuts map[globalShortcutHoldKey]struct{}
34		-	shortcutAction *gtk.NamedAction
35	34		globalAction *gio.SimpleAction
36	35		globalActionCb func(gio.SimpleAction, uintptr)
37	36		globalActionHandlerID uint

@@ -145,7 +144,6 @@ func (h *GlobalShortcutHandler) configureGlobalControllers() {
145	144		// Set global scope - this is the key to making shortcuts work
146	145		// even when WebView has focus.
147	146		h.controller.SetScope(gtk.ShortcutScopeGlobalValue)
148		-	h.shortcutAction = gtk.NewNamedAction(globalShortcutActionFullName)
149	147		h.globalAction = gio.NewSimpleAction(globalShortcutActionName, glib.NewVariantType("s"))
150	148		if h.globalAction != nil {
151	149		h.globalActionCb = func(_ gio.SimpleAction, parameter uintptr) {

@@ -218,23 +216,26 @@ func (h *GlobalShortcutHandler) registerShortcut(keyval uint, modifiers gdk.Modi
218	216		return false
219	217		}
220	218
221		-	if h.shortcutAction == nil {
	219	+	// Create trigger for this key combination.
	220	+	trigger := gtk.NewKeyvalTrigger(keyval, modifiers)
	221	+	if trigger == nil {
222	222		logging.FromContext(h.ctx).Error().
223	223		Uint("keyval", keyval).
224		-	Msg("global shortcut action is unavailable")
	224	+	Msg("failed to create keyval trigger")
225	225		return false
226	226		}
227	227
228		-	// Create trigger for this key combination.
229		-	trigger := gtk.NewKeyvalTrigger(keyval, modifiers)
230		-	if trigger == nil {
	228	+	// GtkShortcut takes ownership of its action. Each shortcut needs its own
	229	+	// named action even though they all dispatch through the same window action.
	230	+	shortcutAction := gtk.NewNamedAction(globalShortcutActionFullName)
	231	+	if shortcutAction == nil {
231	232		logging.FromContext(h.ctx).Error().
232	233		Uint("keyval", keyval).
233		-	Msg("failed to create keyval trigger")
	234	+	Msg("failed to create global shortcut action")
234	235		return false
235	236		}
236	237
237		-	shortcut := gtk.NewShortcut(&trigger.ShortcutTrigger, &h.shortcutAction.ShortcutAction)
	238	+	shortcut := gtk.NewShortcut(&trigger.ShortcutTrigger, &shortcutAction.ShortcutAction)
238	239		if shortcut == nil {
239	240		logging.FromContext(h.ctx).Error().
240	241		Uint("keyval", keyval).

@@ -634,7 +635,6 @@ func (h *GlobalShortcutHandler) detach(removeFromWindow bool) {
634	635		}
635	636		h.controller = nil
636	637		h.releaseController = nil
637		-	h.shortcutAction = nil
638	638		h.globalAction = nil
639	639		h.globalActionCb = nil
640	640		h.globalActionHandlerID = 0

 	shortcutRefs          map[string]globalShortcutRegistration
 	lastDispatchAt        map[Action]time.Time
 	heldShortcuts         map[globalShortcutHoldKey]struct{}
-	shortcutAction        *gtk.NamedAction
 	globalAction          *gio.SimpleAction
 	globalActionCb        func(gio.SimpleAction, uintptr)
 	globalActionHandlerID uint
 	// Set global scope - this is the key to making shortcuts work
 	// even when WebView has focus.
 	h.controller.SetScope(gtk.ShortcutScopeGlobalValue)
-	h.shortcutAction = gtk.NewNamedAction(globalShortcutActionFullName)
 	h.globalAction = gio.NewSimpleAction(globalShortcutActionName, glib.NewVariantType("s"))
 	if h.globalAction != nil {
 		h.globalActionCb = func(_ gio.SimpleAction, parameter uintptr) {
 		return false
 	}
-	if h.shortcutAction == nil {
+	// Create trigger for this key combination.
+	trigger := gtk.NewKeyvalTrigger(keyval, modifiers)
+	if trigger == nil {
 		logging.FromContext(h.ctx).Error().
 			Uint("keyval", keyval).
-			Msg("global shortcut action is unavailable")
+			Msg("failed to create keyval trigger")
 		return false
 	}
-	// Create trigger for this key combination.
+	// GtkShortcut takes ownership of its action. Each shortcut needs its own
-	trigger := gtk.NewKeyvalTrigger(keyval, modifiers)
+	// named action even though they all dispatch through the same window action.
-	if trigger == nil {
+	shortcutAction := gtk.NewNamedAction(globalShortcutActionFullName)
+	if shortcutAction == nil {
 		logging.FromContext(h.ctx).Error().
 			Uint("keyval", keyval).
-			Msg("failed to create keyval trigger")
+			Msg("failed to create global shortcut action")
 		return false
 	}
-	shortcut := gtk.NewShortcut(&trigger.ShortcutTrigger, &h.shortcutAction.ShortcutAction)
+	shortcut := gtk.NewShortcut(&trigger.ShortcutTrigger, &shortcutAction.ShortcutAction)
 	if shortcut == nil {
 		logging.FromContext(h.ctx).Error().
 			Uint("keyval", keyval).
 	}
 	h.controller = nil
 	h.releaseController = nil
-	h.shortcutAction = nil
 	h.globalAction = nil
 	h.globalActionCb = nil
 	h.globalActionHandlerID = 0

M internal/ui/input/global_shortcuts_test.go +66 -3

@@ -1,6 +1,7 @@
1	1		package input
2	2
3	3		import (
	4	+	"context"
4	5		"testing"
5	6		"time"
6	7

@@ -54,6 +55,71 @@ func TestGlobalShortcutHandlerCallbackBudgetIsIndependentOfShortcutCount(t *test
54	55		}
55	56		}
56	57
	58	+	func TestGlobalShortcutHandlerUsesDistinctNamedActionsAcrossReloads(t *testing.T) {
	59	+	if !gtk.InitCheck() {
	60	+	t.Skip("GTK native display prerequisite unavailable (gtk.InitCheck returned false)")
	61	+	}
	62	+	display := gdk.DisplayGetDefault()
	63	+	if display == nil {
	64	+	t.Skip("GTK native display prerequisite unavailable (no default GDK display)")
	65	+	}
	66	+	display.Unref()
	67	+
	68	+	appID := "com.dumber.GlobalShortcutHandlerTest"
	69	+	app := gtk.NewApplication(&appID, gio.GApplicationNonUniqueValue)
	70	+	if app == nil {
	71	+	t.Fatal("gtk application creation failed")
	72	+	}
	73	+	registered, err := app.Register(nil)
	74	+	if err != nil \|\| !registered {
	75	+	t.Fatalf("gtk application registration failed: registered=%t err=%v", registered, err)
	76	+	}
	77	+	window := gtk.NewApplicationWindow(app)
	78	+	if window == nil {
	79	+	t.Fatal("gtk application window creation failed")
	80	+	}
	81	+	h := NewGlobalShortcutHandler(context.Background(), window, nil, nil, nil, nil)
	82	+	if h == nil {
	83	+	t.Fatal("global shortcut handler creation failed")
	84	+	}
	85	+	t.Cleanup(func() {
	86	+	h.Detach()
	87	+	window.Destroy()
	88	+	app.Unref()
	89	+	})
	90	+
	91	+	assertDistinctShortcutActions := func() {
	92	+	t.Helper()
	93	+	shortcutCount := h.controller.GetNItems()
	94	+	if shortcutCount < 2 {
	95	+	t.Fatalf("shortcut count = %d, want at least 2", shortcutCount)
	96	+	}
	97	+	actions := make(map[uintptr]struct{}, shortcutCount)
	98	+	for index := uint(0); index < shortcutCount; index++ {
	99	+	shortcut := gtk.ShortcutNewFromInternalPtr(h.controller.GetItem(index))
	100	+	if shortcut == nil {
	101	+	t.Fatalf("shortcut %d was nil", index)
	102	+	}
	103	+	action := shortcut.GetAction()
	104	+	if action == nil {
	105	+	t.Fatalf("shortcut %d action was nil", index)
	106	+	}
	107	+	actions[action.GoPointer()] = struct{}{}
	108	+	action.Unref()
	109	+	shortcut.Unref()
	110	+	}
	111	+	if got := uint(len(actions)); got != shortcutCount {
	112	+	t.Fatalf("distinct shortcut actions = %d, want %d", got, shortcutCount)
	113	+	}
	114	+	}
	115	+
	116	+	assertDistinctShortcutActions()
	117	+	h.ReloadShortcuts(context.Background(), nil, nil)
	118	+	assertDistinctShortcutActions()
	119	+	h.ReloadShortcuts(context.Background(), nil, nil)
	120	+	assertDistinctShortcutActions()
	121	+	}
	122	+
57	123		func TestGlobalShortcutIDIncludesBinding(t *testing.T) {
58	124		first := encodeGlobalShortcutID(ActionSwitchTabIndex1, KeyBinding{Keyval: uint(gdk.KEY_1), Modifiers: Modifier(gdk.AltMaskValue)}, 0)
59	125		second := encodeGlobalShortcutID(ActionSwitchTabIndex1, KeyBinding{Keyval: uint(gdk.KEY_2), Modifiers: Modifier(gdk.AltMaskValue)}, 0)

@@ -174,9 +240,6 @@ func TestGlobalShortcutHandlerDetachForDestroyClearsRetainedState(t *testing.T)
174	240		if h.releaseController != nil {
175	241		t.Fatal("releaseController should be cleared during destroy detach")
176	242		}
177		-	if h.shortcutAction != nil {
178		-	t.Fatal("shortcutAction should be cleared during destroy detach")
179		-	}
180	243		if h.globalAction != nil {
181	244		t.Fatal("globalAction should be cleared during destroy detach")
182	245		}

@@ -1,6 +1,7 @@
1		package input	1		package input
2			2
3		import (	3		import (
			4	+	"context"
4		"testing"	5		"testing"
5		"time"	6		"time"
6			7

@@ -54,6 +55,71 @@ func TestGlobalShortcutHandlerCallbackBudgetIsIndependentOfShortcutCount(t *test
54		}	55		}
55		}	56		}
56			57
			58	+	func TestGlobalShortcutHandlerUsesDistinctNamedActionsAcrossReloads(t *testing.T) {
			59	+	if !gtk.InitCheck() {
			60	+	t.Skip("GTK native display prerequisite unavailable (gtk.InitCheck returned false)")
			61	+	}
			62	+	display := gdk.DisplayGetDefault()
			63	+	if display == nil {
			64	+	t.Skip("GTK native display prerequisite unavailable (no default GDK display)")
			65	+	}
			66	+	display.Unref()
			67	+
			68	+	appID := "com.dumber.GlobalShortcutHandlerTest"
			69	+	app := gtk.NewApplication(&appID, gio.GApplicationNonUniqueValue)
			70	+	if app == nil {
			71	+	t.Fatal("gtk application creation failed")
			72	+	}
			73	+	registered, err := app.Register(nil)
			74	+	if err != nil \|\| !registered {
			75	+	t.Fatalf("gtk application registration failed: registered=%t err=%v", registered, err)
			76	+	}
			77	+	window := gtk.NewApplicationWindow(app)
			78	+	if window == nil {
			79	+	t.Fatal("gtk application window creation failed")
			80	+	}
			81	+	h := NewGlobalShortcutHandler(context.Background(), window, nil, nil, nil, nil)
			82	+	if h == nil {
			83	+	t.Fatal("global shortcut handler creation failed")
			84	+	}
			85	+	t.Cleanup(func() {
			86	+	h.Detach()
			87	+	window.Destroy()
			88	+	app.Unref()
			89	+	})
			90	+
			91	+	assertDistinctShortcutActions := func() {
			92	+	t.Helper()
			93	+	shortcutCount := h.controller.GetNItems()
			94	+	if shortcutCount < 2 {
			95	+	t.Fatalf("shortcut count = %d, want at least 2", shortcutCount)
			96	+	}
			97	+	actions := make(map[uintptr]struct{}, shortcutCount)
			98	+	for index := uint(0); index < shortcutCount; index++ {
			99	+	shortcut := gtk.ShortcutNewFromInternalPtr(h.controller.GetItem(index))
			100	+	if shortcut == nil {
			101	+	t.Fatalf("shortcut %d was nil", index)
			102	+	}
			103	+	action := shortcut.GetAction()
			104	+	if action == nil {
			105	+	t.Fatalf("shortcut %d action was nil", index)
			106	+	}
			107	+	actions[action.GoPointer()] = struct{}{}
			108	+	action.Unref()
			109	+	shortcut.Unref()
			110	+	}
			111	+	if got := uint(len(actions)); got != shortcutCount {
			112	+	t.Fatalf("distinct shortcut actions = %d, want %d", got, shortcutCount)
			113	+	}
			114	+	}
			115	+
			116	+	assertDistinctShortcutActions()
			117	+	h.ReloadShortcuts(context.Background(), nil, nil)
			118	+	assertDistinctShortcutActions()
			119	+	h.ReloadShortcuts(context.Background(), nil, nil)
			120	+	assertDistinctShortcutActions()
			121	+	}
			122	+
57		func TestGlobalShortcutIDIncludesBinding(t *testing.T) {	123		func TestGlobalShortcutIDIncludesBinding(t *testing.T) {
58		first := encodeGlobalShortcutID(ActionSwitchTabIndex1, KeyBinding{Keyval: uint(gdk.KEY_1), Modifiers: Modifier(gdk.AltMaskValue)}, 0)	124		first := encodeGlobalShortcutID(ActionSwitchTabIndex1, KeyBinding{Keyval: uint(gdk.KEY_1), Modifiers: Modifier(gdk.AltMaskValue)}, 0)
59		second := encodeGlobalShortcutID(ActionSwitchTabIndex1, KeyBinding{Keyval: uint(gdk.KEY_2), Modifiers: Modifier(gdk.AltMaskValue)}, 0)	125		second := encodeGlobalShortcutID(ActionSwitchTabIndex1, KeyBinding{Keyval: uint(gdk.KEY_2), Modifiers: Modifier(gdk.AltMaskValue)}, 0)

@@ -174,9 +240,6 @@ func TestGlobalShortcutHandlerDetachForDestroyClearsRetainedState(t *testing.T)
174		if h.releaseController != nil {	240		if h.releaseController != nil {
175		t.Fatal("releaseController should be cleared during destroy detach")	241		t.Fatal("releaseController should be cleared during destroy detach")
176		}	242		}
177	-	if h.shortcutAction != nil {
178	-	t.Fatal("shortcutAction should be cleared during destroy detach")
179	-	}
180		if h.globalAction != nil {	243		if h.globalAction != nil {
181		t.Fatal("globalAction should be cleared during destroy detach")	244		t.Fatal("globalAction should be cleared during destroy detach")
182		}	245		}