fix(ui): avoid controller teardown during destroy
- commit
- 0337582b219fde6dbbd4dbcdf9d55cc0015e564b
- author
- brice · · 2026-06-11 07:32 UTC
- parent
- 4b8e76f
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +145 -17
M internal/ui/app.go +6 -2
| @@ -2838,10 +2838,14 @@ func (a *App) pruneStaleBrowserWindows(runtimeWindows []*browserWindow) { | |||
| 2838 | 2838 | } | |
| 2839 | 2839 | for id, bw := range a.browserWindows { | |
| 2840 | 2840 | if !runtimeSet[id] { | |
| 2841 | - | if bw != nil && bw.mainWindow != nil { | |
| 2842 | - | bw.mainWindow.Destroy() | |
| 2841 | + | var mainWindowToDestroy *window.MainWindow | |
| 2842 | + | if bw != nil { | |
| 2843 | + | mainWindowToDestroy = bw.mainWindow | |
| 2843 | 2844 | } | |
| 2844 | 2845 | a.removeBrowserWindow(id) | |
| 2846 | + | if mainWindowToDestroy != nil { | |
| 2847 | + | mainWindowToDestroy.Destroy() | |
| 2848 | + | } | |
| 2845 | 2849 | } | |
| 2846 | 2850 | } | |
| 2847 | 2851 | ||
| @@ -2838,10 +2838,14 @@ func (a *App) pruneStaleBrowserWindows(runtimeWindows []*browserWindow) { | |||||
| 2838 | } | 2838 | } | ||
| 2839 | for id, bw := range a.browserWindows { | 2839 | for id, bw := range a.browserWindows { | ||
| 2840 | if !runtimeSet[id] { | 2840 | if !runtimeSet[id] { | ||
| 2841 | - | if bw != nil && bw.mainWindow != nil { | 2841 | + | var mainWindowToDestroy *window.MainWindow |
| 2842 | - | bw.mainWindow.Destroy() | 2842 | + | if bw != nil { |
| 2843 | + | mainWindowToDestroy = bw.mainWindow | |||
| 2843 | } | 2844 | } | ||
| 2844 | a.removeBrowserWindow(id) | 2845 | a.removeBrowserWindow(id) | ||
| 2846 | + | if mainWindowToDestroy != nil { | |||
| 2847 | + | mainWindowToDestroy.Destroy() | |||
| 2848 | + | } | |||
| 2845 | } | 2849 | } | ||
| 2846 | } | 2850 | } | ||
| 2847 | 2851 | ||||
M internal/ui/browser_window.go +4 -4
| @@ -39,15 +39,15 @@ type browserWindow struct { | |||
| 39 | 39 | webrtcIndicator *component.WebRTCPermissionIndicator | |
| 40 | 40 | } | |
| 41 | 41 | ||
| 42 | - | func (bw *browserWindow) detachInput() { | |
| 42 | + | func (bw *browserWindow) detachInputForDestroy() { | |
| 43 | 43 | if bw == nil { | |
| 44 | 44 | return | |
| 45 | 45 | } | |
| 46 | 46 | if bw.keyboardHandler != nil { | |
| 47 | - | bw.keyboardHandler.Detach() | |
| 47 | + | bw.keyboardHandler.DetachForDestroy() | |
| 48 | 48 | } | |
| 49 | 49 | if bw.globalShortcutHandler != nil { | |
| 50 | - | bw.globalShortcutHandler.Detach() | |
| 50 | + | bw.globalShortcutHandler.DetachForDestroy() | |
| 51 | 51 | } | |
| 52 | 52 | } | |
| 53 | 53 | ||
| @@ -299,7 +299,7 @@ func (a *App) removeBrowserWindow(id string) { | |||
| 299 | 299 | } | |
| 300 | 300 | } | |
| 301 | 301 | if removed != nil { | |
| 302 | - | removed.detachInput() | |
| 302 | + | removed.detachInputForDestroy() | |
| 303 | 303 | removed.clearShellState() | |
| 304 | 304 | } | |
| 305 | 305 | if a.contentCoord != nil { | |
| @@ -39,15 +39,15 @@ type browserWindow struct { | |||||
| 39 | webrtcIndicator *component.WebRTCPermissionIndicator | 39 | webrtcIndicator *component.WebRTCPermissionIndicator | ||
| 40 | } | 40 | } | ||
| 41 | 41 | ||||
| 42 | - | func (bw *browserWindow) detachInput() { | 42 | + | func (bw *browserWindow) detachInputForDestroy() { |
| 43 | if bw == nil { | 43 | if bw == nil { | ||
| 44 | return | 44 | return | ||
| 45 | } | 45 | } | ||
| 46 | if bw.keyboardHandler != nil { | 46 | if bw.keyboardHandler != nil { | ||
| 47 | - | bw.keyboardHandler.Detach() | 47 | + | bw.keyboardHandler.DetachForDestroy() |
| 48 | } | 48 | } | ||
| 49 | if bw.globalShortcutHandler != nil { | 49 | if bw.globalShortcutHandler != nil { | ||
| 50 | - | bw.globalShortcutHandler.Detach() | 50 | + | bw.globalShortcutHandler.DetachForDestroy() |
| 51 | } | 51 | } | ||
| 52 | } | 52 | } | ||
| 53 | 53 | ||||
| @@ -299,7 +299,7 @@ func (a *App) removeBrowserWindow(id string) { | |||||
| 299 | } | 299 | } | ||
| 300 | } | 300 | } | ||
| 301 | if removed != nil { | 301 | if removed != nil { | ||
| 302 | - | removed.detachInput() | 302 | + | removed.detachInputForDestroy() |
| 303 | removed.clearShellState() | 303 | removed.clearShellState() | ||
| 304 | } | 304 | } | ||
| 305 | if a.contentCoord != nil { | 305 | if a.contentCoord != nil { | ||
M internal/ui/input/gesture.go +14 -3
| @@ -104,7 +104,7 @@ func (h *GestureHandler) AttachTo(widget *gtk.Widget) { | |||
| 104 | 104 | h.pressedHandlerID = h.clickGesture.ConnectPressed(&h.pressedCb) | |
| 105 | 105 | h.widget = widget | |
| 106 | 106 | h.destroyCb = func(_ gtk.Widget) { | |
| 107 | - | h.Detach() | |
| 107 | + | h.DetachForDestroy() | |
| 108 | 108 | } | |
| 109 | 109 | h.destroyHandlerID = widget.ConnectDestroy(&h.destroyCb) | |
| 110 | 110 | ||
| @@ -182,8 +182,19 @@ func (h *GestureHandler) detachExistingAttachment() { | |||
| 182 | 182 | } | |
| 183 | 183 | } | |
| 184 | 184 | ||
| 185 | - | // Detach removes the gesture handler and disconnects its GTK signals. | |
| 185 | + | // Detach removes the gesture handler from a live widget and disconnects its | |
| 186 | + | // GTK signals. | |
| 186 | 187 | func (h *GestureHandler) Detach() { | |
| 188 | + | h.detach(true) | |
| 189 | + | } | |
| 190 | + | ||
| 191 | + | // DetachForDestroy disconnects retained GTK callbacks during widget teardown | |
| 192 | + | // without mutating the widget's controller list while GTK is destroying it. | |
| 193 | + | func (h *GestureHandler) DetachForDestroy() { | |
| 194 | + | h.detach(false) | |
| 195 | + | } | |
| 196 | + | ||
| 197 | + | func (h *GestureHandler) detach(removeController bool) { | |
| 187 | 198 | if h == nil { | |
| 188 | 199 | return | |
| 189 | 200 | } | |
| @@ -209,7 +220,7 @@ func (h *GestureHandler) Detach() { | |||
| 209 | 220 | if widget != nil && destroyHandlerID != 0 { | |
| 210 | 221 | widget.DisconnectSignal(destroyHandlerID) | |
| 211 | 222 | } | |
| 212 | - | if widget != nil && clickGesture != nil { | |
| 223 | + | if removeController && widget != nil && clickGesture != nil { | |
| 213 | 224 | widget.RemoveController(&clickGesture.EventController) | |
| 214 | 225 | } | |
| 215 | 226 | } | |
| @@ -104,7 +104,7 @@ func (h *GestureHandler) AttachTo(widget *gtk.Widget) { | |||||
| 104 | h.pressedHandlerID = h.clickGesture.ConnectPressed(&h.pressedCb) | 104 | h.pressedHandlerID = h.clickGesture.ConnectPressed(&h.pressedCb) | ||
| 105 | h.widget = widget | 105 | h.widget = widget | ||
| 106 | h.destroyCb = func(_ gtk.Widget) { | 106 | h.destroyCb = func(_ gtk.Widget) { | ||
| 107 | - | h.Detach() | 107 | + | h.DetachForDestroy() |
| 108 | } | 108 | } | ||
| 109 | h.destroyHandlerID = widget.ConnectDestroy(&h.destroyCb) | 109 | h.destroyHandlerID = widget.ConnectDestroy(&h.destroyCb) | ||
| 110 | 110 | ||||
| @@ -182,8 +182,19 @@ func (h *GestureHandler) detachExistingAttachment() { | |||||
| 182 | } | 182 | } | ||
| 183 | } | 183 | } | ||
| 184 | 184 | ||||
| 185 | - | // Detach removes the gesture handler and disconnects its GTK signals. | 185 | + | // Detach removes the gesture handler from a live widget and disconnects its |
| 186 | + | // GTK signals. | |||
| 186 | func (h *GestureHandler) Detach() { | 187 | func (h *GestureHandler) Detach() { | ||
| 188 | + | h.detach(true) | |||
| 189 | + | } | |||
| 190 | + | ||||
| 191 | + | // DetachForDestroy disconnects retained GTK callbacks during widget teardown | |||
| 192 | + | // without mutating the widget's controller list while GTK is destroying it. | |||
| 193 | + | func (h *GestureHandler) DetachForDestroy() { | |||
| 194 | + | h.detach(false) | |||
| 195 | + | } | |||
| 196 | + | ||||
| 197 | + | func (h *GestureHandler) detach(removeController bool) { | |||
| 187 | if h == nil { | 198 | if h == nil { | ||
| 188 | return | 199 | return | ||
| 189 | } | 200 | } | ||
| @@ -209,7 +220,7 @@ func (h *GestureHandler) Detach() { | |||||
| 209 | if widget != nil && destroyHandlerID != 0 { | 220 | if widget != nil && destroyHandlerID != 0 { | ||
| 210 | widget.DisconnectSignal(destroyHandlerID) | 221 | widget.DisconnectSignal(destroyHandlerID) | ||
| 211 | } | 222 | } | ||
| 212 | - | if widget != nil && clickGesture != nil { | 223 | + | if removeController && widget != nil && clickGesture != nil { |
| 213 | widget.RemoveController(&clickGesture.EventController) | 224 | widget.RemoveController(&clickGesture.EventController) | ||
| 214 | } | 225 | } | ||
| 215 | } | 226 | } | ||
M internal/ui/input/gesture_test.go +19 -0
| @@ -50,3 +50,22 @@ func TestGestureHandlerDetachClearsRetainedCallbacksAndState(t *testing.T) { | |||
| 50 | 50 | assert.Nil(t, h.onAction) | |
| 51 | 51 | assert.Nil(t, h.navigator) | |
| 52 | 52 | } | |
| 53 | + | ||
| 54 | + | func TestGestureHandlerDetachForDestroyClearsRetainedCallbacksAndState(t *testing.T) { | |
| 55 | + | h := NewGestureHandler(context.Background()) | |
| 56 | + | h.pressedCb = func(gtk.GestureClick, int, float64, float64) {} | |
| 57 | + | h.destroyCb = func(gtk.Widget) {} | |
| 58 | + | h.pressedHandlerID = 1 | |
| 59 | + | h.destroyHandlerID = 2 | |
| 60 | + | h.onAction = func(context.Context, Action) error { return nil } | |
| 61 | + | h.navigator = testGestureNavigator{} | |
| 62 | + | ||
| 63 | + | h.DetachForDestroy() | |
| 64 | + | ||
| 65 | + | assert.Nil(t, h.pressedCb) | |
| 66 | + | assert.Nil(t, h.destroyCb) | |
| 67 | + | assert.Zero(t, h.pressedHandlerID) | |
| 68 | + | assert.Zero(t, h.destroyHandlerID) | |
| 69 | + | assert.Nil(t, h.onAction) | |
| 70 | + | assert.Nil(t, h.navigator) | |
| 71 | + | } | |
| @@ -50,3 +50,22 @@ func TestGestureHandlerDetachClearsRetainedCallbacksAndState(t *testing.T) { | |||||
| 50 | assert.Nil(t, h.onAction) | 50 | assert.Nil(t, h.onAction) | ||
| 51 | assert.Nil(t, h.navigator) | 51 | assert.Nil(t, h.navigator) | ||
| 52 | } | 52 | } | ||
| 53 | + | ||||
| 54 | + | func TestGestureHandlerDetachForDestroyClearsRetainedCallbacksAndState(t *testing.T) { | |||
| 55 | + | h := NewGestureHandler(context.Background()) | |||
| 56 | + | h.pressedCb = func(gtk.GestureClick, int, float64, float64) {} | |||
| 57 | + | h.destroyCb = func(gtk.Widget) {} | |||
| 58 | + | h.pressedHandlerID = 1 | |||
| 59 | + | h.destroyHandlerID = 2 | |||
| 60 | + | h.onAction = func(context.Context, Action) error { return nil } | |||
| 61 | + | h.navigator = testGestureNavigator{} | |||
| 62 | + | ||||
| 63 | + | h.DetachForDestroy() | |||
| 64 | + | ||||
| 65 | + | assert.Nil(t, h.pressedCb) | |||
| 66 | + | assert.Nil(t, h.destroyCb) | |||
| 67 | + | assert.Zero(t, h.pressedHandlerID) | |||
| 68 | + | assert.Zero(t, h.destroyHandlerID) | |||
| 69 | + | assert.Nil(t, h.onAction) | |||
| 70 | + | assert.Nil(t, h.navigator) | |||
| 71 | + | } | |||
M internal/ui/input/global_shortcuts.go +16 -6
| @@ -579,24 +579,34 @@ func (h *GlobalShortcutHandler) ReloadShortcuts(ctx context.Context, workspace * | |||
| 579 | 579 | Msg("global shortcuts reloaded") | |
| 580 | 580 | } | |
| 581 | 581 | ||
| 582 | - | // Detach removes the global shortcut handler from the window. | |
| 583 | - | // Note: GTK handles cleanup when the widget is destroyed, | |
| 584 | - | // but we clear our references here. | |
| 582 | + | // Detach removes the global shortcut handler from a live GTK window. | |
| 583 | + | // Use DetachForDestroy when the window is already closing or being destroyed. | |
| 585 | 584 | func (h *GlobalShortcutHandler) Detach() { | |
| 585 | + | h.detach(true) | |
| 586 | + | } | |
| 587 | + | ||
| 588 | + | // DetachForDestroy releases retained GTK callbacks during window teardown | |
| 589 | + | // without mutating the window's controller or action lists while GTK is | |
| 590 | + | // destroying them. | |
| 591 | + | func (h *GlobalShortcutHandler) DetachForDestroy() { | |
| 592 | + | h.detach(false) | |
| 593 | + | } | |
| 594 | + | ||
| 595 | + | func (h *GlobalShortcutHandler) detach(removeFromWindow bool) { | |
| 586 | 596 | h.generation++ | |
| 587 | - | if h.window != nil && h.controller != nil { | |
| 597 | + | if removeFromWindow && h.window != nil && h.controller != nil { | |
| 588 | 598 | h.window.RemoveController(&h.controller.EventController) | |
| 589 | 599 | } | |
| 590 | 600 | if h.releaseController != nil && h.keyReleasedHandlerID != 0 { | |
| 591 | 601 | h.releaseController.DisconnectSignal(h.keyReleasedHandlerID) | |
| 592 | 602 | } | |
| 593 | - | if h.window != nil && h.releaseController != nil { | |
| 603 | + | if removeFromWindow && h.window != nil && h.releaseController != nil { | |
| 594 | 604 | h.window.RemoveController(&h.releaseController.EventController) | |
| 595 | 605 | } | |
| 596 | 606 | if h.globalAction != nil && h.globalActionHandlerID != 0 { | |
| 597 | 607 | h.globalAction.DisconnectSignal(h.globalActionHandlerID) | |
| 598 | 608 | } | |
| 599 | - | if h.window != nil && h.globalAction != nil { | |
| 609 | + | if removeFromWindow && h.window != nil && h.globalAction != nil { | |
| 600 | 610 | h.window.RemoveAction(globalShortcutActionName) | |
| 601 | 611 | } | |
| 602 | 612 | h.controller = nil | |
| @@ -579,24 +579,34 @@ func (h *GlobalShortcutHandler) ReloadShortcuts(ctx context.Context, workspace * | |||||
| 579 | Msg("global shortcuts reloaded") | 579 | Msg("global shortcuts reloaded") | ||
| 580 | } | 580 | } | ||
| 581 | 581 | ||||
| 582 | - | // Detach removes the global shortcut handler from the window. | 582 | + | // Detach removes the global shortcut handler from a live GTK window. |
| 583 | - | // Note: GTK handles cleanup when the widget is destroyed, | 583 | + | // Use DetachForDestroy when the window is already closing or being destroyed. |
| 584 | - | // but we clear our references here. | |||
| 585 | func (h *GlobalShortcutHandler) Detach() { | 584 | func (h *GlobalShortcutHandler) Detach() { | ||
| 585 | + | h.detach(true) | |||
| 586 | + | } | |||
| 587 | + | ||||
| 588 | + | // DetachForDestroy releases retained GTK callbacks during window teardown | |||
| 589 | + | // without mutating the window's controller or action lists while GTK is | |||
| 590 | + | // destroying them. | |||
| 591 | + | func (h *GlobalShortcutHandler) DetachForDestroy() { | |||
| 592 | + | h.detach(false) | |||
| 593 | + | } | |||
| 594 | + | ||||
| 595 | + | func (h *GlobalShortcutHandler) detach(removeFromWindow bool) { | |||
| 586 | h.generation++ | 596 | h.generation++ | ||
| 587 | - | if h.window != nil && h.controller != nil { | 597 | + | if removeFromWindow && h.window != nil && h.controller != nil { |
| 588 | h.window.RemoveController(&h.controller.EventController) | 598 | h.window.RemoveController(&h.controller.EventController) | ||
| 589 | } | 599 | } | ||
| 590 | if h.releaseController != nil && h.keyReleasedHandlerID != 0 { | 600 | if h.releaseController != nil && h.keyReleasedHandlerID != 0 { | ||
| 591 | h.releaseController.DisconnectSignal(h.keyReleasedHandlerID) | 601 | h.releaseController.DisconnectSignal(h.keyReleasedHandlerID) | ||
| 592 | } | 602 | } | ||
| 593 | - | if h.window != nil && h.releaseController != nil { | 603 | + | if removeFromWindow && h.window != nil && h.releaseController != nil { |
| 594 | h.window.RemoveController(&h.releaseController.EventController) | 604 | h.window.RemoveController(&h.releaseController.EventController) | ||
| 595 | } | 605 | } | ||
| 596 | if h.globalAction != nil && h.globalActionHandlerID != 0 { | 606 | if h.globalAction != nil && h.globalActionHandlerID != 0 { | ||
| 597 | h.globalAction.DisconnectSignal(h.globalActionHandlerID) | 607 | h.globalAction.DisconnectSignal(h.globalActionHandlerID) | ||
| 598 | } | 608 | } | ||
| 599 | - | if h.window != nil && h.globalAction != nil { | 609 | + | if removeFromWindow && h.window != nil && h.globalAction != nil { |
| 600 | h.window.RemoveAction(globalShortcutActionName) | 610 | h.window.RemoveAction(globalShortcutActionName) | ||
| 601 | } | 611 | } | ||
| 602 | h.controller = nil | 612 | h.controller = nil | ||
M internal/ui/input/global_shortcuts_test.go +52 -0
| @@ -133,6 +133,58 @@ func TestGlobalShortcutHandlerSuppressesOneShotActionsAfterDetach(t *testing.T) | |||
| 133 | 133 | } | |
| 134 | 134 | } | |
| 135 | 135 | ||
| 136 | + | func TestGlobalShortcutHandlerDetachForDestroyClearsRetainedState(t *testing.T) { | |
| 137 | + | h := &GlobalShortcutHandler{ | |
| 138 | + | registered: make(map[KeyBinding]Action), | |
| 139 | + | shortcutRefs: make(map[string]globalShortcutRegistration), | |
| 140 | + | lastDispatchAt: make(map[Action]time.Time), | |
| 141 | + | heldShortcuts: make(map[globalShortcutHoldKey]struct{}), | |
| 142 | + | globalActionCb: func(_ gio.SimpleAction, _ uintptr) {}, | |
| 143 | + | globalActionHandlerID: 1, | |
| 144 | + | keyReleasedCb: func(_ gtk.EventControllerKey, _ uint, _ uint, _ gdk.ModifierType) {}, | |
| 145 | + | keyReleasedHandlerID: 2, | |
| 146 | + | } | |
| 147 | + | ||
| 148 | + | h.DetachForDestroy() | |
| 149 | + | ||
| 150 | + | if h.controller != nil { | |
| 151 | + | t.Fatal("controller should be cleared during destroy detach") | |
| 152 | + | } | |
| 153 | + | if h.releaseController != nil { | |
| 154 | + | t.Fatal("releaseController should be cleared during destroy detach") | |
| 155 | + | } | |
| 156 | + | if h.shortcutAction != nil { | |
| 157 | + | t.Fatal("shortcutAction should be cleared during destroy detach") | |
| 158 | + | } | |
| 159 | + | if h.globalAction != nil { | |
| 160 | + | t.Fatal("globalAction should be cleared during destroy detach") | |
| 161 | + | } | |
| 162 | + | if h.globalActionCb != nil { | |
| 163 | + | t.Fatal("globalActionCb should be cleared during destroy detach") | |
| 164 | + | } | |
| 165 | + | if h.keyReleasedCb != nil { | |
| 166 | + | t.Fatal("keyReleasedCb should be cleared during destroy detach") | |
| 167 | + | } | |
| 168 | + | if h.globalActionHandlerID != 0 { | |
| 169 | + | t.Fatal("globalActionHandlerID should be reset during destroy detach") | |
| 170 | + | } | |
| 171 | + | if h.keyReleasedHandlerID != 0 { | |
| 172 | + | t.Fatal("keyReleasedHandlerID should be reset during destroy detach") | |
| 173 | + | } | |
| 174 | + | if h.registered != nil { | |
| 175 | + | t.Fatal("registered shortcuts should be cleared during destroy detach") | |
| 176 | + | } | |
| 177 | + | if h.shortcutRefs != nil { | |
| 178 | + | t.Fatal("shortcut refs should be cleared during destroy detach") | |
| 179 | + | } | |
| 180 | + | if h.lastDispatchAt != nil { | |
| 181 | + | t.Fatal("lastDispatchAt should be cleared during destroy detach") | |
| 182 | + | } | |
| 183 | + | if h.heldShortcuts != nil { | |
| 184 | + | t.Fatal("heldShortcuts should be cleared during destroy detach") | |
| 185 | + | } | |
| 186 | + | } | |
| 187 | + | ||
| 136 | 188 | func TestGlobalShortcutHandlerGenerationMarksOldCallbacksStale(t *testing.T) { | |
| 137 | 189 | h := &GlobalShortcutHandler{} | |
| 138 | 190 | generation := h.generation | |
| @@ -133,6 +133,58 @@ func TestGlobalShortcutHandlerSuppressesOneShotActionsAfterDetach(t *testing.T) | |||||
| 133 | } | 133 | } | ||
| 134 | } | 134 | } | ||
| 135 | 135 | ||||
| 136 | + | func TestGlobalShortcutHandlerDetachForDestroyClearsRetainedState(t *testing.T) { | |||
| 137 | + | h := &GlobalShortcutHandler{ | |||
| 138 | + | registered: make(map[KeyBinding]Action), | |||
| 139 | + | shortcutRefs: make(map[string]globalShortcutRegistration), | |||
| 140 | + | lastDispatchAt: make(map[Action]time.Time), | |||
| 141 | + | heldShortcuts: make(map[globalShortcutHoldKey]struct{}), | |||
| 142 | + | globalActionCb: func(_ gio.SimpleAction, _ uintptr) {}, | |||
| 143 | + | globalActionHandlerID: 1, | |||
| 144 | + | keyReleasedCb: func(_ gtk.EventControllerKey, _ uint, _ uint, _ gdk.ModifierType) {}, | |||
| 145 | + | keyReleasedHandlerID: 2, | |||
| 146 | + | } | |||
| 147 | + | ||||
| 148 | + | h.DetachForDestroy() | |||
| 149 | + | ||||
| 150 | + | if h.controller != nil { | |||
| 151 | + | t.Fatal("controller should be cleared during destroy detach") | |||
| 152 | + | } | |||
| 153 | + | if h.releaseController != nil { | |||
| 154 | + | t.Fatal("releaseController should be cleared during destroy detach") | |||
| 155 | + | } | |||
| 156 | + | if h.shortcutAction != nil { | |||
| 157 | + | t.Fatal("shortcutAction should be cleared during destroy detach") | |||
| 158 | + | } | |||
| 159 | + | if h.globalAction != nil { | |||
| 160 | + | t.Fatal("globalAction should be cleared during destroy detach") | |||
| 161 | + | } | |||
| 162 | + | if h.globalActionCb != nil { | |||
| 163 | + | t.Fatal("globalActionCb should be cleared during destroy detach") | |||
| 164 | + | } | |||
| 165 | + | if h.keyReleasedCb != nil { | |||
| 166 | + | t.Fatal("keyReleasedCb should be cleared during destroy detach") | |||
| 167 | + | } | |||
| 168 | + | if h.globalActionHandlerID != 0 { | |||
| 169 | + | t.Fatal("globalActionHandlerID should be reset during destroy detach") | |||
| 170 | + | } | |||
| 171 | + | if h.keyReleasedHandlerID != 0 { | |||
| 172 | + | t.Fatal("keyReleasedHandlerID should be reset during destroy detach") | |||
| 173 | + | } | |||
| 174 | + | if h.registered != nil { | |||
| 175 | + | t.Fatal("registered shortcuts should be cleared during destroy detach") | |||
| 176 | + | } | |||
| 177 | + | if h.shortcutRefs != nil { | |||
| 178 | + | t.Fatal("shortcut refs should be cleared during destroy detach") | |||
| 179 | + | } | |||
| 180 | + | if h.lastDispatchAt != nil { | |||
| 181 | + | t.Fatal("lastDispatchAt should be cleared during destroy detach") | |||
| 182 | + | } | |||
| 183 | + | if h.heldShortcuts != nil { | |||
| 184 | + | t.Fatal("heldShortcuts should be cleared during destroy detach") | |||
| 185 | + | } | |||
| 186 | + | } | |||
| 187 | + | ||||
| 136 | func TestGlobalShortcutHandlerGenerationMarksOldCallbacksStale(t *testing.T) { | 188 | func TestGlobalShortcutHandlerGenerationMarksOldCallbacksStale(t *testing.T) { | ||
| 137 | h := &GlobalShortcutHandler{} | 189 | h := &GlobalShortcutHandler{} | ||
| 138 | generation := h.generation | 190 | generation := h.generation | ||
M internal/ui/input/keyboard.go +14 -2
| @@ -243,8 +243,20 @@ func (h *KeyboardHandler) AttachTo(window *gtk.ApplicationWindow) { | |||
| 243 | 243 | log.Debug().Msg("keyboard handler attached to window") | |
| 244 | 244 | } | |
| 245 | 245 | ||
| 246 | - | // Detach removes the keyboard handler and releases its GTK signal callbacks. | |
| 246 | + | // Detach removes the keyboard handler from a live GTK window and releases its | |
| 247 | + | // retained GTK signal callbacks. | |
| 247 | 248 | func (h *KeyboardHandler) Detach() { | |
| 249 | + | h.detach(true) | |
| 250 | + | } | |
| 251 | + | ||
| 252 | + | // DetachForDestroy releases retained GTK signal callbacks during window | |
| 253 | + | // teardown without mutating the window's controller list while GTK is | |
| 254 | + | // destroying it. | |
| 255 | + | func (h *KeyboardHandler) DetachForDestroy() { | |
| 256 | + | h.detach(false) | |
| 257 | + | } | |
| 258 | + | ||
| 259 | + | func (h *KeyboardHandler) detach(removeController bool) { | |
| 248 | 260 | h.mu.Lock() | |
| 249 | 261 | defer h.mu.Unlock() | |
| 250 | 262 | if h.controller != nil && h.keyPressedHandlerID != 0 { | |
| @@ -253,7 +265,7 @@ func (h *KeyboardHandler) Detach() { | |||
| 253 | 265 | if h.controller != nil && h.keyReleasedHandlerID != 0 { | |
| 254 | 266 | h.controller.DisconnectSignal(h.keyReleasedHandlerID) | |
| 255 | 267 | } | |
| 256 | - | if h.window != nil && h.controller != nil { | |
| 268 | + | if removeController && h.window != nil && h.controller != nil { | |
| 257 | 269 | h.window.RemoveController(&h.controller.EventController) | |
| 258 | 270 | } | |
| 259 | 271 | h.controller = nil | |
| @@ -243,8 +243,20 @@ func (h *KeyboardHandler) AttachTo(window *gtk.ApplicationWindow) { | |||||
| 243 | log.Debug().Msg("keyboard handler attached to window") | 243 | log.Debug().Msg("keyboard handler attached to window") | ||
| 244 | } | 244 | } | ||
| 245 | 245 | ||||
| 246 | - | // Detach removes the keyboard handler and releases its GTK signal callbacks. | 246 | + | // Detach removes the keyboard handler from a live GTK window and releases its |
| 247 | + | // retained GTK signal callbacks. | |||
| 247 | func (h *KeyboardHandler) Detach() { | 248 | func (h *KeyboardHandler) Detach() { | ||
| 249 | + | h.detach(true) | |||
| 250 | + | } | |||
| 251 | + | ||||
| 252 | + | // DetachForDestroy releases retained GTK signal callbacks during window | |||
| 253 | + | // teardown without mutating the window's controller list while GTK is | |||
| 254 | + | // destroying it. | |||
| 255 | + | func (h *KeyboardHandler) DetachForDestroy() { | |||
| 256 | + | h.detach(false) | |||
| 257 | + | } | |||
| 258 | + | ||||
| 259 | + | func (h *KeyboardHandler) detach(removeController bool) { | |||
| 248 | h.mu.Lock() | 260 | h.mu.Lock() | ||
| 249 | defer h.mu.Unlock() | 261 | defer h.mu.Unlock() | ||
| 250 | if h.controller != nil && h.keyPressedHandlerID != 0 { | 262 | if h.controller != nil && h.keyPressedHandlerID != 0 { | ||
| @@ -253,7 +265,7 @@ func (h *KeyboardHandler) Detach() { | |||||
| 253 | if h.controller != nil && h.keyReleasedHandlerID != 0 { | 265 | if h.controller != nil && h.keyReleasedHandlerID != 0 { | ||
| 254 | h.controller.DisconnectSignal(h.keyReleasedHandlerID) | 266 | h.controller.DisconnectSignal(h.keyReleasedHandlerID) | ||
| 255 | } | 267 | } | ||
| 256 | - | if h.window != nil && h.controller != nil { | 268 | + | if removeController && h.window != nil && h.controller != nil { |
| 257 | h.window.RemoveController(&h.controller.EventController) | 269 | h.window.RemoveController(&h.controller.EventController) | ||
| 258 | } | 270 | } | ||
| 259 | h.controller = nil | 271 | h.controller = nil | ||
M internal/ui/input/keyboard_test.go +20 -0
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | ||
| 7 | 7 | "github.com/bnema/dumber/internal/domain/entity" | |
| 8 | 8 | "github.com/bnema/puregotk/v4/gdk" | |
| 9 | + | "github.com/bnema/puregotk/v4/gtk" | |
| 9 | 10 | "github.com/stretchr/testify/assert" | |
| 10 | 11 | ) | |
| 11 | 12 | ||
| @@ -231,6 +232,25 @@ func TestHandleKeyPress_NoRouteCallback_DefaultsToShortcuts(t *testing.T) { | |||
| 231 | 232 | assert.False(t, result, "unregistered key in ModeNormal returns false") | |
| 232 | 233 | } | |
| 233 | 234 | ||
| 235 | + | func TestKeyboardHandlerDetachForDestroyClearsRetainedCallbacksAndState(t *testing.T) { | |
| 236 | + | h := NewKeyboardHandler(context.Background(), newTestWorkspace(), newTestSession()) | |
| 237 | + | h.keyPressedCb = func(gtk.EventControllerKey, uint, uint, gdk.ModifierType) bool { return true } | |
| 238 | + | h.keyReleasedCb = func(gtk.EventControllerKey, uint, uint, gdk.ModifierType) {} | |
| 239 | + | h.keyPressedHandlerID = 1 | |
| 240 | + | h.keyReleasedHandlerID = 2 | |
| 241 | + | h.activePressedActions = map[Action]uint{ActionReload: 1} | |
| 242 | + | ||
| 243 | + | h.DetachForDestroy() | |
| 244 | + | ||
| 245 | + | assert.Nil(t, h.controller) | |
| 246 | + | assert.Nil(t, h.window) | |
| 247 | + | assert.Nil(t, h.keyPressedCb) | |
| 248 | + | assert.Nil(t, h.keyReleasedCb) | |
| 249 | + | assert.Zero(t, h.keyPressedHandlerID) | |
| 250 | + | assert.Zero(t, h.keyReleasedHandlerID) | |
| 251 | + | assert.Nil(t, h.activePressedActions) | |
| 252 | + | } | |
| 253 | + | ||
| 234 | 254 | func TestHandleKeyPress_SuppressesHeldHardwareFallbackTabSwitchUntilRelease(t *testing.T) { | |
| 235 | 255 | h := NewKeyboardHandler(context.Background(), newTestWorkspace(), newTestSession()) | |
| 236 | 256 | calls := 0 | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | 6 | ||||
| 7 | "github.com/bnema/dumber/internal/domain/entity" | 7 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 8 | "github.com/bnema/puregotk/v4/gdk" | 8 | "github.com/bnema/puregotk/v4/gdk" | ||
| 9 | + | "github.com/bnema/puregotk/v4/gtk" | |||
| 9 | "github.com/stretchr/testify/assert" | 10 | "github.com/stretchr/testify/assert" | ||
| 10 | ) | 11 | ) | ||
| 11 | 12 | ||||
| @@ -231,6 +232,25 @@ func TestHandleKeyPress_NoRouteCallback_DefaultsToShortcuts(t *testing.T) { | |||||
| 231 | assert.False(t, result, "unregistered key in ModeNormal returns false") | 232 | assert.False(t, result, "unregistered key in ModeNormal returns false") | ||
| 232 | } | 233 | } | ||
| 233 | 234 | ||||
| 235 | + | func TestKeyboardHandlerDetachForDestroyClearsRetainedCallbacksAndState(t *testing.T) { | |||
| 236 | + | h := NewKeyboardHandler(context.Background(), newTestWorkspace(), newTestSession()) | |||
| 237 | + | h.keyPressedCb = func(gtk.EventControllerKey, uint, uint, gdk.ModifierType) bool { return true } | |||
| 238 | + | h.keyReleasedCb = func(gtk.EventControllerKey, uint, uint, gdk.ModifierType) {} | |||
| 239 | + | h.keyPressedHandlerID = 1 | |||
| 240 | + | h.keyReleasedHandlerID = 2 | |||
| 241 | + | h.activePressedActions = map[Action]uint{ActionReload: 1} | |||
| 242 | + | ||||
| 243 | + | h.DetachForDestroy() | |||
| 244 | + | ||||
| 245 | + | assert.Nil(t, h.controller) | |||
| 246 | + | assert.Nil(t, h.window) | |||
| 247 | + | assert.Nil(t, h.keyPressedCb) | |||
| 248 | + | assert.Nil(t, h.keyReleasedCb) | |||
| 249 | + | assert.Zero(t, h.keyPressedHandlerID) | |||
| 250 | + | assert.Zero(t, h.keyReleasedHandlerID) | |||
| 251 | + | assert.Nil(t, h.activePressedActions) | |||
| 252 | + | } | |||
| 253 | + | ||||
| 234 | func TestHandleKeyPress_SuppressesHeldHardwareFallbackTabSwitchUntilRelease(t *testing.T) { | 254 | func TestHandleKeyPress_SuppressesHeldHardwareFallbackTabSwitchUntilRelease(t *testing.T) { | ||
| 235 | h := NewKeyboardHandler(context.Background(), newTestWorkspace(), newTestSession()) | 255 | h := NewKeyboardHandler(context.Background(), newTestWorkspace(), newTestSession()) | ||
| 236 | calls := 0 | 256 | calls := 0 | ||