Merge pull request #190 from bnema/refactor/ui-reliability
fix: UI reliability — favicon races, keyboard routing, accent picker
14 files · +598 -212
M internal/application/port/text_input.go +6 -0
| @@ -12,6 +12,12 @@ type TextInputTarget interface { | |||
| 12 | 12 | DeleteBeforeCursor(ctx context.Context, n int) error | |
| 13 | 13 | } | |
| 14 | 14 | ||
| 15 | + | // Focusable is optionally implemented by text input targets that can restore | |
| 16 | + | // focus after the accent picker closes. | |
| 17 | + | type Focusable interface { | |
| 18 | + | Focus(ctx context.Context) | |
| 19 | + | } | |
| 20 | + | ||
| 15 | 21 | // FocusedInputProvider tracks which text input target currently has focus. | |
| 16 | 22 | // This allows the accent picker to insert text into the correct input field. | |
| 17 | 23 | type FocusedInputProvider interface { | |
| @@ -12,6 +12,12 @@ type TextInputTarget interface { | |||||
| 12 | DeleteBeforeCursor(ctx context.Context, n int) error | 12 | DeleteBeforeCursor(ctx context.Context, n int) error | ||
| 13 | } | 13 | } | ||
| 14 | 14 | ||||
| 15 | + | // Focusable is optionally implemented by text input targets that can restore | |||
| 16 | + | // focus after the accent picker closes. | |||
| 17 | + | type Focusable interface { | |||
| 18 | + | Focus(ctx context.Context) | |||
| 19 | + | } | |||
| 20 | + | ||||
| 15 | // FocusedInputProvider tracks which text input target currently has focus. | 21 | // FocusedInputProvider tracks which text input target currently has focus. | ||
| 16 | // This allows the accent picker to insert text into the correct input field. | 22 | // This allows the accent picker to insert text into the correct input field. | ||
| 17 | type FocusedInputProvider interface { | 23 | type FocusedInputProvider interface { | ||
M internal/application/usecase/insert_accent.go +22 -6
| @@ -137,16 +137,23 @@ func (uc *InsertAccentUseCase) Cancel(ctx context.Context) { | |||
| 137 | 137 | log := logging.FromContext(ctx) | |
| 138 | 138 | ||
| 139 | 139 | uc.mu.Lock() | |
| 140 | - | defer uc.mu.Unlock() | |
| 140 | + | target := uc.targetInput | |
| 141 | + | wasVisible := uc.pickerVisible | |
| 142 | + | uc.cancelTimerLocked() | |
| 143 | + | if wasVisible { | |
| 144 | + | uc.accentPicker.Hide() | |
| 145 | + | uc.pickerVisible = false | |
| 146 | + | } | |
| 147 | + | uc.mu.Unlock() | |
| 141 | 148 | ||
| 142 | - | if uc.timer != nil || uc.pickerVisible { | |
| 149 | + | if wasVisible { | |
| 143 | 150 | log.Debug().Msg("canceling accent picker") | |
| 144 | 151 | } | |
| 145 | 152 | ||
| 146 | - | uc.cancelTimerLocked() | |
| 147 | - | if uc.pickerVisible { | |
| 148 | - | uc.accentPicker.Hide() | |
| 149 | - | uc.pickerVisible = false | |
| 153 | + | if wasVisible { | |
| 154 | + | if focusable, ok := target.(port.Focusable); ok { | |
| 155 | + | focusable.Focus(ctx) | |
| 156 | + | } | |
| 150 | 157 | } | |
| 151 | 158 | } | |
| 152 | 159 | ||
| @@ -249,6 +256,10 @@ func (uc *InsertAccentUseCase) onAccentSelected(ctx context.Context, accent rune | |||
| 249 | 256 | if err := target.InsertText(ctx, string(accent)); err != nil { | |
| 250 | 257 | log.Error().Err(err).Msg("failed to insert accent") | |
| 251 | 258 | } | |
| 259 | + | ||
| 260 | + | if focusable, ok := target.(port.Focusable); ok { | |
| 261 | + | focusable.Focus(ctx) | |
| 262 | + | } | |
| 252 | 263 | } | |
| 253 | 264 | ||
| 254 | 265 | // onPickerCanceled is called when the user cancels the picker (Escape). | |
| @@ -256,6 +267,7 @@ func (uc *InsertAccentUseCase) onPickerCanceled(ctx context.Context) { | |||
| 256 | 267 | log := logging.FromContext(ctx) | |
| 257 | 268 | ||
| 258 | 269 | uc.mu.Lock() | |
| 270 | + | target := uc.targetInput | |
| 259 | 271 | uc.pickerVisible = false | |
| 260 | 272 | uc.pressedChar = 0 | |
| 261 | 273 | uc.repeatCount = 0 | |
| @@ -264,6 +276,10 @@ func (uc *InsertAccentUseCase) onPickerCanceled(ctx context.Context) { | |||
| 264 | 276 | ||
| 265 | 277 | uc.accentPicker.Hide() | |
| 266 | 278 | ||
| 279 | + | if focusable, ok := target.(port.Focusable); ok { | |
| 280 | + | focusable.Focus(ctx) | |
| 281 | + | } | |
| 282 | + | ||
| 267 | 283 | log.Debug().Msg("accent picker canceled") | |
| 268 | 284 | } | |
| 269 | 285 | ||
| @@ -137,16 +137,23 @@ func (uc *InsertAccentUseCase) Cancel(ctx context.Context) { | |||||
| 137 | log := logging.FromContext(ctx) | 137 | log := logging.FromContext(ctx) | ||
| 138 | 138 | ||||
| 139 | uc.mu.Lock() | 139 | uc.mu.Lock() | ||
| 140 | - | defer uc.mu.Unlock() | 140 | + | target := uc.targetInput |
| 141 | + | wasVisible := uc.pickerVisible | |||
| 142 | + | uc.cancelTimerLocked() | |||
| 143 | + | if wasVisible { | |||
| 144 | + | uc.accentPicker.Hide() | |||
| 145 | + | uc.pickerVisible = false | |||
| 146 | + | } | |||
| 147 | + | uc.mu.Unlock() | |||
| 141 | 148 | ||||
| 142 | - | if uc.timer != nil || uc.pickerVisible { | 149 | + | if wasVisible { |
| 143 | log.Debug().Msg("canceling accent picker") | 150 | log.Debug().Msg("canceling accent picker") | ||
| 144 | } | 151 | } | ||
| 145 | 152 | ||||
| 146 | - | uc.cancelTimerLocked() | 153 | + | if wasVisible { |
| 147 | - | if uc.pickerVisible { | 154 | + | if focusable, ok := target.(port.Focusable); ok { |
| 148 | - | uc.accentPicker.Hide() | 155 | + | focusable.Focus(ctx) |
| 149 | - | uc.pickerVisible = false | 156 | + | } |
| 150 | } | 157 | } | ||
| 151 | } | 158 | } | ||
| 152 | 159 | ||||
| @@ -249,6 +256,10 @@ func (uc *InsertAccentUseCase) onAccentSelected(ctx context.Context, accent rune | |||||
| 249 | if err := target.InsertText(ctx, string(accent)); err != nil { | 256 | if err := target.InsertText(ctx, string(accent)); err != nil { | ||
| 250 | log.Error().Err(err).Msg("failed to insert accent") | 257 | log.Error().Err(err).Msg("failed to insert accent") | ||
| 251 | } | 258 | } | ||
| 259 | + | ||||
| 260 | + | if focusable, ok := target.(port.Focusable); ok { | |||
| 261 | + | focusable.Focus(ctx) | |||
| 262 | + | } | |||
| 252 | } | 263 | } | ||
| 253 | 264 | ||||
| 254 | // onPickerCanceled is called when the user cancels the picker (Escape). | 265 | // onPickerCanceled is called when the user cancels the picker (Escape). | ||
| @@ -256,6 +267,7 @@ func (uc *InsertAccentUseCase) onPickerCanceled(ctx context.Context) { | |||||
| 256 | log := logging.FromContext(ctx) | 267 | log := logging.FromContext(ctx) | ||
| 257 | 268 | ||||
| 258 | uc.mu.Lock() | 269 | uc.mu.Lock() | ||
| 270 | + | target := uc.targetInput | |||
| 259 | uc.pickerVisible = false | 271 | uc.pickerVisible = false | ||
| 260 | uc.pressedChar = 0 | 272 | uc.pressedChar = 0 | ||
| 261 | uc.repeatCount = 0 | 273 | uc.repeatCount = 0 | ||
| @@ -264,6 +276,10 @@ func (uc *InsertAccentUseCase) onPickerCanceled(ctx context.Context) { | |||||
| 264 | 276 | ||||
| 265 | uc.accentPicker.Hide() | 277 | uc.accentPicker.Hide() | ||
| 266 | 278 | ||||
| 279 | + | if focusable, ok := target.(port.Focusable); ok { | |||
| 280 | + | focusable.Focus(ctx) | |||
| 281 | + | } | |||
| 282 | + | ||||
| 267 | log.Debug().Msg("accent picker canceled") | 283 | log.Debug().Msg("accent picker canceled") | ||
| 268 | } | 284 | } | ||
| 269 | 285 | ||||
M internal/application/usecase/insert_accent_test.go +45 -0
| @@ -6,11 +6,21 @@ import ( | |||
| 6 | 6 | "testing" | |
| 7 | 7 | "time" | |
| 8 | 8 | ||
| 9 | + | "github.com/bnema/dumber/internal/application/port" | |
| 9 | 10 | "github.com/bnema/dumber/internal/application/port/mocks" | |
| 10 | 11 | "github.com/stretchr/testify/assert" | |
| 11 | 12 | "github.com/stretchr/testify/mock" | |
| 12 | 13 | ) | |
| 13 | 14 | ||
| 15 | + | type focusableTextInputTarget struct { | |
| 16 | + | port.TextInputTarget | |
| 17 | + | focusCalls int | |
| 18 | + | } | |
| 19 | + | ||
| 20 | + | func (t *focusableTextInputTarget) Focus(_ context.Context) { | |
| 21 | + | t.focusCalls++ | |
| 22 | + | } | |
| 23 | + | ||
| 14 | 24 | func TestInsertAccentUseCase_OnKeyPressed_NoAccents(t *testing.T) { | |
| 15 | 25 | ctx := context.Background() | |
| 16 | 26 | focusProvider := mocks.NewMockFocusedInputProvider(t) | |
| @@ -217,3 +227,38 @@ func TestInsertAccentUseCase_PickerVisibleBlocksNewLongPress(t *testing.T) { | |||
| 217 | 227 | ||
| 218 | 228 | uc.Cancel(ctx) | |
| 219 | 229 | } | |
| 230 | + | ||
| 231 | + | func TestInsertAccentUseCase_OnAccentSelected_RestoresFocusForFocusableTarget(t *testing.T) { | |
| 232 | + | ctx := context.Background() | |
| 233 | + | focusProvider := mocks.NewMockFocusedInputProvider(t) | |
| 234 | + | accentPicker := mocks.NewMockAccentPickerUI(t) | |
| 235 | + | textInput := mocks.NewMockTextInputTarget(t) | |
| 236 | + | target := &focusableTextInputTarget{TextInputTarget: textInput} | |
| 237 | + | ||
| 238 | + | focusProvider.EXPECT().GetFocusedInput().Return(target).Once() | |
| 239 | + | textInput.EXPECT().InsertText(ctx, "à").Return(nil).Once() | |
| 240 | + | accentPicker.EXPECT().Hide().Once() | |
| 241 | + | ||
| 242 | + | uc := NewInsertAccentUseCase(focusProvider, accentPicker, func(fn func()) { fn() }) | |
| 243 | + | uc.OnKeyPressed(ctx, 'a', false) | |
| 244 | + | uc.onAccentSelected(ctx, 'à') | |
| 245 | + | ||
| 246 | + | assert.Equal(t, 1, target.focusCalls, "focus should be restored after accent insertion") | |
| 247 | + | } | |
| 248 | + | ||
| 249 | + | func TestInsertAccentUseCase_OnPickerCanceled_RestoresFocusForFocusableTarget(t *testing.T) { | |
| 250 | + | ctx := context.Background() | |
| 251 | + | focusProvider := mocks.NewMockFocusedInputProvider(t) | |
| 252 | + | accentPicker := mocks.NewMockAccentPickerUI(t) | |
| 253 | + | textInput := mocks.NewMockTextInputTarget(t) | |
| 254 | + | target := &focusableTextInputTarget{TextInputTarget: textInput} | |
| 255 | + | ||
| 256 | + | focusProvider.EXPECT().GetFocusedInput().Return(target).Once() | |
| 257 | + | accentPicker.EXPECT().Hide().Once() | |
| 258 | + | ||
| 259 | + | uc := NewInsertAccentUseCase(focusProvider, accentPicker, func(fn func()) { fn() }) | |
| 260 | + | uc.OnKeyPressed(ctx, 'a', false) | |
| 261 | + | uc.onPickerCanceled(ctx) | |
| 262 | + | ||
| 263 | + | assert.Equal(t, 1, target.focusCalls, "focus should be restored after picker cancel") | |
| 264 | + | } | |
| @@ -6,11 +6,21 @@ import ( | |||||
| 6 | "testing" | 6 | "testing" | ||
| 7 | "time" | 7 | "time" | ||
| 8 | 8 | ||||
| 9 | + | "github.com/bnema/dumber/internal/application/port" | |||
| 9 | "github.com/bnema/dumber/internal/application/port/mocks" | 10 | "github.com/bnema/dumber/internal/application/port/mocks" | ||
| 10 | "github.com/stretchr/testify/assert" | 11 | "github.com/stretchr/testify/assert" | ||
| 11 | "github.com/stretchr/testify/mock" | 12 | "github.com/stretchr/testify/mock" | ||
| 12 | ) | 13 | ) | ||
| 13 | 14 | ||||
| 15 | + | type focusableTextInputTarget struct { | |||
| 16 | + | port.TextInputTarget | |||
| 17 | + | focusCalls int | |||
| 18 | + | } | |||
| 19 | + | ||||
| 20 | + | func (t *focusableTextInputTarget) Focus(_ context.Context) { | |||
| 21 | + | t.focusCalls++ | |||
| 22 | + | } | |||
| 23 | + | ||||
| 14 | func TestInsertAccentUseCase_OnKeyPressed_NoAccents(t *testing.T) { | 24 | func TestInsertAccentUseCase_OnKeyPressed_NoAccents(t *testing.T) { | ||
| 15 | ctx := context.Background() | 25 | ctx := context.Background() | ||
| 16 | focusProvider := mocks.NewMockFocusedInputProvider(t) | 26 | focusProvider := mocks.NewMockFocusedInputProvider(t) | ||
| @@ -217,3 +227,38 @@ func TestInsertAccentUseCase_PickerVisibleBlocksNewLongPress(t *testing.T) { | |||||
| 217 | 227 | ||||
| 218 | uc.Cancel(ctx) | 228 | uc.Cancel(ctx) | ||
| 219 | } | 229 | } | ||
| 230 | + | ||||
| 231 | + | func TestInsertAccentUseCase_OnAccentSelected_RestoresFocusForFocusableTarget(t *testing.T) { | |||
| 232 | + | ctx := context.Background() | |||
| 233 | + | focusProvider := mocks.NewMockFocusedInputProvider(t) | |||
| 234 | + | accentPicker := mocks.NewMockAccentPickerUI(t) | |||
| 235 | + | textInput := mocks.NewMockTextInputTarget(t) | |||
| 236 | + | target := &focusableTextInputTarget{TextInputTarget: textInput} | |||
| 237 | + | ||||
| 238 | + | focusProvider.EXPECT().GetFocusedInput().Return(target).Once() | |||
| 239 | + | textInput.EXPECT().InsertText(ctx, "à").Return(nil).Once() | |||
| 240 | + | accentPicker.EXPECT().Hide().Once() | |||
| 241 | + | ||||
| 242 | + | uc := NewInsertAccentUseCase(focusProvider, accentPicker, func(fn func()) { fn() }) | |||
| 243 | + | uc.OnKeyPressed(ctx, 'a', false) | |||
| 244 | + | uc.onAccentSelected(ctx, 'à') | |||
| 245 | + | ||||
| 246 | + | assert.Equal(t, 1, target.focusCalls, "focus should be restored after accent insertion") | |||
| 247 | + | } | |||
| 248 | + | ||||
| 249 | + | func TestInsertAccentUseCase_OnPickerCanceled_RestoresFocusForFocusableTarget(t *testing.T) { | |||
| 250 | + | ctx := context.Background() | |||
| 251 | + | focusProvider := mocks.NewMockFocusedInputProvider(t) | |||
| 252 | + | accentPicker := mocks.NewMockAccentPickerUI(t) | |||
| 253 | + | textInput := mocks.NewMockTextInputTarget(t) | |||
| 254 | + | target := &focusableTextInputTarget{TextInputTarget: textInput} | |||
| 255 | + | ||||
| 256 | + | focusProvider.EXPECT().GetFocusedInput().Return(target).Once() | |||
| 257 | + | accentPicker.EXPECT().Hide().Once() | |||
| 258 | + | ||||
| 259 | + | uc := NewInsertAccentUseCase(focusProvider, accentPicker, func(fn func()) { fn() }) | |||
| 260 | + | uc.OnKeyPressed(ctx, 'a', false) | |||
| 261 | + | uc.onPickerCanceled(ctx) | |||
| 262 | + | ||||
| 263 | + | assert.Equal(t, 1, target.focusCalls, "focus should be restored after picker cancel") | |||
| 264 | + | } | |||
M internal/infrastructure/textinput/webview_target.go +55 -48
| @@ -3,22 +3,18 @@ package textinput | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "fmt" | |
| 6 | - | "time" | |
| 7 | 6 | ||
| 8 | 7 | "github.com/bnema/dumber/internal/application/port" | |
| 9 | 8 | "github.com/bnema/dumber/internal/logging" | |
| 10 | 9 | "github.com/bnema/puregotk-webkit/webkit" | |
| 11 | 10 | ) | |
| 12 | 11 | ||
| 13 | - | // clipboardRestoreDelay is the delay before restoring the clipboard after paste. | |
| 14 | - | const clipboardRestoreDelay = 50 * time.Millisecond | |
| 15 | - | ||
| 16 | 12 | // deleteBeforeCursorJS is the JavaScript template to delete n characters before cursor. | |
| 17 | 13 | // It handles both input/textarea elements and contenteditable elements. | |
| 18 | 14 | const deleteBeforeCursorJS = ` | |
| 19 | 15 | (function() { | |
| 20 | 16 | const n = %d; | |
| 21 | - | const el = document.activeElement; | |
| 17 | + | const el = window.__dumber_lastEditableEl || document.activeElement; | |
| 22 | 18 | if (!el) return; | |
| 23 | 19 | ||
| 24 | 20 | // Handle input and textarea elements | |
| @@ -38,7 +34,6 @@ const deleteBeforeCursorJS = ` | |||
| 38 | 34 | if (el.isContentEditable) { | |
| 39 | 35 | const sel = window.getSelection(); | |
| 40 | 36 | if (!sel.rangeCount) return; | |
| 41 | - | const range = sel.getRangeAt(0); | |
| 42 | 37 | for (let i = 0; i < n; i++) { | |
| 43 | 38 | sel.modify('extend', 'backward', 'character'); | |
| 44 | 39 | } | |
| @@ -47,30 +42,51 @@ const deleteBeforeCursorJS = ` | |||
| 47 | 42 | })(); | |
| 48 | 43 | ` | |
| 49 | 44 | ||
| 50 | - | // WebViewTarget implements TextInputTarget for WebView text fields. | |
| 51 | - | // It uses the clipboard-paste pattern to insert text: | |
| 52 | - | // 1. Save current clipboard | |
| 53 | - | // 2. Write text to clipboard | |
| 54 | - | // 3. Execute paste command | |
| 55 | - | // 4. Restore original clipboard after a short delay | |
| 45 | + | // insertTextJS inserts text at the cursor position of the last focused editable element. | |
| 46 | + | // Uses the tracked element from the accent detection script so insertion still works | |
| 47 | + | // after GTK focus moves to the accent picker. | |
| 48 | + | const insertTextJS = ` | |
| 49 | + | (function() { | |
| 50 | + | const text = %q; | |
| 51 | + | const el = window.__dumber_lastEditableEl || document.activeElement; | |
| 52 | + | if (!el) return; | |
| 53 | + | ||
| 54 | + | el.focus(); | |
| 55 | + | ||
| 56 | + | if (el.tagName === 'INPUT' || el.tagName === 'TEXTAREA') { | |
| 57 | + | const start = el.selectionStart; | |
| 58 | + | const end = el.selectionEnd; | |
| 59 | + | el.value = el.value.slice(0, start) + text + el.value.slice(end); | |
| 60 | + | el.selectionStart = el.selectionEnd = start + text.length; | |
| 61 | + | el.dispatchEvent(new Event('input', { bubbles: true })); | |
| 62 | + | return; | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | // execCommand is deprecated but remains the most reliable way to insert | |
| 66 | + | // text into contenteditable elements across WebKit/GTK. The modern | |
| 67 | + | // InputEvent('insertText') alternative has inconsistent support. | |
| 68 | + | if (el.isContentEditable) { | |
| 69 | + | document.execCommand('insertText', false, text); | |
| 70 | + | } | |
| 71 | + | })(); | |
| 72 | + | ` | |
| 73 | + | ||
| 74 | + | // WebViewTarget implements text input and focus restoration for WebView fields. | |
| 56 | 75 | type WebViewTarget struct { | |
| 57 | - | webView *webkit.WebView | |
| 58 | - | clipboard port.Clipboard | |
| 76 | + | webView *webkit.WebView | |
| 59 | 77 | } | |
| 60 | 78 | ||
| 61 | - | // Compile-time interface check. | |
| 62 | 79 | var _ port.TextInputTarget = (*WebViewTarget)(nil) | |
| 80 | + | var _ port.Focusable = (*WebViewTarget)(nil) | |
| 63 | 81 | ||
| 64 | 82 | // NewWebViewTarget creates a new WebView target. | |
| 65 | - | func NewWebViewTarget(webView *webkit.WebView, clipboard port.Clipboard) *WebViewTarget { | |
| 83 | + | func NewWebViewTarget(webView *webkit.WebView) *WebViewTarget { | |
| 66 | 84 | return &WebViewTarget{ | |
| 67 | - | webView: webView, | |
| 68 | - | clipboard: clipboard, | |
| 85 | + | webView: webView, | |
| 69 | 86 | } | |
| 70 | 87 | } | |
| 71 | 88 | ||
| 72 | 89 | // InsertText inserts text at the current cursor position in the WebView. | |
| 73 | - | // Uses clipboard-paste pattern since WebView doesn't expose direct text insertion. | |
| 74 | 90 | func (t *WebViewTarget) InsertText(ctx context.Context, text string) error { | |
| 75 | 91 | log := logging.FromContext(ctx) | |
| 76 | 92 | ||
| @@ -79,37 +95,12 @@ func (t *WebViewTarget) InsertText(ctx context.Context, text string) error { | |||
| 79 | 95 | return nil | |
| 80 | 96 | } | |
| 81 | 97 | ||
| 82 | - | // Save current clipboard contents | |
| 83 | - | originalClipboard, _ := t.clipboard.ReadText(ctx) | |
| 84 | - | ||
| 85 | - | // Write the accent to clipboard | |
| 86 | - | if err := t.clipboard.WriteText(ctx, text); err != nil { | |
| 87 | - | log.Error().Err(err).Msg("failed to write text to clipboard") | |
| 88 | - | return err | |
| 89 | - | } | |
| 90 | - | ||
| 91 | - | // Execute paste command | |
| 92 | - | t.webView.ExecuteEditingCommand(webkit.EDITING_COMMAND_PASTE) | |
| 98 | + | script := fmt.Sprintf(insertTextJS, text) | |
| 99 | + | t.webView.EvaluateJavascript(script, -1, nil, nil, nil, nil, 0) | |
| 93 | 100 | ||
| 94 | 101 | log.Debug(). | |
| 95 | - | Str("text", text). | |
| 96 | - | Msg("inserted text into WebView via clipboard paste") | |
| 97 | - | ||
| 98 | - | // Restore original clipboard after a short delay. | |
| 99 | - | // This allows the paste operation to complete first. | |
| 100 | - | // Derive a detached context so the restore runs even if ctx is canceled, | |
| 101 | - | // but still carries its values/logger. | |
| 102 | - | detachedCtx := context.WithoutCancel(ctx) | |
| 103 | - | go func() { | |
| 104 | - | time.Sleep(clipboardRestoreDelay) | |
| 105 | - | restoreCtx, cancelRestore := context.WithTimeout(detachedCtx, clipboardRestoreDelay) | |
| 106 | - | defer cancelRestore() | |
| 107 | - | if originalClipboard != "" { | |
| 108 | - | if err := t.clipboard.WriteText(restoreCtx, originalClipboard); err != nil { | |
| 109 | - | log.Debug().Err(err).Msg("failed to restore clipboard") | |
| 110 | - | } | |
| 111 | - | } | |
| 112 | - | }() | |
| 102 | + | Int("len", len(text)). | |
| 103 | + | Msg("inserted text into WebView via JS") | |
| 113 | 104 | ||
| 114 | 105 | return nil | |
| 115 | 106 | } | |
| @@ -135,3 +126,19 @@ func (t *WebViewTarget) DeleteBeforeCursor(ctx context.Context, n int) error { | |||
| 135 | 126 | log.Debug().Int("n", n).Msg("deleted characters before cursor in WebView") | |
| 136 | 127 | return nil | |
| 137 | 128 | } | |
| 129 | + | ||
| 130 | + | // Focus restores GTK focus to the WebView widget and refocuses the last | |
| 131 | + | // editable element inside the page. | |
| 132 | + | func (t *WebViewTarget) Focus(_ context.Context) { | |
| 133 | + | if t.webView == nil { | |
| 134 | + | return | |
| 135 | + | } | |
| 136 | + | ||
| 137 | + | t.webView.GrabFocus() | |
| 138 | + | t.webView.EvaluateJavascript(` | |
| 139 | + | (function() { | |
| 140 | + | var el = window.__dumber_lastEditableEl; | |
| 141 | + | if (el) el.focus(); | |
| 142 | + | })(); | |
| 143 | + | `, -1, nil, nil, nil, nil, 0) | |
| 144 | + | } | |
| @@ -3,22 +3,18 @@ package textinput | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | - | "time" | |||
| 7 | 6 | ||||
| 8 | "github.com/bnema/dumber/internal/application/port" | 7 | "github.com/bnema/dumber/internal/application/port" | ||
| 9 | "github.com/bnema/dumber/internal/logging" | 8 | "github.com/bnema/dumber/internal/logging" | ||
| 10 | "github.com/bnema/puregotk-webkit/webkit" | 9 | "github.com/bnema/puregotk-webkit/webkit" | ||
| 11 | ) | 10 | ) | ||
| 12 | 11 | ||||
| 13 | - | // clipboardRestoreDelay is the delay before restoring the clipboard after paste. | |||
| 14 | - | const clipboardRestoreDelay = 50 * time.Millisecond | |||
| 15 | - | ||||
| 16 | // deleteBeforeCursorJS is the JavaScript template to delete n characters before cursor. | 12 | // deleteBeforeCursorJS is the JavaScript template to delete n characters before cursor. | ||
| 17 | // It handles both input/textarea elements and contenteditable elements. | 13 | // It handles both input/textarea elements and contenteditable elements. | ||
| 18 | const deleteBeforeCursorJS = ` | 14 | const deleteBeforeCursorJS = ` | ||
| 19 | (function() { | 15 | (function() { | ||
| 20 | const n = %d; | 16 | const n = %d; | ||
| 21 | - | const el = document.activeElement; | 17 | + | const el = window.__dumber_lastEditableEl || document.activeElement; |
| 22 | if (!el) return; | 18 | if (!el) return; | ||
| 23 | 19 | ||||
| 24 | // Handle input and textarea elements | 20 | // Handle input and textarea elements | ||
| @@ -38,7 +34,6 @@ const deleteBeforeCursorJS = ` | |||||
| 38 | if (el.isContentEditable) { | 34 | if (el.isContentEditable) { | ||
| 39 | const sel = window.getSelection(); | 35 | const sel = window.getSelection(); | ||
| 40 | if (!sel.rangeCount) return; | 36 | if (!sel.rangeCount) return; | ||
| 41 | - | const range = sel.getRangeAt(0); | |||
| 42 | for (let i = 0; i < n; i++) { | 37 | for (let i = 0; i < n; i++) { | ||
| 43 | sel.modify('extend', 'backward', 'character'); | 38 | sel.modify('extend', 'backward', 'character'); | ||
| 44 | } | 39 | } | ||
| @@ -47,30 +42,51 @@ const deleteBeforeCursorJS = ` | |||||
| 47 | })(); | 42 | })(); | ||
| 48 | ` | 43 | ` | ||
| 49 | 44 | ||||
| 50 | - | // WebViewTarget implements TextInputTarget for WebView text fields. | 45 | + | // insertTextJS inserts text at the cursor position of the last focused editable element. |
| 51 | - | // It uses the clipboard-paste pattern to insert text: | 46 | + | // Uses the tracked element from the accent detection script so insertion still works |
| 52 | - | // 1. Save current clipboard | 47 | + | // after GTK focus moves to the accent picker. |
| 53 | - | // 2. Write text to clipboard | 48 | + | const insertTextJS = ` |
| 54 | - | // 3. Execute paste command | 49 | + | (function() { |
| 55 | - | // 4. Restore original clipboard after a short delay | 50 | + | const text = %q; |
| 51 | + | const el = window.__dumber_lastEditableEl || document.activeElement; | |||
| 52 | + | if (!el) return; | |||
| 53 | + | ||||
| 54 | + | el.focus(); | |||
| 55 | + | ||||
| 56 | + | if (el.tagName === 'INPUT' || el.tagName === 'TEXTAREA') { | |||
| 57 | + | const start = el.selectionStart; | |||
| 58 | + | const end = el.selectionEnd; | |||
| 59 | + | el.value = el.value.slice(0, start) + text + el.value.slice(end); | |||
| 60 | + | el.selectionStart = el.selectionEnd = start + text.length; | |||
| 61 | + | el.dispatchEvent(new Event('input', { bubbles: true })); | |||
| 62 | + | return; | |||
| 63 | + | } | |||
| 64 | + | ||||
| 65 | + | // execCommand is deprecated but remains the most reliable way to insert | |||
| 66 | + | // text into contenteditable elements across WebKit/GTK. The modern | |||
| 67 | + | // InputEvent('insertText') alternative has inconsistent support. | |||
| 68 | + | if (el.isContentEditable) { | |||
| 69 | + | document.execCommand('insertText', false, text); | |||
| 70 | + | } | |||
| 71 | + | })(); | |||
| 72 | + | ` | |||
| 73 | + | ||||
| 74 | + | // WebViewTarget implements text input and focus restoration for WebView fields. | |||
| 56 | type WebViewTarget struct { | 75 | type WebViewTarget struct { | ||
| 57 | - | webView *webkit.WebView | 76 | + | webView *webkit.WebView |
| 58 | - | clipboard port.Clipboard | |||
| 59 | } | 77 | } | ||
| 60 | 78 | ||||
| 61 | - | // Compile-time interface check. | |||
| 62 | var _ port.TextInputTarget = (*WebViewTarget)(nil) | 79 | var _ port.TextInputTarget = (*WebViewTarget)(nil) | ||
| 80 | + | var _ port.Focusable = (*WebViewTarget)(nil) | |||
| 63 | 81 | ||||
| 64 | // NewWebViewTarget creates a new WebView target. | 82 | // NewWebViewTarget creates a new WebView target. | ||
| 65 | - | func NewWebViewTarget(webView *webkit.WebView, clipboard port.Clipboard) *WebViewTarget { | 83 | + | func NewWebViewTarget(webView *webkit.WebView) *WebViewTarget { |
| 66 | return &WebViewTarget{ | 84 | return &WebViewTarget{ | ||
| 67 | - | webView: webView, | 85 | + | webView: webView, |
| 68 | - | clipboard: clipboard, | |||
| 69 | } | 86 | } | ||
| 70 | } | 87 | } | ||
| 71 | 88 | ||||
| 72 | // InsertText inserts text at the current cursor position in the WebView. | 89 | // InsertText inserts text at the current cursor position in the WebView. | ||
| 73 | - | // Uses clipboard-paste pattern since WebView doesn't expose direct text insertion. | |||
| 74 | func (t *WebViewTarget) InsertText(ctx context.Context, text string) error { | 90 | func (t *WebViewTarget) InsertText(ctx context.Context, text string) error { | ||
| 75 | log := logging.FromContext(ctx) | 91 | log := logging.FromContext(ctx) | ||
| 76 | 92 | ||||
| @@ -79,37 +95,12 @@ func (t *WebViewTarget) InsertText(ctx context.Context, text string) error { | |||||
| 79 | return nil | 95 | return nil | ||
| 80 | } | 96 | } | ||
| 81 | 97 | ||||
| 82 | - | // Save current clipboard contents | 98 | + | script := fmt.Sprintf(insertTextJS, text) |
| 83 | - | originalClipboard, _ := t.clipboard.ReadText(ctx) | 99 | + | t.webView.EvaluateJavascript(script, -1, nil, nil, nil, nil, 0) |
| 84 | - | ||||
| 85 | - | // Write the accent to clipboard | |||
| 86 | - | if err := t.clipboard.WriteText(ctx, text); err != nil { | |||
| 87 | - | log.Error().Err(err).Msg("failed to write text to clipboard") | |||
| 88 | - | return err | |||
| 89 | - | } | |||
| 90 | - | ||||
| 91 | - | // Execute paste command | |||
| 92 | - | t.webView.ExecuteEditingCommand(webkit.EDITING_COMMAND_PASTE) | |||
| 93 | 100 | ||||
| 94 | log.Debug(). | 101 | log.Debug(). | ||
| 95 | - | Str("text", text). | 102 | + | Int("len", len(text)). |
| 96 | - | Msg("inserted text into WebView via clipboard paste") | 103 | + | Msg("inserted text into WebView via JS") |
| 97 | - | ||||
| 98 | - | // Restore original clipboard after a short delay. | |||
| 99 | - | // This allows the paste operation to complete first. | |||
| 100 | - | // Derive a detached context so the restore runs even if ctx is canceled, | |||
| 101 | - | // but still carries its values/logger. | |||
| 102 | - | detachedCtx := context.WithoutCancel(ctx) | |||
| 103 | - | go func() { | |||
| 104 | - | time.Sleep(clipboardRestoreDelay) | |||
| 105 | - | restoreCtx, cancelRestore := context.WithTimeout(detachedCtx, clipboardRestoreDelay) | |||
| 106 | - | defer cancelRestore() | |||
| 107 | - | if originalClipboard != "" { | |||
| 108 | - | if err := t.clipboard.WriteText(restoreCtx, originalClipboard); err != nil { | |||
| 109 | - | log.Debug().Err(err).Msg("failed to restore clipboard") | |||
| 110 | - | } | |||
| 111 | - | } | |||
| 112 | - | }() | |||
| 113 | 104 | ||||
| 114 | return nil | 105 | return nil | ||
| 115 | } | 106 | } | ||
| @@ -135,3 +126,19 @@ func (t *WebViewTarget) DeleteBeforeCursor(ctx context.Context, n int) error { | |||||
| 135 | log.Debug().Int("n", n).Msg("deleted characters before cursor in WebView") | 126 | log.Debug().Int("n", n).Msg("deleted characters before cursor in WebView") | ||
| 136 | return nil | 127 | return nil | ||
| 137 | } | 128 | } | ||
| 129 | + | ||||
| 130 | + | // Focus restores GTK focus to the WebView widget and refocuses the last | |||
| 131 | + | // editable element inside the page. | |||
| 132 | + | func (t *WebViewTarget) Focus(_ context.Context) { | |||
| 133 | + | if t.webView == nil { | |||
| 134 | + | return | |||
| 135 | + | } | |||
| 136 | + | ||||
| 137 | + | t.webView.GrabFocus() | |||
| 138 | + | t.webView.EvaluateJavascript(` | |||
| 139 | + | (function() { | |||
| 140 | + | var el = window.__dumber_lastEditableEl; | |||
| 141 | + | if (el) el.focus(); | |||
| 142 | + | })(); | |||
| 143 | + | `, -1, nil, nil, nil, nil, 0) | |||
| 144 | + | } | |||
M internal/infrastructure/webkit/injector.go +26 -19
| @@ -3,9 +3,11 @@ package webkit | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "fmt" | |
| 6 | + | "sort" | |
| 6 | 7 | "strings" | |
| 7 | 8 | ||
| 8 | 9 | "github.com/bnema/dumber/internal/application/port" | |
| 10 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 9 | 11 | "github.com/bnema/dumber/internal/logging" | |
| 10 | 12 | "github.com/bnema/puregotk-webkit/webkit" | |
| 11 | 13 | ) | |
| @@ -141,16 +143,23 @@ const autoCopySelectionScript = `(function() { | |||
| 141 | 143 | }); | |
| 142 | 144 | })();` | |
| 143 | 145 | ||
| 144 | - | // accentDetectionScript detects keydown/keyup of a-z keys in WebView inputs and | |
| 145 | - | // forwards them to the Go-side InsertAccentUseCase via the message bridge. | |
| 146 | - | // Go handles timing, accent lookup, and picker display — JS only reports events. | |
| 147 | - | const accentDetectionScript = `(function() { | |
| 148 | - | 'use strict'; | |
| 146 | + | // accentDetectionScript is built at init from entity.AccentMap so the JS | |
| 147 | + | // filter stays in sync with the Go-side accent table. | |
| 148 | + | var accentDetectionScript string | |
| 149 | + | ||
| 150 | + | func init() { | |
| 151 | + | // Build JS Set literal from AccentMap keys: "new Set(['a','c','e',...])" | |
| 152 | + | keys := make([]string, 0, len(entity.AccentMap)) | |
| 153 | + | for k := range entity.AccentMap { | |
| 154 | + | keys = append(keys, fmt.Sprintf("'%c'", k)) | |
| 155 | + | } | |
| 156 | + | sort.Strings(keys) | |
| 149 | 157 | ||
| 150 | - | // Track pressed key for release matching | |
| 158 | + | accentDetectionScript = fmt.Sprintf(`(function() { | |
| 159 | + | 'use strict'; | |
| 160 | + | const accentKeys = new Set([%s]); | |
| 151 | 161 | let pressedKey = null; | |
| 152 | 162 | ||
| 153 | - | // Check if the event target is an editable element (input, textarea, contenteditable) | |
| 154 | 163 | function isEditableTarget(target) { | |
| 155 | 164 | const el = target instanceof Element ? target : null; | |
| 156 | 165 | if (!el) return false; | |
| @@ -158,29 +167,20 @@ const accentDetectionScript = `(function() { | |||
| 158 | 167 | if (el.closest('textarea')) return true; | |
| 159 | 168 | const input = el.closest('input'); | |
| 160 | 169 | if (!input) return false; | |
| 161 | - | // Exclude non-text input types | |
| 162 | 170 | return !/^(button|checkbox|color|file|hidden|image|radio|range|reset|submit)$/i.test(input.type); | |
| 163 | 171 | } | |
| 164 | 172 | ||
| 165 | 173 | document.addEventListener('keydown', function(e) { | |
| 166 | 174 | if (!isEditableTarget(e.target)) return; | |
| 167 | - | ||
| 168 | - | // Only handle a-z keys, with optional Shift | |
| 169 | - | // Skip if Ctrl or Alt are held (those are shortcuts) | |
| 170 | 175 | if (e.ctrlKey || e.altKey || e.metaKey) return; | |
| 171 | - | ||
| 172 | - | // Only single character keys a-z | |
| 173 | 176 | const key = e.key.toLowerCase(); | |
| 174 | - | if (key.length !== 1 || key < 'a' || key > 'z') return; | |
| 175 | - | ||
| 177 | + | if (key.length !== 1 || !accentKeys.has(key)) return; | |
| 176 | 178 | pressedKey = key; | |
| 177 | - | ||
| 178 | - | // Send to Go for accent detection | |
| 179 | 179 | window.webkit.messageHandlers.dumber.postMessage({ | |
| 180 | 180 | type: 'accent_key_press', | |
| 181 | 181 | payload: { char: key, shift: e.shiftKey } | |
| 182 | 182 | }); | |
| 183 | - | }, true); // capture phase to see events first | |
| 183 | + | }, true); | |
| 184 | 184 | ||
| 185 | 185 | document.addEventListener('keyup', function(e) { | |
| 186 | 186 | if (!isEditableTarget(e.target)) return; | |
| @@ -193,7 +193,14 @@ const accentDetectionScript = `(function() { | |||
| 193 | 193 | }); | |
| 194 | 194 | } | |
| 195 | 195 | }, true); | |
| 196 | - | })();` | |
| 196 | + | ||
| 197 | + | document.addEventListener('focusin', function(e) { | |
| 198 | + | if (isEditableTarget(e.target)) { | |
| 199 | + | window.__dumber_lastEditableEl = e.target; | |
| 200 | + | } | |
| 201 | + | }, true); | |
| 202 | + | })();`, strings.Join(keys, ",")) | |
| 203 | + | } | |
| 197 | 204 | ||
| 198 | 205 | // accentDetectionInjectionMode controls which frames receive the accent detection script. | |
| 199 | 206 | // Set to AllFrames so the script runs in iframes as well as the top-level document. | |
| @@ -3,9 +3,11 @@ package webkit | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | + | "sort" | |||
| 6 | "strings" | 7 | "strings" | ||
| 7 | 8 | ||||
| 8 | "github.com/bnema/dumber/internal/application/port" | 9 | "github.com/bnema/dumber/internal/application/port" | ||
| 10 | + | "github.com/bnema/dumber/internal/domain/entity" | |||
| 9 | "github.com/bnema/dumber/internal/logging" | 11 | "github.com/bnema/dumber/internal/logging" | ||
| 10 | "github.com/bnema/puregotk-webkit/webkit" | 12 | "github.com/bnema/puregotk-webkit/webkit" | ||
| 11 | ) | 13 | ) | ||
| @@ -141,16 +143,23 @@ const autoCopySelectionScript = `(function() { | |||||
| 141 | }); | 143 | }); | ||
| 142 | })();` | 144 | })();` | ||
| 143 | 145 | ||||
| 144 | - | // accentDetectionScript detects keydown/keyup of a-z keys in WebView inputs and | 146 | + | // accentDetectionScript is built at init from entity.AccentMap so the JS |
| 145 | - | // forwards them to the Go-side InsertAccentUseCase via the message bridge. | 147 | + | // filter stays in sync with the Go-side accent table. |
| 146 | - | // Go handles timing, accent lookup, and picker display — JS only reports events. | 148 | + | var accentDetectionScript string |
| 147 | - | const accentDetectionScript = `(function() { | 149 | + | |
| 148 | - | 'use strict'; | 150 | + | func init() { |
| 151 | + | // Build JS Set literal from AccentMap keys: "new Set(['a','c','e',...])" | |||
| 152 | + | keys := make([]string, 0, len(entity.AccentMap)) | |||
| 153 | + | for k := range entity.AccentMap { | |||
| 154 | + | keys = append(keys, fmt.Sprintf("'%c'", k)) | |||
| 155 | + | } | |||
| 156 | + | sort.Strings(keys) | |||
| 149 | 157 | ||||
| 150 | - | // Track pressed key for release matching | 158 | + | accentDetectionScript = fmt.Sprintf(`(function() { |
| 159 | + | 'use strict'; | |||
| 160 | + | const accentKeys = new Set([%s]); | |||
| 151 | let pressedKey = null; | 161 | let pressedKey = null; | ||
| 152 | 162 | ||||
| 153 | - | // Check if the event target is an editable element (input, textarea, contenteditable) | |||
| 154 | function isEditableTarget(target) { | 163 | function isEditableTarget(target) { | ||
| 155 | const el = target instanceof Element ? target : null; | 164 | const el = target instanceof Element ? target : null; | ||
| 156 | if (!el) return false; | 165 | if (!el) return false; | ||
| @@ -158,29 +167,20 @@ const accentDetectionScript = `(function() { | |||||
| 158 | if (el.closest('textarea')) return true; | 167 | if (el.closest('textarea')) return true; | ||
| 159 | const input = el.closest('input'); | 168 | const input = el.closest('input'); | ||
| 160 | if (!input) return false; | 169 | if (!input) return false; | ||
| 161 | - | // Exclude non-text input types | |||
| 162 | return !/^(button|checkbox|color|file|hidden|image|radio|range|reset|submit)$/i.test(input.type); | 170 | return !/^(button|checkbox|color|file|hidden|image|radio|range|reset|submit)$/i.test(input.type); | ||
| 163 | } | 171 | } | ||
| 164 | 172 | ||||
| 165 | document.addEventListener('keydown', function(e) { | 173 | document.addEventListener('keydown', function(e) { | ||
| 166 | if (!isEditableTarget(e.target)) return; | 174 | if (!isEditableTarget(e.target)) return; | ||
| 167 | - | ||||
| 168 | - | // Only handle a-z keys, with optional Shift | |||
| 169 | - | // Skip if Ctrl or Alt are held (those are shortcuts) | |||
| 170 | if (e.ctrlKey || e.altKey || e.metaKey) return; | 175 | if (e.ctrlKey || e.altKey || e.metaKey) return; | ||
| 171 | - | ||||
| 172 | - | // Only single character keys a-z | |||
| 173 | const key = e.key.toLowerCase(); | 176 | const key = e.key.toLowerCase(); | ||
| 174 | - | if (key.length !== 1 || key < 'a' || key > 'z') return; | 177 | + | if (key.length !== 1 || !accentKeys.has(key)) return; |
| 175 | - | ||||
| 176 | pressedKey = key; | 178 | pressedKey = key; | ||
| 177 | - | ||||
| 178 | - | // Send to Go for accent detection | |||
| 179 | window.webkit.messageHandlers.dumber.postMessage({ | 179 | window.webkit.messageHandlers.dumber.postMessage({ | ||
| 180 | type: 'accent_key_press', | 180 | type: 'accent_key_press', | ||
| 181 | payload: { char: key, shift: e.shiftKey } | 181 | payload: { char: key, shift: e.shiftKey } | ||
| 182 | }); | 182 | }); | ||
| 183 | - | }, true); // capture phase to see events first | 183 | + | }, true); |
| 184 | 184 | ||||
| 185 | document.addEventListener('keyup', function(e) { | 185 | document.addEventListener('keyup', function(e) { | ||
| 186 | if (!isEditableTarget(e.target)) return; | 186 | if (!isEditableTarget(e.target)) return; | ||
| @@ -193,7 +193,14 @@ const accentDetectionScript = `(function() { | |||||
| 193 | }); | 193 | }); | ||
| 194 | } | 194 | } | ||
| 195 | }, true); | 195 | }, true); | ||
| 196 | - | })();` | 196 | + | |
| 197 | + | document.addEventListener('focusin', function(e) { | |||
| 198 | + | if (isEditableTarget(e.target)) { | |||
| 199 | + | window.__dumber_lastEditableEl = e.target; | |||
| 200 | + | } | |||
| 201 | + | }, true); | |||
| 202 | + | })();`, strings.Join(keys, ",")) | |||
| 203 | + | } | |||
| 197 | 204 | ||||
| 198 | // accentDetectionInjectionMode controls which frames receive the accent detection script. | 205 | // accentDetectionInjectionMode controls which frames receive the accent detection script. | ||
| 199 | // Set to AllFrames so the script runs in iframes as well as the top-level document. | 206 | // Set to AllFrames so the script runs in iframes as well as the top-level document. | ||
M internal/infrastructure/webkit/injector_accent_test.go +13 -0
| @@ -17,3 +17,16 @@ func TestAccentDetectionScriptInjectionMode(t *testing.T) { | |||
| 17 | 17 | "accent detection script must inject into all frames, not just the top frame", | |
| 18 | 18 | ) | |
| 19 | 19 | } | |
| 20 | + | ||
| 21 | + | func TestAccentDetectionScriptTracksLastFocusedEditableElement(t *testing.T) { | |
| 22 | + | assert.Contains(t, | |
| 23 | + | accentDetectionScript, | |
| 24 | + | "window.__dumber_lastEditableEl = e.target", | |
| 25 | + | "accent detection script must track the last focused editable element", | |
| 26 | + | ) | |
| 27 | + | assert.Contains(t, | |
| 28 | + | accentDetectionScript, | |
| 29 | + | "document.addEventListener('focusin'", | |
| 30 | + | "accent detection script must register a focusin listener", | |
| 31 | + | ) | |
| 32 | + | } | |
| @@ -17,3 +17,16 @@ func TestAccentDetectionScriptInjectionMode(t *testing.T) { | |||||
| 17 | "accent detection script must inject into all frames, not just the top frame", | 17 | "accent detection script must inject into all frames, not just the top frame", | ||
| 18 | ) | 18 | ) | ||
| 19 | } | 19 | } | ||
| 20 | + | ||||
| 21 | + | func TestAccentDetectionScriptTracksLastFocusedEditableElement(t *testing.T) { | |||
| 22 | + | assert.Contains(t, | |||
| 23 | + | accentDetectionScript, | |||
| 24 | + | "window.__dumber_lastEditableEl = e.target", | |||
| 25 | + | "accent detection script must track the last focused editable element", | |||
| 26 | + | ) | |||
| 27 | + | assert.Contains(t, | |||
| 28 | + | accentDetectionScript, | |||
| 29 | + | "document.addEventListener('focusin'", | |||
| 30 | + | "accent detection script must register a focusin listener", | |||
| 31 | + | ) | |||
| 32 | + | } | |||
M internal/infrastructure/webkit/pool.go +1 -0
| @@ -171,6 +171,7 @@ func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | |||
| 171 | 171 | if p.filterApplier != nil { | |
| 172 | 172 | p.filterApplier.ApplyTo(ctx, wv.ucm) | |
| 173 | 173 | } | |
| 174 | + | wv.ReconnectSignals() | |
| 174 | 175 | p.acquireCount.Add(1) | |
| 175 | 176 | p.acquirePoolHits.Add(1) | |
| 176 | 177 | p.acquireTotalNanos.Add(time.Since(start).Nanoseconds()) | |
| @@ -171,6 +171,7 @@ func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | |||||
| 171 | if p.filterApplier != nil { | 171 | if p.filterApplier != nil { | ||
| 172 | p.filterApplier.ApplyTo(ctx, wv.ucm) | 172 | p.filterApplier.ApplyTo(ctx, wv.ucm) | ||
| 173 | } | 173 | } | ||
| 174 | + | wv.ReconnectSignals() | |||
| 174 | p.acquireCount.Add(1) | 175 | p.acquireCount.Add(1) | ||
| 175 | p.acquirePoolHits.Add(1) | 176 | p.acquirePoolHits.Add(1) | ||
| 176 | p.acquireTotalNanos.Add(time.Since(start).Nanoseconds()) | 177 | p.acquireTotalNanos.Add(time.Since(start).Nanoseconds()) | ||
M internal/infrastructure/webkit/webview.go +28 -1
| @@ -137,7 +137,8 @@ type WebView struct { | |||
| 137 | 137 | lastProgressUpdate atomic.Int64 // Unix nanoseconds | |
| 138 | 138 | ||
| 139 | 139 | // Signal handler IDs for disconnection | |
| 140 | - | signalIDs []uintptr | |
| 140 | + | signalIDs []uintptr | |
| 141 | + | generation atomic.Uint64 | |
| 141 | 142 | ||
| 142 | 143 | // Callbacks (set by UI layer) | |
| 143 | 144 | OnLoadChanged func(LoadEvent) | |
| @@ -1405,6 +1406,12 @@ func (wv *WebView) Favicon() *gdk.Texture { | |||
| 1405 | 1406 | return wv.inner.GetFavicon() | |
| 1406 | 1407 | } | |
| 1407 | 1408 | ||
| 1409 | + | // Generation returns the current reuse generation of this WebView. | |
| 1410 | + | // Used by callback holders to detect stale closures after pool reuse. | |
| 1411 | + | func (wv *WebView) Generation() uint64 { | |
| 1412 | + | return wv.generation.Load() | |
| 1413 | + | } | |
| 1414 | + | ||
| 1408 | 1415 | // IsLoading returns true if a page is currently loading. | |
| 1409 | 1416 | func (wv *WebView) IsLoading() bool { | |
| 1410 | 1417 | wv.mu.RLock() | |
| @@ -1623,6 +1630,20 @@ func (wv *WebView) DisconnectSignals() { | |||
| 1623 | 1630 | wv.logger.Debug().Uint64("id", uint64(wv.id)).Msg("signals disconnected") | |
| 1624 | 1631 | } | |
| 1625 | 1632 | ||
| 1633 | + | // ReconnectSignals re-establishes GLib signal handlers after pool reuse. | |
| 1634 | + | // Must be called after acquiring a WebView from the pool and before setting up callbacks. | |
| 1635 | + | func (wv *WebView) ReconnectSignals() { | |
| 1636 | + | if wv == nil || wv.destroyed.Load() || wv.inner == nil { | |
| 1637 | + | return | |
| 1638 | + | } | |
| 1639 | + | // Only reconnect if signals were previously disconnected | |
| 1640 | + | if len(wv.signalIDs) > 0 { | |
| 1641 | + | return // Already connected | |
| 1642 | + | } | |
| 1643 | + | wv.connectSignals() | |
| 1644 | + | wv.logger.Debug().Uint64("id", uint64(wv.id)).Msg("signals reconnected after pool reuse") | |
| 1645 | + | } | |
| 1646 | + | ||
| 1626 | 1647 | // Destroy cleans up the WebView resources and terminates the web process. | |
| 1627 | 1648 | // This must be called when a WebView is no longer needed to free GPU resources, | |
| 1628 | 1649 | // VA-API decoder contexts, and DMA-BUF buffers held by the web process. | |
| @@ -1697,6 +1718,11 @@ func (wv *WebView) ResetForPoolReuse() { | |||
| 1697 | 1718 | if wv == nil || wv.destroyed.Load() { | |
| 1698 | 1719 | return | |
| 1699 | 1720 | } | |
| 1721 | + | wv.generation.Add(1) | |
| 1722 | + | ||
| 1723 | + | // Disconnect GLib signals to prevent stale callbacks from firing on reused WebView. | |
| 1724 | + | // They will be reconnected when the WebView is re-acquired from the pool. | |
| 1725 | + | wv.DisconnectSignals() | |
| 1700 | 1726 | ||
| 1701 | 1727 | wv.OnLoadChanged = nil | |
| 1702 | 1728 | wv.OnTitleChanged = nil | |
| @@ -1712,6 +1738,7 @@ func (wv *WebView) ResetForPoolReuse() { | |||
| 1712 | 1738 | wv.OnAudioStateChanged = nil | |
| 1713 | 1739 | wv.OnLinkHover = nil | |
| 1714 | 1740 | wv.OnWebProcessTerminated = nil | |
| 1741 | + | wv.OnPermissionRequest = nil | |
| 1715 | 1742 | ||
| 1716 | 1743 | wv.mu.Lock() | |
| 1717 | 1744 | wv.uri = "" | |
| @@ -137,7 +137,8 @@ type WebView struct { | |||||
| 137 | lastProgressUpdate atomic.Int64 // Unix nanoseconds | 137 | lastProgressUpdate atomic.Int64 // Unix nanoseconds | ||
| 138 | 138 | ||||
| 139 | // Signal handler IDs for disconnection | 139 | // Signal handler IDs for disconnection | ||
| 140 | - | signalIDs []uintptr | 140 | + | signalIDs []uintptr |
| 141 | + | generation atomic.Uint64 | |||
| 141 | 142 | ||||
| 142 | // Callbacks (set by UI layer) | 143 | // Callbacks (set by UI layer) | ||
| 143 | OnLoadChanged func(LoadEvent) | 144 | OnLoadChanged func(LoadEvent) | ||
| @@ -1405,6 +1406,12 @@ func (wv *WebView) Favicon() *gdk.Texture { | |||||
| 1405 | return wv.inner.GetFavicon() | 1406 | return wv.inner.GetFavicon() | ||
| 1406 | } | 1407 | } | ||
| 1407 | 1408 | ||||
| 1409 | + | // Generation returns the current reuse generation of this WebView. | |||
| 1410 | + | // Used by callback holders to detect stale closures after pool reuse. | |||
| 1411 | + | func (wv *WebView) Generation() uint64 { | |||
| 1412 | + | return wv.generation.Load() | |||
| 1413 | + | } | |||
| 1414 | + | ||||
| 1408 | // IsLoading returns true if a page is currently loading. | 1415 | // IsLoading returns true if a page is currently loading. | ||
| 1409 | func (wv *WebView) IsLoading() bool { | 1416 | func (wv *WebView) IsLoading() bool { | ||
| 1410 | wv.mu.RLock() | 1417 | wv.mu.RLock() | ||
| @@ -1623,6 +1630,20 @@ func (wv *WebView) DisconnectSignals() { | |||||
| 1623 | wv.logger.Debug().Uint64("id", uint64(wv.id)).Msg("signals disconnected") | 1630 | wv.logger.Debug().Uint64("id", uint64(wv.id)).Msg("signals disconnected") | ||
| 1624 | } | 1631 | } | ||
| 1625 | 1632 | ||||
| 1633 | + | // ReconnectSignals re-establishes GLib signal handlers after pool reuse. | |||
| 1634 | + | // Must be called after acquiring a WebView from the pool and before setting up callbacks. | |||
| 1635 | + | func (wv *WebView) ReconnectSignals() { | |||
| 1636 | + | if wv == nil || wv.destroyed.Load() || wv.inner == nil { | |||
| 1637 | + | return | |||
| 1638 | + | } | |||
| 1639 | + | // Only reconnect if signals were previously disconnected | |||
| 1640 | + | if len(wv.signalIDs) > 0 { | |||
| 1641 | + | return // Already connected | |||
| 1642 | + | } | |||
| 1643 | + | wv.connectSignals() | |||
| 1644 | + | wv.logger.Debug().Uint64("id", uint64(wv.id)).Msg("signals reconnected after pool reuse") | |||
| 1645 | + | } | |||
| 1646 | + | ||||
| 1626 | // Destroy cleans up the WebView resources and terminates the web process. | 1647 | // Destroy cleans up the WebView resources and terminates the web process. | ||
| 1627 | // This must be called when a WebView is no longer needed to free GPU resources, | 1648 | // This must be called when a WebView is no longer needed to free GPU resources, | ||
| 1628 | // VA-API decoder contexts, and DMA-BUF buffers held by the web process. | 1649 | // VA-API decoder contexts, and DMA-BUF buffers held by the web process. | ||
| @@ -1697,6 +1718,11 @@ func (wv *WebView) ResetForPoolReuse() { | |||||
| 1697 | if wv == nil || wv.destroyed.Load() { | 1718 | if wv == nil || wv.destroyed.Load() { | ||
| 1698 | return | 1719 | return | ||
| 1699 | } | 1720 | } | ||
| 1721 | + | wv.generation.Add(1) | |||
| 1722 | + | ||||
| 1723 | + | // Disconnect GLib signals to prevent stale callbacks from firing on reused WebView. | |||
| 1724 | + | // They will be reconnected when the WebView is re-acquired from the pool. | |||
| 1725 | + | wv.DisconnectSignals() | |||
| 1700 | 1726 | ||||
| 1701 | wv.OnLoadChanged = nil | 1727 | wv.OnLoadChanged = nil | ||
| 1702 | wv.OnTitleChanged = nil | 1728 | wv.OnTitleChanged = nil | ||
| @@ -1712,6 +1738,7 @@ func (wv *WebView) ResetForPoolReuse() { | |||||
| 1712 | wv.OnAudioStateChanged = nil | 1738 | wv.OnAudioStateChanged = nil | ||
| 1713 | wv.OnLinkHover = nil | 1739 | wv.OnLinkHover = nil | ||
| 1714 | wv.OnWebProcessTerminated = nil | 1740 | wv.OnWebProcessTerminated = nil | ||
| 1741 | + | wv.OnPermissionRequest = nil | |||
| 1715 | 1742 | ||||
| 1716 | wv.mu.Lock() | 1743 | wv.mu.Lock() | ||
| 1717 | wv.uri = "" | 1744 | wv.uri = "" | ||
M internal/ui/app.go +12 -2
| @@ -630,6 +630,8 @@ func (d *downloadEventAdapter) OnDownloadEvent(ctx context.Context, event port.D | |||
| 630 | 630 | } | |
| 631 | 631 | ||
| 632 | 632 | func (a *App) initKeyboardHandler(ctx context.Context) { | |
| 633 | + | log := logging.FromContext(ctx) | |
| 634 | + | ||
| 633 | 635 | if a.mainWindow == nil || a.deps == nil || a.deps.Config == nil { | |
| 634 | 636 | return | |
| 635 | 637 | } | |
| @@ -711,10 +713,14 @@ func (a *App) initKeyboardHandler(ctx context.Context) { | |||
| 711 | 713 | ctx, | |
| 712 | 714 | a.mainWindow.Window(), | |
| 713 | 715 | a.deps.Config, | |
| 716 | + | a.keyboardHandler, | |
| 714 | 717 | func(ctx context.Context, action input.Action) error { | |
| 715 | 718 | return a.kbDispatcher.Dispatch(ctx, action) | |
| 716 | 719 | }, | |
| 717 | 720 | ) | |
| 721 | + | if a.globalShortcutHandler == nil { | |
| 722 | + | log.Warn().Msg("global shortcut handler creation failed, shortcuts may not work when WebView has focus") | |
| 723 | + | } | |
| 718 | 724 | } | |
| 719 | 725 | ||
| 720 | 726 | // handleAccentKeyPress handles accent key press events for GTK entry widgets | |
| @@ -2110,7 +2116,7 @@ func (a *App) activeWorkspaceView() *component.WorkspaceView { | |||
| 2110 | 2116 | // getActiveWebViewTarget returns a TextInputTarget for the active pane's WebView. | |
| 2111 | 2117 | // Used by the accent picker to insert accented characters into web content. | |
| 2112 | 2118 | func (a *App) getActiveWebViewTarget() port.TextInputTarget { | |
| 2113 | - | if a.contentCoord == nil || a.deps == nil || a.deps.Clipboard == nil { | |
| 2119 | + | if a.contentCoord == nil { | |
| 2114 | 2120 | return nil | |
| 2115 | 2121 | } | |
| 2116 | 2122 | ||
| @@ -2120,7 +2126,7 @@ func (a *App) getActiveWebViewTarget() port.TextInputTarget { | |||
| 2120 | 2126 | } | |
| 2121 | 2127 | ||
| 2122 | 2128 | // Get the underlying webkit.WebView for the text input target | |
| 2123 | - | return textinput.NewWebViewTarget(wv.Widget(), a.deps.Clipboard) | |
| 2129 | + | return textinput.NewWebViewTarget(wv.Widget()) | |
| 2124 | 2130 | } | |
| 2125 | 2131 | ||
| 2126 | 2132 | // attachPopupToTab attaches a popup WebView to a newly created tab. | |
| @@ -3047,6 +3053,10 @@ func (a *App) initConfigWatcher(ctx context.Context) { | |||
| 3047 | 3053 | if a.keyboardHandler != nil { | |
| 3048 | 3054 | a.keyboardHandler.ReloadShortcuts(ctx, cfgCopy) | |
| 3049 | 3055 | } | |
| 3056 | + | // Reload global shortcuts (removes stale, re-registers from config) | |
| 3057 | + | if a.globalShortcutHandler != nil { | |
| 3058 | + | a.globalShortcutHandler.ReloadShortcuts(ctx, cfgCopy) | |
| 3059 | + | } | |
| 3050 | 3060 | return false | |
| 3051 | 3061 | }) | |
| 3052 | 3062 | glib.IdleAdd(&cb, 0) | |
| @@ -630,6 +630,8 @@ func (d *downloadEventAdapter) OnDownloadEvent(ctx context.Context, event port.D | |||||
| 630 | } | 630 | } | ||
| 631 | 631 | ||||
| 632 | func (a *App) initKeyboardHandler(ctx context.Context) { | 632 | func (a *App) initKeyboardHandler(ctx context.Context) { | ||
| 633 | + | log := logging.FromContext(ctx) | |||
| 634 | + | ||||
| 633 | if a.mainWindow == nil || a.deps == nil || a.deps.Config == nil { | 635 | if a.mainWindow == nil || a.deps == nil || a.deps.Config == nil { | ||
| 634 | return | 636 | return | ||
| 635 | } | 637 | } | ||
| @@ -711,10 +713,14 @@ func (a *App) initKeyboardHandler(ctx context.Context) { | |||||
| 711 | ctx, | 713 | ctx, | ||
| 712 | a.mainWindow.Window(), | 714 | a.mainWindow.Window(), | ||
| 713 | a.deps.Config, | 715 | a.deps.Config, | ||
| 716 | + | a.keyboardHandler, | |||
| 714 | func(ctx context.Context, action input.Action) error { | 717 | func(ctx context.Context, action input.Action) error { | ||
| 715 | return a.kbDispatcher.Dispatch(ctx, action) | 718 | return a.kbDispatcher.Dispatch(ctx, action) | ||
| 716 | }, | 719 | }, | ||
| 717 | ) | 720 | ) | ||
| 721 | + | if a.globalShortcutHandler == nil { | |||
| 722 | + | log.Warn().Msg("global shortcut handler creation failed, shortcuts may not work when WebView has focus") | |||
| 723 | + | } | |||
| 718 | } | 724 | } | ||
| 719 | 725 | ||||
| 720 | // handleAccentKeyPress handles accent key press events for GTK entry widgets | 726 | // handleAccentKeyPress handles accent key press events for GTK entry widgets | ||
| @@ -2110,7 +2116,7 @@ func (a *App) activeWorkspaceView() *component.WorkspaceView { | |||||
| 2110 | // getActiveWebViewTarget returns a TextInputTarget for the active pane's WebView. | 2116 | // getActiveWebViewTarget returns a TextInputTarget for the active pane's WebView. | ||
| 2111 | // Used by the accent picker to insert accented characters into web content. | 2117 | // Used by the accent picker to insert accented characters into web content. | ||
| 2112 | func (a *App) getActiveWebViewTarget() port.TextInputTarget { | 2118 | func (a *App) getActiveWebViewTarget() port.TextInputTarget { | ||
| 2113 | - | if a.contentCoord == nil || a.deps == nil || a.deps.Clipboard == nil { | 2119 | + | if a.contentCoord == nil { |
| 2114 | return nil | 2120 | return nil | ||
| 2115 | } | 2121 | } | ||
| 2116 | 2122 | ||||
| @@ -2120,7 +2126,7 @@ func (a *App) getActiveWebViewTarget() port.TextInputTarget { | |||||
| 2120 | } | 2126 | } | ||
| 2121 | 2127 | ||||
| 2122 | // Get the underlying webkit.WebView for the text input target | 2128 | // Get the underlying webkit.WebView for the text input target | ||
| 2123 | - | return textinput.NewWebViewTarget(wv.Widget(), a.deps.Clipboard) | 2129 | + | return textinput.NewWebViewTarget(wv.Widget()) |
| 2124 | } | 2130 | } | ||
| 2125 | 2131 | ||||
| 2126 | // attachPopupToTab attaches a popup WebView to a newly created tab. | 2132 | // attachPopupToTab attaches a popup WebView to a newly created tab. | ||
| @@ -3047,6 +3053,10 @@ func (a *App) initConfigWatcher(ctx context.Context) { | |||||
| 3047 | if a.keyboardHandler != nil { | 3053 | if a.keyboardHandler != nil { | ||
| 3048 | a.keyboardHandler.ReloadShortcuts(ctx, cfgCopy) | 3054 | a.keyboardHandler.ReloadShortcuts(ctx, cfgCopy) | ||
| 3049 | } | 3055 | } | ||
| 3056 | + | // Reload global shortcuts (removes stale, re-registers from config) | |||
| 3057 | + | if a.globalShortcutHandler != nil { | |||
| 3058 | + | a.globalShortcutHandler.ReloadShortcuts(ctx, cfgCopy) | |||
| 3059 | + | } | |||
| 3050 | return false | 3060 | return false | ||
| 3051 | }) | 3061 | }) | ||
| 3052 | glib.IdleAdd(&cb, 0) | 3062 | glib.IdleAdd(&cb, 0) | ||
M internal/ui/coordinator/content.go +101 -18
| @@ -982,15 +982,19 @@ func (c *ContentCoordinator) syncStackedTitle(ctx context.Context, paneID entity | |||
| 982 | 982 | } | |
| 983 | 983 | ||
| 984 | 984 | // onFaviconChanged updates favicon tracking when a WebView's favicon changes. | |
| 985 | - | func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity.PaneID, favicon *gdk.Texture) { | |
| 985 | + | func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity.PaneID, emittingWV *webkit.WebView, favicon *gdk.Texture) { | |
| 986 | 986 | log := logging.FromContext(ctx) | |
| 987 | 987 | ||
| 988 | - | // Get current URI to extract domain for caching | |
| 989 | - | wv := c.getWebViewLocked(paneID) | |
| 990 | - | if wv == nil { | |
| 988 | + | // Verify this WebView is still bound to the expected pane | |
| 989 | + | currentWV := c.getWebViewLocked(paneID) | |
| 990 | + | if currentWV == nil || currentWV != emittingWV { | |
| 991 | + | log.Debug(). | |
| 992 | + | Str("pane_id", string(paneID)). | |
| 993 | + | Msg("ignoring favicon change from unbound webview") | |
| 991 | 994 | return | |
| 992 | 995 | } | |
| 993 | - | uri := wv.URI() | |
| 996 | + | ||
| 997 | + | uri := emittingWV.URI() | |
| 994 | 998 | ||
| 995 | 999 | // Update favicon cache with domain key (handles cross-domain redirects) | |
| 996 | 1000 | if c.faviconAdapter != nil && favicon != nil && uri != "" { | |
| @@ -1001,16 +1005,7 @@ func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity | |||
| 1001 | 1005 | } | |
| 1002 | 1006 | ||
| 1003 | 1007 | // Update StackedView favicon if this pane is in a stack | |
| 1004 | - | _, wsView := c.getActiveWS() | |
| 1005 | - | if wsView != nil { | |
| 1006 | - | tr := wsView.TreeRenderer() | |
| 1007 | - | if tr != nil { | |
| 1008 | - | stackedView := tr.GetStackedViewForPane(string(paneID)) | |
| 1009 | - | if stackedView != nil { | |
| 1010 | - | c.updateStackedPaneFavicon(ctx, stackedView, paneID, favicon) | |
| 1011 | - | } | |
| 1012 | - | } | |
| 1013 | - | } | |
| 1008 | + | c.updateStackedFaviconForPane(ctx, paneID, favicon) | |
| 1014 | 1009 | ||
| 1015 | 1010 | log.Debug(). | |
| 1016 | 1011 | Str("pane_id", string(paneID)). | |
| @@ -1042,6 +1037,83 @@ func (c *ContentCoordinator) updateStackedPaneFavicon( | |||
| 1042 | 1037 | } | |
| 1043 | 1038 | } | |
| 1044 | 1039 | ||
| 1040 | + | // updateStackedFaviconForPane updates the stacked title bar favicon for a pane. | |
| 1041 | + | func (c *ContentCoordinator) updateStackedFaviconForPane(ctx context.Context, paneID entity.PaneID, texture *gdk.Texture) { | |
| 1042 | + | _, wsView := c.getActiveWS() | |
| 1043 | + | if wsView == nil { | |
| 1044 | + | return | |
| 1045 | + | } | |
| 1046 | + | tr := wsView.TreeRenderer() | |
| 1047 | + | if tr == nil { | |
| 1048 | + | return | |
| 1049 | + | } | |
| 1050 | + | if sv := tr.GetStackedViewForPane(string(paneID)); sv != nil { | |
| 1051 | + | c.updateStackedPaneFavicon(ctx, sv, paneID, texture) | |
| 1052 | + | } | |
| 1053 | + | } | |
| 1054 | + | ||
| 1055 | + | // resolveCommittedFavicon ensures the stacked title bar has a favicon after navigation commits. | |
| 1056 | + | // First checks if WebKit already has a favicon for the page (common for subpath URLs). | |
| 1057 | + | // Falls back to the full GetOrFetch pipeline (cache + WebKit DB + DuckDuckGo API). | |
| 1058 | + | // This closes the gap where title bars relied solely on the notify::favicon signal. | |
| 1059 | + | func (c *ContentCoordinator) resolveCommittedFavicon(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | |
| 1060 | + | if c.faviconAdapter == nil || wv == nil { | |
| 1061 | + | return | |
| 1062 | + | } | |
| 1063 | + | ||
| 1064 | + | uri := wv.URI() | |
| 1065 | + | if uri == "" || strings.HasPrefix(uri, "about:") || strings.HasPrefix(uri, "dumb://") { | |
| 1066 | + | return | |
| 1067 | + | } | |
| 1068 | + | ||
| 1069 | + | log := logging.FromContext(ctx) | |
| 1070 | + | ||
| 1071 | + | // Check if we already have a texture cached for this domain | |
| 1072 | + | if texture := c.faviconAdapter.GetTextureByURL(uri); texture != nil { | |
| 1073 | + | // Already cached - just ensure the stacked view is updated | |
| 1074 | + | c.updateStackedFaviconForPane(ctx, paneID, texture) | |
| 1075 | + | return | |
| 1076 | + | } | |
| 1077 | + | ||
| 1078 | + | // Check if WebKit already has a favicon for this page | |
| 1079 | + | if icon := wv.Favicon(); icon != nil { | |
| 1080 | + | log.Debug().Str("pane_id", string(paneID)).Str("uri", uri).Msg("using existing WebKit favicon for committed page") | |
| 1081 | + | // Store and update through the normal path | |
| 1082 | + | c.navOriginMu.RLock() | |
| 1083 | + | originURL := c.navOrigins[paneID] | |
| 1084 | + | c.navOriginMu.RUnlock() | |
| 1085 | + | c.faviconAdapter.StoreFromWebKitWithOrigin(ctx, uri, originURL, icon) | |
| 1086 | + | c.updateStackedFaviconForPane(ctx, paneID, icon) | |
| 1087 | + | return | |
| 1088 | + | } | |
| 1089 | + | ||
| 1090 | + | // Fall back to GetOrFetch (checks service cache, WebKit DB, then DuckDuckGo API) | |
| 1091 | + | // Capture current generation to guard against stale callbacks | |
| 1092 | + | gen := wv.Generation() | |
| 1093 | + | capturedURI := uri | |
| 1094 | + | c.faviconAdapter.GetOrFetch(ctx, uri, func(texture *gdk.Texture) { | |
| 1095 | + | // Skip nil results — a nil means "couldn't resolve", not "no favicon". | |
| 1096 | + | // Without this guard, a late nil callback can overwrite a good favicon | |
| 1097 | + | // that was already set by an earlier onFaviconChanged signal. | |
| 1098 | + | if texture == nil { | |
| 1099 | + | return | |
| 1100 | + | } | |
| 1101 | + | // Verify WebView is still bound to pane and hasn't been reused | |
| 1102 | + | if wv.Generation() != gen { | |
| 1103 | + | return | |
| 1104 | + | } | |
| 1105 | + | currentWV := c.getWebViewLocked(paneID) | |
| 1106 | + | if currentWV != wv { | |
| 1107 | + | return | |
| 1108 | + | } | |
| 1109 | + | // Verify the WebView hasn't navigated away since we started the fetch | |
| 1110 | + | if currentWV.URI() != capturedURI { | |
| 1111 | + | return | |
| 1112 | + | } | |
| 1113 | + | c.updateStackedFaviconForPane(ctx, paneID, texture) | |
| 1114 | + | }) | |
| 1115 | + | } | |
| 1116 | + | ||
| 1045 | 1117 | // FaviconAdapter returns the favicon adapter for external use (e.g., omnibox). | |
| 1046 | 1118 | func (c *ContentCoordinator) FaviconAdapter() *adapter.FaviconAdapter { | |
| 1047 | 1119 | return c.faviconAdapter | |
| @@ -1087,7 +1159,11 @@ func (c *ContentCoordinator) PreloadCachedFavicon(ctx context.Context, paneID en | |||
| 1087 | 1159 | if c.getActiveWS == nil { | |
| 1088 | 1160 | return false | |
| 1089 | 1161 | } | |
| 1090 | - | // A nil texture triggers the default icon fallback, which avoids stale favicons. | |
| 1162 | + | // Only update if we found a cached texture. | |
| 1163 | + | // If nil, let resolveCommittedFavicon handle it after load commits. | |
| 1164 | + | if texture == nil { | |
| 1165 | + | return false | |
| 1166 | + | } | |
| 1091 | 1167 | _, wsView := c.getActiveWS() | |
| 1092 | 1168 | if wsView == nil { | |
| 1093 | 1169 | return false | |
| @@ -1190,6 +1266,9 @@ func (c *ContentCoordinator) onLoadCommitted(ctx context.Context, paneID entity. | |||
| 1190 | 1266 | c.onHistoryRecord(ctx, paneID, uri) | |
| 1191 | 1267 | } | |
| 1192 | 1268 | ||
| 1269 | + | // Resolve favicon for stacked title bar (fills gap when WebKit doesn't emit notify::favicon) | |
| 1270 | + | c.resolveCommittedFavicon(ctx, paneID, wv) | |
| 1271 | + | ||
| 1193 | 1272 | // Notify active pane navigation for permission indicator reset. | |
| 1194 | 1273 | c.notifyActiveNavigation(paneID, uri) | |
| 1195 | 1274 | ||
| @@ -1675,9 +1754,13 @@ func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID e | |||
| 1675 | 1754 | c.onTitleChanged(ctx, paneID, title) | |
| 1676 | 1755 | } | |
| 1677 | 1756 | ||
| 1678 | - | // Favicon changes | |
| 1757 | + | // Favicon changes - capture generation to detect stale callbacks after pool reuse | |
| 1758 | + | faviconGen := wv.Generation() | |
| 1679 | 1759 | wv.OnFaviconChanged = func(favicon *gdk.Texture) { | |
| 1680 | - | c.onFaviconChanged(ctx, paneID, favicon) | |
| 1760 | + | if wv.Generation() != faviconGen { | |
| 1761 | + | return // WebView was reused, ignore stale signal | |
| 1762 | + | } | |
| 1763 | + | c.onFaviconChanged(ctx, paneID, wv, favicon) | |
| 1681 | 1764 | } | |
| 1682 | 1765 | ||
| 1683 | 1766 | // Load events | |
| @@ -982,15 +982,19 @@ func (c *ContentCoordinator) syncStackedTitle(ctx context.Context, paneID entity | |||||
| 982 | } | 982 | } | ||
| 983 | 983 | ||||
| 984 | // onFaviconChanged updates favicon tracking when a WebView's favicon changes. | 984 | // onFaviconChanged updates favicon tracking when a WebView's favicon changes. | ||
| 985 | - | func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity.PaneID, favicon *gdk.Texture) { | 985 | + | func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity.PaneID, emittingWV *webkit.WebView, favicon *gdk.Texture) { |
| 986 | log := logging.FromContext(ctx) | 986 | log := logging.FromContext(ctx) | ||
| 987 | 987 | ||||
| 988 | - | // Get current URI to extract domain for caching | 988 | + | // Verify this WebView is still bound to the expected pane |
| 989 | - | wv := c.getWebViewLocked(paneID) | 989 | + | currentWV := c.getWebViewLocked(paneID) |
| 990 | - | if wv == nil { | 990 | + | if currentWV == nil || currentWV != emittingWV { |
| 991 | + | log.Debug(). | |||
| 992 | + | Str("pane_id", string(paneID)). | |||
| 993 | + | Msg("ignoring favicon change from unbound webview") | |||
| 991 | return | 994 | return | ||
| 992 | } | 995 | } | ||
| 993 | - | uri := wv.URI() | 996 | + | |
| 997 | + | uri := emittingWV.URI() | |||
| 994 | 998 | ||||
| 995 | // Update favicon cache with domain key (handles cross-domain redirects) | 999 | // Update favicon cache with domain key (handles cross-domain redirects) | ||
| 996 | if c.faviconAdapter != nil && favicon != nil && uri != "" { | 1000 | if c.faviconAdapter != nil && favicon != nil && uri != "" { | ||
| @@ -1001,16 +1005,7 @@ func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity | |||||
| 1001 | } | 1005 | } | ||
| 1002 | 1006 | ||||
| 1003 | // Update StackedView favicon if this pane is in a stack | 1007 | // Update StackedView favicon if this pane is in a stack | ||
| 1004 | - | _, wsView := c.getActiveWS() | 1008 | + | c.updateStackedFaviconForPane(ctx, paneID, favicon) |
| 1005 | - | if wsView != nil { | |||
| 1006 | - | tr := wsView.TreeRenderer() | |||
| 1007 | - | if tr != nil { | |||
| 1008 | - | stackedView := tr.GetStackedViewForPane(string(paneID)) | |||
| 1009 | - | if stackedView != nil { | |||
| 1010 | - | c.updateStackedPaneFavicon(ctx, stackedView, paneID, favicon) | |||
| 1011 | - | } | |||
| 1012 | - | } | |||
| 1013 | - | } | |||
| 1014 | 1009 | ||||
| 1015 | log.Debug(). | 1010 | log.Debug(). | ||
| 1016 | Str("pane_id", string(paneID)). | 1011 | Str("pane_id", string(paneID)). | ||
| @@ -1042,6 +1037,83 @@ func (c *ContentCoordinator) updateStackedPaneFavicon( | |||||
| 1042 | } | 1037 | } | ||
| 1043 | } | 1038 | } | ||
| 1044 | 1039 | ||||
| 1040 | + | // updateStackedFaviconForPane updates the stacked title bar favicon for a pane. | |||
| 1041 | + | func (c *ContentCoordinator) updateStackedFaviconForPane(ctx context.Context, paneID entity.PaneID, texture *gdk.Texture) { | |||
| 1042 | + | _, wsView := c.getActiveWS() | |||
| 1043 | + | if wsView == nil { | |||
| 1044 | + | return | |||
| 1045 | + | } | |||
| 1046 | + | tr := wsView.TreeRenderer() | |||
| 1047 | + | if tr == nil { | |||
| 1048 | + | return | |||
| 1049 | + | } | |||
| 1050 | + | if sv := tr.GetStackedViewForPane(string(paneID)); sv != nil { | |||
| 1051 | + | c.updateStackedPaneFavicon(ctx, sv, paneID, texture) | |||
| 1052 | + | } | |||
| 1053 | + | } | |||
| 1054 | + | ||||
| 1055 | + | // resolveCommittedFavicon ensures the stacked title bar has a favicon after navigation commits. | |||
| 1056 | + | // First checks if WebKit already has a favicon for the page (common for subpath URLs). | |||
| 1057 | + | // Falls back to the full GetOrFetch pipeline (cache + WebKit DB + DuckDuckGo API). | |||
| 1058 | + | // This closes the gap where title bars relied solely on the notify::favicon signal. | |||
| 1059 | + | func (c *ContentCoordinator) resolveCommittedFavicon(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | |||
| 1060 | + | if c.faviconAdapter == nil || wv == nil { | |||
| 1061 | + | return | |||
| 1062 | + | } | |||
| 1063 | + | ||||
| 1064 | + | uri := wv.URI() | |||
| 1065 | + | if uri == "" || strings.HasPrefix(uri, "about:") || strings.HasPrefix(uri, "dumb://") { | |||
| 1066 | + | return | |||
| 1067 | + | } | |||
| 1068 | + | ||||
| 1069 | + | log := logging.FromContext(ctx) | |||
| 1070 | + | ||||
| 1071 | + | // Check if we already have a texture cached for this domain | |||
| 1072 | + | if texture := c.faviconAdapter.GetTextureByURL(uri); texture != nil { | |||
| 1073 | + | // Already cached - just ensure the stacked view is updated | |||
| 1074 | + | c.updateStackedFaviconForPane(ctx, paneID, texture) | |||
| 1075 | + | return | |||
| 1076 | + | } | |||
| 1077 | + | ||||
| 1078 | + | // Check if WebKit already has a favicon for this page | |||
| 1079 | + | if icon := wv.Favicon(); icon != nil { | |||
| 1080 | + | log.Debug().Str("pane_id", string(paneID)).Str("uri", uri).Msg("using existing WebKit favicon for committed page") | |||
| 1081 | + | // Store and update through the normal path | |||
| 1082 | + | c.navOriginMu.RLock() | |||
| 1083 | + | originURL := c.navOrigins[paneID] | |||
| 1084 | + | c.navOriginMu.RUnlock() | |||
| 1085 | + | c.faviconAdapter.StoreFromWebKitWithOrigin(ctx, uri, originURL, icon) | |||
| 1086 | + | c.updateStackedFaviconForPane(ctx, paneID, icon) | |||
| 1087 | + | return | |||
| 1088 | + | } | |||
| 1089 | + | ||||
| 1090 | + | // Fall back to GetOrFetch (checks service cache, WebKit DB, then DuckDuckGo API) | |||
| 1091 | + | // Capture current generation to guard against stale callbacks | |||
| 1092 | + | gen := wv.Generation() | |||
| 1093 | + | capturedURI := uri | |||
| 1094 | + | c.faviconAdapter.GetOrFetch(ctx, uri, func(texture *gdk.Texture) { | |||
| 1095 | + | // Skip nil results — a nil means "couldn't resolve", not "no favicon". | |||
| 1096 | + | // Without this guard, a late nil callback can overwrite a good favicon | |||
| 1097 | + | // that was already set by an earlier onFaviconChanged signal. | |||
| 1098 | + | if texture == nil { | |||
| 1099 | + | return | |||
| 1100 | + | } | |||
| 1101 | + | // Verify WebView is still bound to pane and hasn't been reused | |||
| 1102 | + | if wv.Generation() != gen { | |||
| 1103 | + | return | |||
| 1104 | + | } | |||
| 1105 | + | currentWV := c.getWebViewLocked(paneID) | |||
| 1106 | + | if currentWV != wv { | |||
| 1107 | + | return | |||
| 1108 | + | } | |||
| 1109 | + | // Verify the WebView hasn't navigated away since we started the fetch | |||
| 1110 | + | if currentWV.URI() != capturedURI { | |||
| 1111 | + | return | |||
| 1112 | + | } | |||
| 1113 | + | c.updateStackedFaviconForPane(ctx, paneID, texture) | |||
| 1114 | + | }) | |||
| 1115 | + | } | |||
| 1116 | + | ||||
| 1045 | // FaviconAdapter returns the favicon adapter for external use (e.g., omnibox). | 1117 | // FaviconAdapter returns the favicon adapter for external use (e.g., omnibox). | ||
| 1046 | func (c *ContentCoordinator) FaviconAdapter() *adapter.FaviconAdapter { | 1118 | func (c *ContentCoordinator) FaviconAdapter() *adapter.FaviconAdapter { | ||
| 1047 | return c.faviconAdapter | 1119 | return c.faviconAdapter | ||
| @@ -1087,7 +1159,11 @@ func (c *ContentCoordinator) PreloadCachedFavicon(ctx context.Context, paneID en | |||||
| 1087 | if c.getActiveWS == nil { | 1159 | if c.getActiveWS == nil { | ||
| 1088 | return false | 1160 | return false | ||
| 1089 | } | 1161 | } | ||
| 1090 | - | // A nil texture triggers the default icon fallback, which avoids stale favicons. | 1162 | + | // Only update if we found a cached texture. |
| 1163 | + | // If nil, let resolveCommittedFavicon handle it after load commits. | |||
| 1164 | + | if texture == nil { | |||
| 1165 | + | return false | |||
| 1166 | + | } | |||
| 1091 | _, wsView := c.getActiveWS() | 1167 | _, wsView := c.getActiveWS() | ||
| 1092 | if wsView == nil { | 1168 | if wsView == nil { | ||
| 1093 | return false | 1169 | return false | ||
| @@ -1190,6 +1266,9 @@ func (c *ContentCoordinator) onLoadCommitted(ctx context.Context, paneID entity. | |||||
| 1190 | c.onHistoryRecord(ctx, paneID, uri) | 1266 | c.onHistoryRecord(ctx, paneID, uri) | ||
| 1191 | } | 1267 | } | ||
| 1192 | 1268 | ||||
| 1269 | + | // Resolve favicon for stacked title bar (fills gap when WebKit doesn't emit notify::favicon) | |||
| 1270 | + | c.resolveCommittedFavicon(ctx, paneID, wv) | |||
| 1271 | + | ||||
| 1193 | // Notify active pane navigation for permission indicator reset. | 1272 | // Notify active pane navigation for permission indicator reset. | ||
| 1194 | c.notifyActiveNavigation(paneID, uri) | 1273 | c.notifyActiveNavigation(paneID, uri) | ||
| 1195 | 1274 | ||||
| @@ -1675,9 +1754,13 @@ func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID e | |||||
| 1675 | c.onTitleChanged(ctx, paneID, title) | 1754 | c.onTitleChanged(ctx, paneID, title) | ||
| 1676 | } | 1755 | } | ||
| 1677 | 1756 | ||||
| 1678 | - | // Favicon changes | 1757 | + | // Favicon changes - capture generation to detect stale callbacks after pool reuse |
| 1758 | + | faviconGen := wv.Generation() | |||
| 1679 | wv.OnFaviconChanged = func(favicon *gdk.Texture) { | 1759 | wv.OnFaviconChanged = func(favicon *gdk.Texture) { | ||
| 1680 | - | c.onFaviconChanged(ctx, paneID, favicon) | 1760 | + | if wv.Generation() != faviconGen { |
| 1761 | + | return // WebView was reused, ignore stale signal | |||
| 1762 | + | } | |||
| 1763 | + | c.onFaviconChanged(ctx, paneID, wv, favicon) | |||
| 1681 | } | 1764 | } | ||
| 1682 | 1765 | ||||
| 1683 | // Load events | 1766 | // Load events | ||
M internal/ui/coordinator/workspace.go +30 -0
| @@ -1713,6 +1713,36 @@ func (c *WorkspaceCoordinator) SetupStackedPaneCallbacks(ctx context.Context, ws | |||
| 1713 | 1713 | c.onStackedPaneClose(ctx, entity.PaneID(paneID)) | |
| 1714 | 1714 | }) | |
| 1715 | 1715 | ||
| 1716 | + | // Populate stacked title bar favicons from cache. | |
| 1717 | + | // When panes are stacked (split → stack conversion) or restored from session, | |
| 1718 | + | // the title bars are created with default icons. We need to apply any cached | |
| 1719 | + | // favicon textures so they display correctly without waiting for a new navigation. | |
| 1720 | + | if fa := c.contentCoord.FaviconAdapter(); fa != nil { | |
| 1721 | + | for i, child := range node.Children { | |
| 1722 | + | if child.Pane == nil { | |
| 1723 | + | continue | |
| 1724 | + | } | |
| 1725 | + | // Try to get the pane's current URI from its WebView | |
| 1726 | + | uri := child.Pane.URI | |
| 1727 | + | if wv := c.contentCoord.GetWebView(child.Pane.ID); wv != nil { | |
| 1728 | + | if currentURI := wv.URI(); currentURI != "" { | |
| 1729 | + | uri = currentURI | |
| 1730 | + | } | |
| 1731 | + | } | |
| 1732 | + | if uri == "" { | |
| 1733 | + | continue | |
| 1734 | + | } | |
| 1735 | + | if texture := fa.GetTextureByURL(uri); texture != nil { | |
| 1736 | + | if err := stackedView.UpdateFaviconTexture(i, texture); err == nil { | |
| 1737 | + | log.Debug(). | |
| 1738 | + | Str("pane_id", string(child.Pane.ID)). | |
| 1739 | + | Str("domain", domainurl.ExtractDomain(uri)). | |
| 1740 | + | Msg("populated stacked title bar favicon from cache") | |
| 1741 | + | } | |
| 1742 | + | } | |
| 1743 | + | } | |
| 1744 | + | } | |
| 1745 | + | ||
| 1716 | 1746 | log.Debug(). | |
| 1717 | 1747 | Str("stack_id", node.ID). | |
| 1718 | 1748 | Int("stack_size", len(node.Children)). | |
| @@ -1713,6 +1713,36 @@ func (c *WorkspaceCoordinator) SetupStackedPaneCallbacks(ctx context.Context, ws | |||||
| 1713 | c.onStackedPaneClose(ctx, entity.PaneID(paneID)) | 1713 | c.onStackedPaneClose(ctx, entity.PaneID(paneID)) | ||
| 1714 | }) | 1714 | }) | ||
| 1715 | 1715 | ||||
| 1716 | + | // Populate stacked title bar favicons from cache. | |||
| 1717 | + | // When panes are stacked (split → stack conversion) or restored from session, | |||
| 1718 | + | // the title bars are created with default icons. We need to apply any cached | |||
| 1719 | + | // favicon textures so they display correctly without waiting for a new navigation. | |||
| 1720 | + | if fa := c.contentCoord.FaviconAdapter(); fa != nil { | |||
| 1721 | + | for i, child := range node.Children { | |||
| 1722 | + | if child.Pane == nil { | |||
| 1723 | + | continue | |||
| 1724 | + | } | |||
| 1725 | + | // Try to get the pane's current URI from its WebView | |||
| 1726 | + | uri := child.Pane.URI | |||
| 1727 | + | if wv := c.contentCoord.GetWebView(child.Pane.ID); wv != nil { | |||
| 1728 | + | if currentURI := wv.URI(); currentURI != "" { | |||
| 1729 | + | uri = currentURI | |||
| 1730 | + | } | |||
| 1731 | + | } | |||
| 1732 | + | if uri == "" { | |||
| 1733 | + | continue | |||
| 1734 | + | } | |||
| 1735 | + | if texture := fa.GetTextureByURL(uri); texture != nil { | |||
| 1736 | + | if err := stackedView.UpdateFaviconTexture(i, texture); err == nil { | |||
| 1737 | + | log.Debug(). | |||
| 1738 | + | Str("pane_id", string(child.Pane.ID)). | |||
| 1739 | + | Str("domain", domainurl.ExtractDomain(uri)). | |||
| 1740 | + | Msg("populated stacked title bar favicon from cache") | |||
| 1741 | + | } | |||
| 1742 | + | } | |||
| 1743 | + | } | |||
| 1744 | + | } | |||
| 1745 | + | ||||
| 1716 | log.Debug(). | 1746 | log.Debug(). | ||
| 1717 | Str("stack_id", node.ID). | 1747 | Str("stack_id", node.ID). | ||
| 1718 | Int("stack_size", len(node.Children)). | 1748 | Int("stack_size", len(node.Children)). | ||
M internal/ui/input/global_shortcuts.go +125 -0
| @@ -18,6 +18,8 @@ import ( | |||
| 18 | 18 | // to intercept shortcuts before they reach the WebView. | |
| 19 | 19 | type GlobalShortcutHandler struct { | |
| 20 | 20 | controller *gtk.ShortcutController | |
| 21 | + | window *gtk.ApplicationWindow | |
| 22 | + | kbHandler *KeyboardHandler | |
| 21 | 23 | onAction ActionHandler | |
| 22 | 24 | ctx context.Context | |
| 23 | 25 | registered map[KeyBinding]Action | |
| @@ -32,6 +34,7 @@ func NewGlobalShortcutHandler( | |||
| 32 | 34 | ctx context.Context, | |
| 33 | 35 | window *gtk.ApplicationWindow, | |
| 34 | 36 | cfg *config.Config, | |
| 37 | + | kbHandler *KeyboardHandler, | |
| 35 | 38 | onAction ActionHandler, | |
| 36 | 39 | ) *GlobalShortcutHandler { | |
| 37 | 40 | log := logging.FromContext(ctx) | |
| @@ -39,6 +42,8 @@ func NewGlobalShortcutHandler( | |||
| 39 | 42 | ||
| 40 | 43 | h := &GlobalShortcutHandler{ | |
| 41 | 44 | controller: gtk.NewShortcutController(), | |
| 45 | + | window: window, | |
| 46 | + | kbHandler: kbHandler, | |
| 42 | 47 | onAction: onAction, | |
| 43 | 48 | ctx: ctx, | |
| 44 | 49 | callbacks: make([]gtk.ShortcutFunc, 0), | |
| @@ -140,6 +145,16 @@ func NewGlobalShortcutHandler( | |||
| 140 | 145 | Msg("registered floating profile global shortcut") | |
| 141 | 146 | } | |
| 142 | 147 | } | |
| 148 | + | ||
| 149 | + | // Register all app-reserved shortcuts (mode activations, Ctrl+L, Ctrl+F, etc.) | |
| 150 | + | // so they work even when WebView has focus. | |
| 151 | + | shortcuts := NewShortcutSet(ctx, cfg) | |
| 152 | + | for binding, action := range shortcuts.Global { | |
| 153 | + | if _, exists := h.registered[binding]; exists { | |
| 154 | + | continue | |
| 155 | + | } | |
| 156 | + | h.registerShortcut(binding.Keyval, gdk.ModifierType(binding.Modifiers), action) | |
| 157 | + | } | |
| 143 | 158 | } | |
| 144 | 159 | ||
| 145 | 160 | // Attach to window | |
| @@ -182,6 +197,17 @@ func (h *GlobalShortcutHandler) registerShortcut(keyval uint, modifiers gdk.Modi | |||
| 182 | 197 | Str("action", string(actionToDispatch)). | |
| 183 | 198 | Msg("global shortcut triggered") | |
| 184 | 199 | ||
| 200 | + | // Mode-enter/exit actions go through KeyboardHandler for modal state | |
| 201 | + | if isModeAction(actionToDispatch) { | |
| 202 | + | if h.kbHandler != nil { | |
| 203 | + | h.kbHandler.DispatchAction(actionToDispatch) | |
| 204 | + | return true | |
| 205 | + | } | |
| 206 | + | log.Warn(). | |
| 207 | + | Str("action", string(actionToDispatch)). | |
| 208 | + | Msg("mode action triggered but keyboard handler not set, falling through to default handler") | |
| 209 | + | } | |
| 210 | + | ||
| 185 | 211 | if h.onAction != nil { | |
| 186 | 212 | if err := h.onAction(h.ctx, actionToDispatch); err != nil { | |
| 187 | 213 | log.Error(). | |
| @@ -220,6 +246,96 @@ func (h *GlobalShortcutHandler) registerShortcut(keyval uint, modifiers gdk.Modi | |||
| 220 | 246 | return true | |
| 221 | 247 | } | |
| 222 | 248 | ||
| 249 | + | // ReloadShortcuts rebuilds global shortcuts from a new config. | |
| 250 | + | // It replaces the GTK shortcut controller to ensure stale shortcuts | |
| 251 | + | // are removed. | |
| 252 | + | func (h *GlobalShortcutHandler) ReloadShortcuts(ctx context.Context, cfg *config.Config) { | |
| 253 | + | log := logging.FromContext(ctx) | |
| 254 | + | ||
| 255 | + | if h.window == nil || h.controller == nil { | |
| 256 | + | return | |
| 257 | + | } | |
| 258 | + | ||
| 259 | + | // Remove old controller from window | |
| 260 | + | h.window.RemoveController(&h.controller.EventController) | |
| 261 | + | ||
| 262 | + | // Create fresh controller | |
| 263 | + | h.controller = gtk.NewShortcutController() | |
| 264 | + | if h.controller == nil { | |
| 265 | + | log.Error().Msg("failed to create shortcut controller during reload") | |
| 266 | + | return | |
| 267 | + | } | |
| 268 | + | h.controller.SetScope(gtk.ShortcutScopeGlobalValue) | |
| 269 | + | h.callbacks = make([]gtk.ShortcutFunc, 0) | |
| 270 | + | h.registered = make(map[KeyBinding]Action) | |
| 271 | + | ||
| 272 | + | // Re-register hardcoded shortcuts (Alt+1-9, Alt+0, Alt+Tab, Ctrl+Shift+S) | |
| 273 | + | tabActions := []Action{ | |
| 274 | + | ActionSwitchTabIndex1, ActionSwitchTabIndex2, ActionSwitchTabIndex3, | |
| 275 | + | ActionSwitchTabIndex4, ActionSwitchTabIndex5, ActionSwitchTabIndex6, | |
| 276 | + | ActionSwitchTabIndex7, ActionSwitchTabIndex8, ActionSwitchTabIndex9, | |
| 277 | + | } | |
| 278 | + | for i, action := range tabActions { | |
| 279 | + | h.registerShortcut(uint(gdk.KEY_1)+uint(i), gdk.AltMaskValue, action) | |
| 280 | + | } | |
| 281 | + | h.registerShortcut(uint(gdk.KEY_0), gdk.AltMaskValue, ActionSwitchTabIndex10) | |
| 282 | + | h.registerShortcut(uint(gdk.KEY_Tab), gdk.AltMaskValue, ActionSwitchLastTab) | |
| 283 | + | h.registerShortcut(uint(gdk.KEY_s), gdk.ControlMaskValue|gdk.ShiftMaskValue, ActionOpenSessionManager) | |
| 284 | + | ||
| 285 | + | // Re-register config-driven shortcuts | |
| 286 | + | if cfg != nil { | |
| 287 | + | actionMap := map[string]Action{ | |
| 288 | + | "toggle_floating_pane": ActionToggleFloatingPane, | |
| 289 | + | "toggle-floating-pane": ActionToggleFloatingPane, | |
| 290 | + | "consume_or_expel_left": ActionConsumeOrExpelLeft, | |
| 291 | + | "consume_or_expel_right": ActionConsumeOrExpelRight, | |
| 292 | + | "consume_or_expel_up": ActionConsumeOrExpelUp, | |
| 293 | + | "consume_or_expel_down": ActionConsumeOrExpelDown, | |
| 294 | + | } | |
| 295 | + | ||
| 296 | + | for actionName, actionBinding := range cfg.Workspace.Shortcuts.Actions { | |
| 297 | + | action, ok := actionMap[actionName] | |
| 298 | + | if !ok { | |
| 299 | + | continue | |
| 300 | + | } | |
| 301 | + | for _, keyStr := range actionBinding.Keys { | |
| 302 | + | binding, ok := ParseKeyString(keyStr) | |
| 303 | + | if !ok { | |
| 304 | + | continue | |
| 305 | + | } | |
| 306 | + | h.registerShortcut(binding.Keyval, gdk.ModifierType(binding.Modifiers), action) | |
| 307 | + | } | |
| 308 | + | } | |
| 309 | + | ||
| 310 | + | occupied := make(map[KeyBinding]Action, len(h.registered)) | |
| 311 | + | for binding, action := range h.registered { | |
| 312 | + | occupied[binding] = action | |
| 313 | + | } | |
| 314 | + | for _, shortcut := range collectFloatingProfileShortcuts(ctx, cfg, occupied) { | |
| 315 | + | if h.registerShortcut(shortcut.Binding.Keyval, gdk.ModifierType(shortcut.Binding.Modifiers), shortcut.Action) { | |
| 316 | + | if url, ok := ParseFloatingProfileAction(shortcut.Action); ok { | |
| 317 | + | log.Trace().Str("shortcut", formatBinding(shortcut.Binding)).Str("url", url).Msg("registered floating profile global shortcut") | |
| 318 | + | } | |
| 319 | + | } | |
| 320 | + | } | |
| 321 | + | ||
| 322 | + | shortcuts := NewShortcutSet(ctx, cfg) | |
| 323 | + | for binding, action := range shortcuts.Global { | |
| 324 | + | if _, exists := h.registered[binding]; exists { | |
| 325 | + | continue | |
| 326 | + | } | |
| 327 | + | h.registerShortcut(binding.Keyval, gdk.ModifierType(binding.Modifiers), action) | |
| 328 | + | } | |
| 329 | + | } | |
| 330 | + | ||
| 331 | + | // Attach new controller to window | |
| 332 | + | h.window.AddController(&h.controller.EventController) | |
| 333 | + | ||
| 334 | + | log.Debug(). | |
| 335 | + | Int("shortcuts", len(h.callbacks)). | |
| 336 | + | Msg("global shortcuts reloaded") | |
| 337 | + | } | |
| 338 | + | ||
| 223 | 339 | // Detach removes the global shortcut handler from the window. | |
| 224 | 340 | // Note: GTK handles cleanup when the widget is destroyed, | |
| 225 | 341 | // but we clear our references here. | |
| @@ -247,3 +363,12 @@ func formatBinding(binding KeyBinding) string { | |||
| 247 | 363 | parts = append(parts, strings.ToLower(keyName)) | |
| 248 | 364 | return strings.Join(parts, "+") | |
| 249 | 365 | } | |
| 366 | + | ||
| 367 | + | func isModeAction(action Action) bool { | |
| 368 | + | switch action { | |
| 369 | + | case ActionEnterTabMode, ActionEnterPaneMode, ActionEnterSessionMode, ActionEnterResizeMode, ActionExitMode: | |
| 370 | + | return true | |
| 371 | + | default: | |
| 372 | + | return false | |
| 373 | + | } | |
| 374 | + | } | |
| @@ -18,6 +18,8 @@ import ( | |||||
| 18 | // to intercept shortcuts before they reach the WebView. | 18 | // to intercept shortcuts before they reach the WebView. | ||
| 19 | type GlobalShortcutHandler struct { | 19 | type GlobalShortcutHandler struct { | ||
| 20 | controller *gtk.ShortcutController | 20 | controller *gtk.ShortcutController | ||
| 21 | + | window *gtk.ApplicationWindow | |||
| 22 | + | kbHandler *KeyboardHandler | |||
| 21 | onAction ActionHandler | 23 | onAction ActionHandler | ||
| 22 | ctx context.Context | 24 | ctx context.Context | ||
| 23 | registered map[KeyBinding]Action | 25 | registered map[KeyBinding]Action | ||
| @@ -32,6 +34,7 @@ func NewGlobalShortcutHandler( | |||||
| 32 | ctx context.Context, | 34 | ctx context.Context, | ||
| 33 | window *gtk.ApplicationWindow, | 35 | window *gtk.ApplicationWindow, | ||
| 34 | cfg *config.Config, | 36 | cfg *config.Config, | ||
| 37 | + | kbHandler *KeyboardHandler, | |||
| 35 | onAction ActionHandler, | 38 | onAction ActionHandler, | ||
| 36 | ) *GlobalShortcutHandler { | 39 | ) *GlobalShortcutHandler { | ||
| 37 | log := logging.FromContext(ctx) | 40 | log := logging.FromContext(ctx) | ||
| @@ -39,6 +42,8 @@ func NewGlobalShortcutHandler( | |||||
| 39 | 42 | ||||
| 40 | h := &GlobalShortcutHandler{ | 43 | h := &GlobalShortcutHandler{ | ||
| 41 | controller: gtk.NewShortcutController(), | 44 | controller: gtk.NewShortcutController(), | ||
| 45 | + | window: window, | |||
| 46 | + | kbHandler: kbHandler, | |||
| 42 | onAction: onAction, | 47 | onAction: onAction, | ||
| 43 | ctx: ctx, | 48 | ctx: ctx, | ||
| 44 | callbacks: make([]gtk.ShortcutFunc, 0), | 49 | callbacks: make([]gtk.ShortcutFunc, 0), | ||
| @@ -140,6 +145,16 @@ func NewGlobalShortcutHandler( | |||||
| 140 | Msg("registered floating profile global shortcut") | 145 | Msg("registered floating profile global shortcut") | ||
| 141 | } | 146 | } | ||
| 142 | } | 147 | } | ||
| 148 | + | ||||
| 149 | + | // Register all app-reserved shortcuts (mode activations, Ctrl+L, Ctrl+F, etc.) | |||
| 150 | + | // so they work even when WebView has focus. | |||
| 151 | + | shortcuts := NewShortcutSet(ctx, cfg) | |||
| 152 | + | for binding, action := range shortcuts.Global { | |||
| 153 | + | if _, exists := h.registered[binding]; exists { | |||
| 154 | + | continue | |||
| 155 | + | } | |||
| 156 | + | h.registerShortcut(binding.Keyval, gdk.ModifierType(binding.Modifiers), action) | |||
| 157 | + | } | |||
| 143 | } | 158 | } | ||
| 144 | 159 | ||||
| 145 | // Attach to window | 160 | // Attach to window | ||
| @@ -182,6 +197,17 @@ func (h *GlobalShortcutHandler) registerShortcut(keyval uint, modifiers gdk.Modi | |||||
| 182 | Str("action", string(actionToDispatch)). | 197 | Str("action", string(actionToDispatch)). | ||
| 183 | Msg("global shortcut triggered") | 198 | Msg("global shortcut triggered") | ||
| 184 | 199 | ||||
| 200 | + | // Mode-enter/exit actions go through KeyboardHandler for modal state | |||
| 201 | + | if isModeAction(actionToDispatch) { | |||
| 202 | + | if h.kbHandler != nil { | |||
| 203 | + | h.kbHandler.DispatchAction(actionToDispatch) | |||
| 204 | + | return true | |||
| 205 | + | } | |||
| 206 | + | log.Warn(). | |||
| 207 | + | Str("action", string(actionToDispatch)). | |||
| 208 | + | Msg("mode action triggered but keyboard handler not set, falling through to default handler") | |||
| 209 | + | } | |||
| 210 | + | ||||
| 185 | if h.onAction != nil { | 211 | if h.onAction != nil { | ||
| 186 | if err := h.onAction(h.ctx, actionToDispatch); err != nil { | 212 | if err := h.onAction(h.ctx, actionToDispatch); err != nil { | ||
| 187 | log.Error(). | 213 | log.Error(). | ||
| @@ -220,6 +246,96 @@ func (h *GlobalShortcutHandler) registerShortcut(keyval uint, modifiers gdk.Modi | |||||
| 220 | return true | 246 | return true | ||
| 221 | } | 247 | } | ||
| 222 | 248 | ||||
| 249 | + | // ReloadShortcuts rebuilds global shortcuts from a new config. | |||
| 250 | + | // It replaces the GTK shortcut controller to ensure stale shortcuts | |||
| 251 | + | // are removed. | |||
| 252 | + | func (h *GlobalShortcutHandler) ReloadShortcuts(ctx context.Context, cfg *config.Config) { | |||
| 253 | + | log := logging.FromContext(ctx) | |||
| 254 | + | ||||
| 255 | + | if h.window == nil || h.controller == nil { | |||
| 256 | + | return | |||
| 257 | + | } | |||
| 258 | + | ||||
| 259 | + | // Remove old controller from window | |||
| 260 | + | h.window.RemoveController(&h.controller.EventController) | |||
| 261 | + | ||||
| 262 | + | // Create fresh controller | |||
| 263 | + | h.controller = gtk.NewShortcutController() | |||
| 264 | + | if h.controller == nil { | |||
| 265 | + | log.Error().Msg("failed to create shortcut controller during reload") | |||
| 266 | + | return | |||
| 267 | + | } | |||
| 268 | + | h.controller.SetScope(gtk.ShortcutScopeGlobalValue) | |||
| 269 | + | h.callbacks = make([]gtk.ShortcutFunc, 0) | |||
| 270 | + | h.registered = make(map[KeyBinding]Action) | |||
| 271 | + | ||||
| 272 | + | // Re-register hardcoded shortcuts (Alt+1-9, Alt+0, Alt+Tab, Ctrl+Shift+S) | |||
| 273 | + | tabActions := []Action{ | |||
| 274 | + | ActionSwitchTabIndex1, ActionSwitchTabIndex2, ActionSwitchTabIndex3, | |||
| 275 | + | ActionSwitchTabIndex4, ActionSwitchTabIndex5, ActionSwitchTabIndex6, | |||
| 276 | + | ActionSwitchTabIndex7, ActionSwitchTabIndex8, ActionSwitchTabIndex9, | |||
| 277 | + | } | |||
| 278 | + | for i, action := range tabActions { | |||
| 279 | + | h.registerShortcut(uint(gdk.KEY_1)+uint(i), gdk.AltMaskValue, action) | |||
| 280 | + | } | |||
| 281 | + | h.registerShortcut(uint(gdk.KEY_0), gdk.AltMaskValue, ActionSwitchTabIndex10) | |||
| 282 | + | h.registerShortcut(uint(gdk.KEY_Tab), gdk.AltMaskValue, ActionSwitchLastTab) | |||
| 283 | + | h.registerShortcut(uint(gdk.KEY_s), gdk.ControlMaskValue|gdk.ShiftMaskValue, ActionOpenSessionManager) | |||
| 284 | + | ||||
| 285 | + | // Re-register config-driven shortcuts | |||
| 286 | + | if cfg != nil { | |||
| 287 | + | actionMap := map[string]Action{ | |||
| 288 | + | "toggle_floating_pane": ActionToggleFloatingPane, | |||
| 289 | + | "toggle-floating-pane": ActionToggleFloatingPane, | |||
| 290 | + | "consume_or_expel_left": ActionConsumeOrExpelLeft, | |||
| 291 | + | "consume_or_expel_right": ActionConsumeOrExpelRight, | |||
| 292 | + | "consume_or_expel_up": ActionConsumeOrExpelUp, | |||
| 293 | + | "consume_or_expel_down": ActionConsumeOrExpelDown, | |||
| 294 | + | } | |||
| 295 | + | ||||
| 296 | + | for actionName, actionBinding := range cfg.Workspace.Shortcuts.Actions { | |||
| 297 | + | action, ok := actionMap[actionName] | |||
| 298 | + | if !ok { | |||
| 299 | + | continue | |||
| 300 | + | } | |||
| 301 | + | for _, keyStr := range actionBinding.Keys { | |||
| 302 | + | binding, ok := ParseKeyString(keyStr) | |||
| 303 | + | if !ok { | |||
| 304 | + | continue | |||
| 305 | + | } | |||
| 306 | + | h.registerShortcut(binding.Keyval, gdk.ModifierType(binding.Modifiers), action) | |||
| 307 | + | } | |||
| 308 | + | } | |||
| 309 | + | ||||
| 310 | + | occupied := make(map[KeyBinding]Action, len(h.registered)) | |||
| 311 | + | for binding, action := range h.registered { | |||
| 312 | + | occupied[binding] = action | |||
| 313 | + | } | |||
| 314 | + | for _, shortcut := range collectFloatingProfileShortcuts(ctx, cfg, occupied) { | |||
| 315 | + | if h.registerShortcut(shortcut.Binding.Keyval, gdk.ModifierType(shortcut.Binding.Modifiers), shortcut.Action) { | |||
| 316 | + | if url, ok := ParseFloatingProfileAction(shortcut.Action); ok { | |||
| 317 | + | log.Trace().Str("shortcut", formatBinding(shortcut.Binding)).Str("url", url).Msg("registered floating profile global shortcut") | |||
| 318 | + | } | |||
| 319 | + | } | |||
| 320 | + | } | |||
| 321 | + | ||||
| 322 | + | shortcuts := NewShortcutSet(ctx, cfg) | |||
| 323 | + | for binding, action := range shortcuts.Global { | |||
| 324 | + | if _, exists := h.registered[binding]; exists { | |||
| 325 | + | continue | |||
| 326 | + | } | |||
| 327 | + | h.registerShortcut(binding.Keyval, gdk.ModifierType(binding.Modifiers), action) | |||
| 328 | + | } | |||
| 329 | + | } | |||
| 330 | + | ||||
| 331 | + | // Attach new controller to window | |||
| 332 | + | h.window.AddController(&h.controller.EventController) | |||
| 333 | + | ||||
| 334 | + | log.Debug(). | |||
| 335 | + | Int("shortcuts", len(h.callbacks)). | |||
| 336 | + | Msg("global shortcuts reloaded") | |||
| 337 | + | } | |||
| 338 | + | ||||
| 223 | // Detach removes the global shortcut handler from the window. | 339 | // Detach removes the global shortcut handler from the window. | ||
| 224 | // Note: GTK handles cleanup when the widget is destroyed, | 340 | // Note: GTK handles cleanup when the widget is destroyed, | ||
| 225 | // but we clear our references here. | 341 | // but we clear our references here. | ||
| @@ -247,3 +363,12 @@ func formatBinding(binding KeyBinding) string { | |||||
| 247 | parts = append(parts, strings.ToLower(keyName)) | 363 | parts = append(parts, strings.ToLower(keyName)) | ||
| 248 | return strings.Join(parts, "+") | 364 | return strings.Join(parts, "+") | ||
| 249 | } | 365 | } | ||
| 366 | + | ||||
| 367 | + | func isModeAction(action Action) bool { | |||
| 368 | + | switch action { | |||
| 369 | + | case ActionEnterTabMode, ActionEnterPaneMode, ActionEnterSessionMode, ActionEnterResizeMode, ActionExitMode: | |||
| 370 | + | return true | |||
| 371 | + | default: | |||
| 372 | + | return false | |||
| 373 | + | } | |||
| 374 | + | } | |||
M internal/ui/input/keyboard.go +69 -8
| @@ -26,6 +26,7 @@ import ( | |||
| 26 | 26 | "github.com/bnema/dumber/internal/infrastructure/config" | |
| 27 | 27 | "github.com/bnema/dumber/internal/logging" | |
| 28 | 28 | "github.com/jwijenbergh/puregotk/v4/gdk" | |
| 29 | + | "github.com/jwijenbergh/puregotk/v4/glib" | |
| 29 | 30 | "github.com/jwijenbergh/puregotk/v4/gtk" | |
| 30 | 31 | "github.com/rs/zerolog" | |
| 31 | 32 | ) | |
| @@ -116,7 +117,35 @@ func (h *KeyboardHandler) ReloadShortcuts(ctx context.Context, cfg *config.Confi | |||
| 116 | 117 | ||
| 117 | 118 | // SetOnModeChange sets the callback for mode changes (for UI updates). | |
| 118 | 119 | func (h *KeyboardHandler) SetOnModeChange(fn func(from, to Mode)) { | |
| 119 | - | h.modal.SetOnModeChange(fn) | |
| 120 | + | h.modal.SetOnModeChange(func(from, to Mode) { | |
| 121 | + | // Switch controller phase: capture during modal, bubble for normal. | |
| 122 | + | // This ensures plain modal keys (s, h, etc.) reach the handler | |
| 123 | + | // before WebView, while normal typing still goes through WebKit IM. | |
| 124 | + | if to == ModeNormal { | |
| 125 | + | h.setControllerPhase(gtk.PhaseBubbleValue) | |
| 126 | + | } else if from == ModeNormal { | |
| 127 | + | h.setControllerPhase(gtk.PhaseCaptureValue) | |
| 128 | + | } | |
| 129 | + | // Forward to app-level callback | |
| 130 | + | if fn != nil { | |
| 131 | + | fn(from, to) | |
| 132 | + | } | |
| 133 | + | }) | |
| 134 | + | } | |
| 135 | + | ||
| 136 | + | // setControllerPhase changes the propagation phase of the key controller. | |
| 137 | + | // Capture phase during modal modes so plain keys reach the handler before WebView. | |
| 138 | + | // Bubble phase in normal mode for WebKit IM/dead-key support. | |
| 139 | + | // | |
| 140 | + | // This is called synchronously from onModeChange so the phase switch takes | |
| 141 | + | // effect before the next key event. The caller must ensure GTK thread safety; | |
| 142 | + | // modal timeouts dispatch to the GTK main thread via glib.IdleAdd before | |
| 143 | + | // calling ExitMode, so the onModeChange callback always runs on the GTK thread. | |
| 144 | + | func (h *KeyboardHandler) setControllerPhase(phase gtk.PropagationPhase) { | |
| 145 | + | if h.controller == nil { | |
| 146 | + | return | |
| 147 | + | } | |
| 148 | + | h.controller.SetPropagationPhase(phase) | |
| 120 | 149 | } | |
| 121 | 150 | ||
| 122 | 151 | // SetRouteKey sets the callback that determines how each key event should be routed. | |
| @@ -175,6 +204,17 @@ func (h *KeyboardHandler) AttachTo(window *gtk.ApplicationWindow) { | |||
| 175 | 204 | // so app shortcuts still work. | |
| 176 | 205 | h.controller.SetPropagationPhase(gtk.PhaseBubbleValue) | |
| 177 | 206 | ||
| 207 | + | // Wire GTK main thread scheduler for modal timeouts. Timer goroutines | |
| 208 | + | // must dispatch ExitMode to the GTK thread because onModeChange may | |
| 209 | + | // call setControllerPhase (a GTK operation). | |
| 210 | + | h.modal.SetMainThreadScheduler(func(fn func()) { | |
| 211 | + | cb := glib.SourceFunc(func(_ uintptr) bool { | |
| 212 | + | fn() | |
| 213 | + | return false | |
| 214 | + | }) | |
| 215 | + | glib.IdleAdd(&cb, 0) | |
| 216 | + | }) | |
| 217 | + | ||
| 178 | 218 | // Connect key pressed handler (retain callback to prevent GC). | |
| 179 | 219 | // The callback receives: keyval (translated key), keycode (hardware key position), state (modifiers) | |
| 180 | 220 | h.keyPressedCb = func(_ gtk.EventControllerKey, keyval uint, keycode uint, state gdk.ModifierType) bool { | |
| @@ -392,17 +432,21 @@ func isResizeAction(action Action) bool { | |||
| 392 | 432 | } | |
| 393 | 433 | ||
| 394 | 434 | func (h *KeyboardHandler) handleModeAction(action Action) bool { | |
| 435 | + | h.mu.RLock() | |
| 436 | + | cfg := h.cfg | |
| 437 | + | h.mu.RUnlock() | |
| 438 | + | ||
| 395 | 439 | switch action { | |
| 396 | 440 | case ActionEnterTabMode: | |
| 397 | - | timeout := time.Duration(h.cfg.Workspace.TabMode.TimeoutMilliseconds) * time.Millisecond | |
| 441 | + | timeout := time.Duration(cfg.Workspace.TabMode.TimeoutMilliseconds) * time.Millisecond | |
| 398 | 442 | h.modal.EnterTabMode(h.ctx, timeout) | |
| 399 | 443 | return true | |
| 400 | 444 | case ActionEnterPaneMode: | |
| 401 | - | timeout := time.Duration(h.cfg.Workspace.PaneMode.TimeoutMilliseconds) * time.Millisecond | |
| 445 | + | timeout := time.Duration(cfg.Workspace.PaneMode.TimeoutMilliseconds) * time.Millisecond | |
| 402 | 446 | h.modal.EnterPaneMode(h.ctx, timeout) | |
| 403 | 447 | return true | |
| 404 | 448 | case ActionEnterSessionMode: | |
| 405 | - | timeout := time.Duration(h.cfg.Session.SessionMode.TimeoutMilliseconds) * time.Millisecond | |
| 449 | + | timeout := time.Duration(cfg.Session.SessionMode.TimeoutMilliseconds) * time.Millisecond | |
| 406 | 450 | h.modal.EnterSessionMode(h.ctx, timeout) | |
| 407 | 451 | return true | |
| 408 | 452 | case ActionEnterResizeMode: | |
| @@ -410,7 +454,7 @@ func (h *KeyboardHandler) handleModeAction(action Action) bool { | |||
| 410 | 454 | h.modal.ExitMode(h.ctx) | |
| 411 | 455 | return true | |
| 412 | 456 | } | |
| 413 | - | timeout := time.Duration(h.cfg.Workspace.ResizeMode.TimeoutMilliseconds) * time.Millisecond | |
| 457 | + | timeout := time.Duration(cfg.Workspace.ResizeMode.TimeoutMilliseconds) * time.Millisecond | |
| 414 | 458 | h.modal.EnterResizeMode(h.ctx, timeout) | |
| 415 | 459 | return true | |
| 416 | 460 | case ActionExitMode: | |
| @@ -424,21 +468,30 @@ func (h *KeyboardHandler) handleModeAction(action Action) bool { | |||
| 424 | 468 | // EnterTabMode programmatically enters tab mode. | |
| 425 | 469 | // Useful for testing or programmatic mode changes. | |
| 426 | 470 | func (h *KeyboardHandler) EnterTabMode() { | |
| 427 | - | timeout := time.Duration(h.cfg.Workspace.TabMode.TimeoutMilliseconds) * time.Millisecond | |
| 471 | + | h.mu.RLock() | |
| 472 | + | cfg := h.cfg | |
| 473 | + | h.mu.RUnlock() | |
| 474 | + | timeout := time.Duration(cfg.Workspace.TabMode.TimeoutMilliseconds) * time.Millisecond | |
| 428 | 475 | h.modal.EnterTabMode(h.ctx, timeout) | |
| 429 | 476 | } | |
| 430 | 477 | ||
| 431 | 478 | // EnterPaneMode programmatically enters pane mode. | |
| 432 | 479 | // Useful for testing or programmatic mode changes. | |
| 433 | 480 | func (h *KeyboardHandler) EnterPaneMode() { | |
| 434 | - | timeout := time.Duration(h.cfg.Workspace.PaneMode.TimeoutMilliseconds) * time.Millisecond | |
| 481 | + | h.mu.RLock() | |
| 482 | + | cfg := h.cfg | |
| 483 | + | h.mu.RUnlock() | |
| 484 | + | timeout := time.Duration(cfg.Workspace.PaneMode.TimeoutMilliseconds) * time.Millisecond | |
| 435 | 485 | h.modal.EnterPaneMode(h.ctx, timeout) | |
| 436 | 486 | } | |
| 437 | 487 | ||
| 438 | 488 | // EnterSessionMode programmatically enters session mode. | |
| 439 | 489 | // Useful for testing or programmatic mode changes. | |
| 440 | 490 | func (h *KeyboardHandler) EnterSessionMode() { | |
| 441 | - | timeout := time.Duration(h.cfg.Session.SessionMode.TimeoutMilliseconds) * time.Millisecond | |
| 491 | + | h.mu.RLock() | |
| 492 | + | cfg := h.cfg | |
| 493 | + | h.mu.RUnlock() | |
| 494 | + | timeout := time.Duration(cfg.Session.SessionMode.TimeoutMilliseconds) * time.Millisecond | |
| 442 | 495 | h.modal.EnterSessionMode(h.ctx, timeout) | |
| 443 | 496 | } | |
| 444 | 497 | ||
| @@ -448,6 +501,14 @@ func (h *KeyboardHandler) ExitMode() { | |||
| 448 | 501 | h.modal.ExitMode(h.ctx) | |
| 449 | 502 | } | |
| 450 | 503 | ||
| 504 | + | // DispatchAction processes an action externally triggered (e.g., by a global | |
| 505 | + | // shortcut). Mode-enter actions update modal state; other actions are forwarded | |
| 506 | + | // to the registered action handler. | |
| 507 | + | func (h *KeyboardHandler) DispatchAction(action Action) { | |
| 508 | + | mode := h.modal.Mode() | |
| 509 | + | h.dispatchAction(action, mode) | |
| 510 | + | } | |
| 511 | + | ||
| 451 | 512 | // handleKeyRelease processes a key release event for accent detection. | |
| 452 | 513 | func (h *KeyboardHandler) handleKeyRelease(keyval uint) { | |
| 453 | 514 | h.mu.RLock() | |
| @@ -26,6 +26,7 @@ import ( | |||||
| 26 | "github.com/bnema/dumber/internal/infrastructure/config" | 26 | "github.com/bnema/dumber/internal/infrastructure/config" | ||
| 27 | "github.com/bnema/dumber/internal/logging" | 27 | "github.com/bnema/dumber/internal/logging" | ||
| 28 | "github.com/jwijenbergh/puregotk/v4/gdk" | 28 | "github.com/jwijenbergh/puregotk/v4/gdk" | ||
| 29 | + | "github.com/jwijenbergh/puregotk/v4/glib" | |||
| 29 | "github.com/jwijenbergh/puregotk/v4/gtk" | 30 | "github.com/jwijenbergh/puregotk/v4/gtk" | ||
| 30 | "github.com/rs/zerolog" | 31 | "github.com/rs/zerolog" | ||
| 31 | ) | 32 | ) | ||
| @@ -116,7 +117,35 @@ func (h *KeyboardHandler) ReloadShortcuts(ctx context.Context, cfg *config.Confi | |||||
| 116 | 117 | ||||
| 117 | // SetOnModeChange sets the callback for mode changes (for UI updates). | 118 | // SetOnModeChange sets the callback for mode changes (for UI updates). | ||
| 118 | func (h *KeyboardHandler) SetOnModeChange(fn func(from, to Mode)) { | 119 | func (h *KeyboardHandler) SetOnModeChange(fn func(from, to Mode)) { | ||
| 119 | - | h.modal.SetOnModeChange(fn) | 120 | + | h.modal.SetOnModeChange(func(from, to Mode) { |
| 121 | + | // Switch controller phase: capture during modal, bubble for normal. | |||
| 122 | + | // This ensures plain modal keys (s, h, etc.) reach the handler | |||
| 123 | + | // before WebView, while normal typing still goes through WebKit IM. | |||
| 124 | + | if to == ModeNormal { | |||
| 125 | + | h.setControllerPhase(gtk.PhaseBubbleValue) | |||
| 126 | + | } else if from == ModeNormal { | |||
| 127 | + | h.setControllerPhase(gtk.PhaseCaptureValue) | |||
| 128 | + | } | |||
| 129 | + | // Forward to app-level callback | |||
| 130 | + | if fn != nil { | |||
| 131 | + | fn(from, to) | |||
| 132 | + | } | |||
| 133 | + | }) | |||
| 134 | + | } | |||
| 135 | + | ||||
| 136 | + | // setControllerPhase changes the propagation phase of the key controller. | |||
| 137 | + | // Capture phase during modal modes so plain keys reach the handler before WebView. | |||
| 138 | + | // Bubble phase in normal mode for WebKit IM/dead-key support. | |||
| 139 | + | // | |||
| 140 | + | // This is called synchronously from onModeChange so the phase switch takes | |||
| 141 | + | // effect before the next key event. The caller must ensure GTK thread safety; | |||
| 142 | + | // modal timeouts dispatch to the GTK main thread via glib.IdleAdd before | |||
| 143 | + | // calling ExitMode, so the onModeChange callback always runs on the GTK thread. | |||
| 144 | + | func (h *KeyboardHandler) setControllerPhase(phase gtk.PropagationPhase) { | |||
| 145 | + | if h.controller == nil { | |||
| 146 | + | return | |||
| 147 | + | } | |||
| 148 | + | h.controller.SetPropagationPhase(phase) | |||
| 120 | } | 149 | } | ||
| 121 | 150 | ||||
| 122 | // SetRouteKey sets the callback that determines how each key event should be routed. | 151 | // SetRouteKey sets the callback that determines how each key event should be routed. | ||
| @@ -175,6 +204,17 @@ func (h *KeyboardHandler) AttachTo(window *gtk.ApplicationWindow) { | |||||
| 175 | // so app shortcuts still work. | 204 | // so app shortcuts still work. | ||
| 176 | h.controller.SetPropagationPhase(gtk.PhaseBubbleValue) | 205 | h.controller.SetPropagationPhase(gtk.PhaseBubbleValue) | ||
| 177 | 206 | ||||
| 207 | + | // Wire GTK main thread scheduler for modal timeouts. Timer goroutines | |||
| 208 | + | // must dispatch ExitMode to the GTK thread because onModeChange may | |||
| 209 | + | // call setControllerPhase (a GTK operation). | |||
| 210 | + | h.modal.SetMainThreadScheduler(func(fn func()) { | |||
| 211 | + | cb := glib.SourceFunc(func(_ uintptr) bool { | |||
| 212 | + | fn() | |||
| 213 | + | return false | |||
| 214 | + | }) | |||
| 215 | + | glib.IdleAdd(&cb, 0) | |||
| 216 | + | }) | |||
| 217 | + | ||||
| 178 | // Connect key pressed handler (retain callback to prevent GC). | 218 | // Connect key pressed handler (retain callback to prevent GC). | ||
| 179 | // The callback receives: keyval (translated key), keycode (hardware key position), state (modifiers) | 219 | // The callback receives: keyval (translated key), keycode (hardware key position), state (modifiers) | ||
| 180 | h.keyPressedCb = func(_ gtk.EventControllerKey, keyval uint, keycode uint, state gdk.ModifierType) bool { | 220 | h.keyPressedCb = func(_ gtk.EventControllerKey, keyval uint, keycode uint, state gdk.ModifierType) bool { | ||
| @@ -392,17 +432,21 @@ func isResizeAction(action Action) bool { | |||||
| 392 | } | 432 | } | ||
| 393 | 433 | ||||
| 394 | func (h *KeyboardHandler) handleModeAction(action Action) bool { | 434 | func (h *KeyboardHandler) handleModeAction(action Action) bool { | ||
| 435 | + | h.mu.RLock() | |||
| 436 | + | cfg := h.cfg | |||
| 437 | + | h.mu.RUnlock() | |||
| 438 | + | ||||
| 395 | switch action { | 439 | switch action { | ||
| 396 | case ActionEnterTabMode: | 440 | case ActionEnterTabMode: | ||
| 397 | - | timeout := time.Duration(h.cfg.Workspace.TabMode.TimeoutMilliseconds) * time.Millisecond | 441 | + | timeout := time.Duration(cfg.Workspace.TabMode.TimeoutMilliseconds) * time.Millisecond |
| 398 | h.modal.EnterTabMode(h.ctx, timeout) | 442 | h.modal.EnterTabMode(h.ctx, timeout) | ||
| 399 | return true | 443 | return true | ||
| 400 | case ActionEnterPaneMode: | 444 | case ActionEnterPaneMode: | ||
| 401 | - | timeout := time.Duration(h.cfg.Workspace.PaneMode.TimeoutMilliseconds) * time.Millisecond | 445 | + | timeout := time.Duration(cfg.Workspace.PaneMode.TimeoutMilliseconds) * time.Millisecond |
| 402 | h.modal.EnterPaneMode(h.ctx, timeout) | 446 | h.modal.EnterPaneMode(h.ctx, timeout) | ||
| 403 | return true | 447 | return true | ||
| 404 | case ActionEnterSessionMode: | 448 | case ActionEnterSessionMode: | ||
| 405 | - | timeout := time.Duration(h.cfg.Session.SessionMode.TimeoutMilliseconds) * time.Millisecond | 449 | + | timeout := time.Duration(cfg.Session.SessionMode.TimeoutMilliseconds) * time.Millisecond |
| 406 | h.modal.EnterSessionMode(h.ctx, timeout) | 450 | h.modal.EnterSessionMode(h.ctx, timeout) | ||
| 407 | return true | 451 | return true | ||
| 408 | case ActionEnterResizeMode: | 452 | case ActionEnterResizeMode: | ||
| @@ -410,7 +454,7 @@ func (h *KeyboardHandler) handleModeAction(action Action) bool { | |||||
| 410 | h.modal.ExitMode(h.ctx) | 454 | h.modal.ExitMode(h.ctx) | ||
| 411 | return true | 455 | return true | ||
| 412 | } | 456 | } | ||
| 413 | - | timeout := time.Duration(h.cfg.Workspace.ResizeMode.TimeoutMilliseconds) * time.Millisecond | 457 | + | timeout := time.Duration(cfg.Workspace.ResizeMode.TimeoutMilliseconds) * time.Millisecond |
| 414 | h.modal.EnterResizeMode(h.ctx, timeout) | 458 | h.modal.EnterResizeMode(h.ctx, timeout) | ||
| 415 | return true | 459 | return true | ||
| 416 | case ActionExitMode: | 460 | case ActionExitMode: | ||
| @@ -424,21 +468,30 @@ func (h *KeyboardHandler) handleModeAction(action Action) bool { | |||||
| 424 | // EnterTabMode programmatically enters tab mode. | 468 | // EnterTabMode programmatically enters tab mode. | ||
| 425 | // Useful for testing or programmatic mode changes. | 469 | // Useful for testing or programmatic mode changes. | ||
| 426 | func (h *KeyboardHandler) EnterTabMode() { | 470 | func (h *KeyboardHandler) EnterTabMode() { | ||
| 427 | - | timeout := time.Duration(h.cfg.Workspace.TabMode.TimeoutMilliseconds) * time.Millisecond | 471 | + | h.mu.RLock() |
| 472 | + | cfg := h.cfg | |||
| 473 | + | h.mu.RUnlock() | |||
| 474 | + | timeout := time.Duration(cfg.Workspace.TabMode.TimeoutMilliseconds) * time.Millisecond | |||
| 428 | h.modal.EnterTabMode(h.ctx, timeout) | 475 | h.modal.EnterTabMode(h.ctx, timeout) | ||
| 429 | } | 476 | } | ||
| 430 | 477 | ||||
| 431 | // EnterPaneMode programmatically enters pane mode. | 478 | // EnterPaneMode programmatically enters pane mode. | ||
| 432 | // Useful for testing or programmatic mode changes. | 479 | // Useful for testing or programmatic mode changes. | ||
| 433 | func (h *KeyboardHandler) EnterPaneMode() { | 480 | func (h *KeyboardHandler) EnterPaneMode() { | ||
| 434 | - | timeout := time.Duration(h.cfg.Workspace.PaneMode.TimeoutMilliseconds) * time.Millisecond | 481 | + | h.mu.RLock() |
| 482 | + | cfg := h.cfg | |||
| 483 | + | h.mu.RUnlock() | |||
| 484 | + | timeout := time.Duration(cfg.Workspace.PaneMode.TimeoutMilliseconds) * time.Millisecond | |||
| 435 | h.modal.EnterPaneMode(h.ctx, timeout) | 485 | h.modal.EnterPaneMode(h.ctx, timeout) | ||
| 436 | } | 486 | } | ||
| 437 | 487 | ||||
| 438 | // EnterSessionMode programmatically enters session mode. | 488 | // EnterSessionMode programmatically enters session mode. | ||
| 439 | // Useful for testing or programmatic mode changes. | 489 | // Useful for testing or programmatic mode changes. | ||
| 440 | func (h *KeyboardHandler) EnterSessionMode() { | 490 | func (h *KeyboardHandler) EnterSessionMode() { | ||
| 441 | - | timeout := time.Duration(h.cfg.Session.SessionMode.TimeoutMilliseconds) * time.Millisecond | 491 | + | h.mu.RLock() |
| 492 | + | cfg := h.cfg | |||
| 493 | + | h.mu.RUnlock() | |||
| 494 | + | timeout := time.Duration(cfg.Session.SessionMode.TimeoutMilliseconds) * time.Millisecond | |||
| 442 | h.modal.EnterSessionMode(h.ctx, timeout) | 495 | h.modal.EnterSessionMode(h.ctx, timeout) | ||
| 443 | } | 496 | } | ||
| 444 | 497 | ||||
| @@ -448,6 +501,14 @@ func (h *KeyboardHandler) ExitMode() { | |||||
| 448 | h.modal.ExitMode(h.ctx) | 501 | h.modal.ExitMode(h.ctx) | ||
| 449 | } | 502 | } | ||
| 450 | 503 | ||||
| 504 | + | // DispatchAction processes an action externally triggered (e.g., by a global | |||
| 505 | + | // shortcut). Mode-enter actions update modal state; other actions are forwarded | |||
| 506 | + | // to the registered action handler. | |||
| 507 | + | func (h *KeyboardHandler) DispatchAction(action Action) { | |||
| 508 | + | mode := h.modal.Mode() | |||
| 509 | + | h.dispatchAction(action, mode) | |||
| 510 | + | } | |||
| 511 | + | ||||
| 451 | // handleKeyRelease processes a key release event for accent detection. | 512 | // handleKeyRelease processes a key release event for accent detection. | ||
| 452 | func (h *KeyboardHandler) handleKeyRelease(keyval uint) { | 513 | func (h *KeyboardHandler) handleKeyRelease(keyval uint) { | ||
| 453 | h.mu.RLock() | 514 | h.mu.RLock() | ||
M internal/ui/input/modal.go +65 -110
| @@ -60,13 +60,21 @@ func (m Mode) DisplayName() string { | |||
| 60 | 60 | ||
| 61 | 61 | // ModalState manages the current input mode with optional timeout. | |
| 62 | 62 | type ModalState struct { | |
| 63 | - | mode Mode | |
| 64 | - | timeout time.Duration | |
| 65 | - | timer *time.Timer | |
| 63 | + | mode Mode | |
| 64 | + | timeout time.Duration | |
| 65 | + | timer *time.Timer | |
| 66 | + | timerGen int64 // incremented each time a timer is started; used to ignore stale callbacks | |
| 66 | 67 | ||
| 67 | 68 | // Callback for mode changes (called synchronously under lock). | |
| 68 | 69 | onModeChange func(from, to Mode) | |
| 69 | 70 | ||
| 71 | + | // scheduleOnMainThread dispatches a function to the GTK main thread. | |
| 72 | + | // Timer callbacks use this so that onModeChange (which may make GTK | |
| 73 | + | // calls like switching controller phase) always runs on the GTK thread. | |
| 74 | + | // Defaults to direct execution (suitable for tests without a GTK loop). | |
| 75 | + | // The app sets this to a glib.IdleAdd wrapper via SetMainThreadScheduler. | |
| 76 | + | scheduleOnMainThread func(fn func()) | |
| 77 | + | ||
| 70 | 78 | mu sync.RWMutex | |
| 71 | 79 | } | |
| 72 | 80 | ||
| @@ -75,10 +83,23 @@ func NewModalState(ctx context.Context) *ModalState { | |||
| 75 | 83 | log := logging.FromContext(ctx) | |
| 76 | 84 | log.Debug().Msg("creating modal state") | |
| 77 | 85 | return &ModalState{ | |
| 78 | - | mode: ModeNormal, | |
| 86 | + | mode: ModeNormal, | |
| 87 | + | scheduleOnMainThread: func(fn func()) { fn() }, | |
| 79 | 88 | } | |
| 80 | 89 | } | |
| 81 | 90 | ||
| 91 | + | // SetMainThreadScheduler sets the function used to dispatch timer callbacks | |
| 92 | + | // to the GTK main thread. In the real app this should be a glib.IdleAdd | |
| 93 | + | // wrapper. In tests, the default (direct execution) is used. | |
| 94 | + | func (m *ModalState) SetMainThreadScheduler(fn func(func())) { | |
| 95 | + | m.mu.Lock() | |
| 96 | + | defer m.mu.Unlock() | |
| 97 | + | if fn == nil { | |
| 98 | + | return | |
| 99 | + | } | |
| 100 | + | m.scheduleOnMainThread = fn | |
| 101 | + | } | |
| 102 | + | ||
| 82 | 103 | // Mode returns the current mode (thread-safe). | |
| 83 | 104 | func (m *ModalState) Mode() Mode { | |
| 84 | 105 | m.mu.RLock() | |
| @@ -87,142 +108,54 @@ func (m *ModalState) Mode() Mode { | |||
| 87 | 108 | } | |
| 88 | 109 | ||
| 89 | 110 | // EnterTabMode switches to tab mode with an optional timeout. | |
| 90 | - | // If timeout is 0, the mode stays until explicitly exited. | |
| 91 | 111 | func (m *ModalState) EnterTabMode(ctx context.Context, timeout time.Duration) { | |
| 92 | - | log := logging.FromContext(ctx) | |
| 93 | - | m.mu.Lock() | |
| 94 | - | defer m.mu.Unlock() | |
| 95 | - | ||
| 96 | - | if m.mode == ModeTab { | |
| 97 | - | // Already in tab mode, just reset timeout | |
| 98 | - | m.resetTimeoutLocked(ctx, timeout) | |
| 99 | - | return | |
| 100 | - | } | |
| 101 | - | ||
| 102 | - | m.cancelTimerLocked() | |
| 103 | - | oldMode := m.mode | |
| 104 | - | m.mode = ModeTab | |
| 105 | - | m.timeout = timeout | |
| 106 | - | ||
| 107 | - | if timeout > 0 { | |
| 108 | - | m.timer = time.AfterFunc(timeout, func() { | |
| 109 | - | m.ExitMode(ctx) | |
| 110 | - | }) | |
| 111 | - | } | |
| 112 | - | ||
| 113 | - | log.Debug(). | |
| 114 | - | Str("from", oldMode.String()). | |
| 115 | - | Str("to", m.mode.String()). | |
| 116 | - | Dur("timeout", timeout). | |
| 117 | - | Msg("entered tab mode") | |
| 118 | - | ||
| 119 | - | if m.onModeChange != nil { | |
| 120 | - | m.onModeChange(oldMode, m.mode) | |
| 121 | - | } | |
| 112 | + | m.enterMode(ctx, ModeTab, timeout) | |
| 122 | 113 | } | |
| 123 | 114 | ||
| 124 | 115 | // EnterPaneMode switches to pane mode with an optional timeout. | |
| 125 | - | // If timeout is 0, the mode stays until explicitly exited. | |
| 126 | 116 | func (m *ModalState) EnterPaneMode(ctx context.Context, timeout time.Duration) { | |
| 127 | - | log := logging.FromContext(ctx) | |
| 128 | - | m.mu.Lock() | |
| 129 | - | defer m.mu.Unlock() | |
| 130 | - | ||
| 131 | - | if m.mode == ModePane { | |
| 132 | - | // Already in pane mode, just reset timeout | |
| 133 | - | m.resetTimeoutLocked(ctx, timeout) | |
| 134 | - | return | |
| 135 | - | } | |
| 136 | - | ||
| 137 | - | m.cancelTimerLocked() | |
| 138 | - | oldMode := m.mode | |
| 139 | - | m.mode = ModePane | |
| 140 | - | m.timeout = timeout | |
| 141 | - | ||
| 142 | - | if timeout > 0 { | |
| 143 | - | m.timer = time.AfterFunc(timeout, func() { | |
| 144 | - | m.ExitMode(ctx) | |
| 145 | - | }) | |
| 146 | - | } | |
| 147 | - | ||
| 148 | - | log.Debug(). | |
| 149 | - | Str("from", oldMode.String()). | |
| 150 | - | Str("to", m.mode.String()). | |
| 151 | - | Dur("timeout", timeout). | |
| 152 | - | Msg("entered pane mode") | |
| 153 | - | ||
| 154 | - | if m.onModeChange != nil { | |
| 155 | - | m.onModeChange(oldMode, m.mode) | |
| 156 | - | } | |
| 117 | + | m.enterMode(ctx, ModePane, timeout) | |
| 157 | 118 | } | |
| 158 | 119 | ||
| 159 | 120 | // EnterSessionMode switches to session mode with an optional timeout. | |
| 160 | - | // If timeout is 0, the mode stays until explicitly exited. | |
| 161 | 121 | func (m *ModalState) EnterSessionMode(ctx context.Context, timeout time.Duration) { | |
| 162 | - | log := logging.FromContext(ctx) | |
| 163 | - | m.mu.Lock() | |
| 164 | - | defer m.mu.Unlock() | |
| 165 | - | ||
| 166 | - | if m.mode == ModeSession { | |
| 167 | - | // Already in session mode, just reset timeout | |
| 168 | - | m.resetTimeoutLocked(ctx, timeout) | |
| 169 | - | return | |
| 170 | - | } | |
| 171 | - | ||
| 172 | - | m.cancelTimerLocked() | |
| 173 | - | oldMode := m.mode | |
| 174 | - | m.mode = ModeSession | |
| 175 | - | m.timeout = timeout | |
| 176 | - | ||
| 177 | - | if timeout > 0 { | |
| 178 | - | m.timer = time.AfterFunc(timeout, func() { | |
| 179 | - | m.ExitMode(ctx) | |
| 180 | - | }) | |
| 181 | - | } | |
| 182 | - | ||
| 183 | - | log.Debug(). | |
| 184 | - | Str("from", oldMode.String()). | |
| 185 | - | Str("to", m.mode.String()). | |
| 186 | - | Dur("timeout", timeout). | |
| 187 | - | Msg("entered session mode") | |
| 188 | - | ||
| 189 | - | if m.onModeChange != nil { | |
| 190 | - | m.onModeChange(oldMode, m.mode) | |
| 191 | - | } | |
| 122 | + | m.enterMode(ctx, ModeSession, timeout) | |
| 192 | 123 | } | |
| 193 | 124 | ||
| 194 | 125 | // EnterResizeMode switches to resize mode with an optional timeout. | |
| 195 | - | // If timeout is 0, the mode stays until explicitly exited. | |
| 196 | 126 | func (m *ModalState) EnterResizeMode(ctx context.Context, timeout time.Duration) { | |
| 127 | + | m.enterMode(ctx, ModeResize, timeout) | |
| 128 | + | } | |
| 129 | + | ||
| 130 | + | // enterMode is the shared implementation for all mode-enter methods. | |
| 131 | + | // If already in the target mode, it resets the timeout instead. | |
| 132 | + | func (m *ModalState) enterMode(ctx context.Context, target Mode, timeout time.Duration) { | |
| 197 | 133 | log := logging.FromContext(ctx) | |
| 198 | 134 | m.mu.Lock() | |
| 199 | 135 | defer m.mu.Unlock() | |
| 200 | 136 | ||
| 201 | - | if m.mode == ModeResize { | |
| 202 | - | // Already in resize mode, just reset timeout | |
| 137 | + | if m.mode == target { | |
| 203 | 138 | m.resetTimeoutLocked(ctx, timeout) | |
| 204 | 139 | return | |
| 205 | 140 | } | |
| 206 | 141 | ||
| 207 | 142 | m.cancelTimerLocked() | |
| 208 | 143 | oldMode := m.mode | |
| 209 | - | m.mode = ModeResize | |
| 144 | + | m.mode = target | |
| 210 | 145 | m.timeout = timeout | |
| 211 | 146 | ||
| 212 | 147 | if timeout > 0 { | |
| 213 | - | m.timer = time.AfterFunc(timeout, func() { | |
| 214 | - | m.ExitMode(ctx) | |
| 215 | - | }) | |
| 148 | + | m.startTimeoutLocked(ctx, timeout) | |
| 216 | 149 | } | |
| 217 | 150 | ||
| 218 | 151 | log.Debug(). | |
| 219 | 152 | Str("from", oldMode.String()). | |
| 220 | - | Str("to", m.mode.String()). | |
| 153 | + | Str("to", target.String()). | |
| 221 | 154 | Dur("timeout", timeout). | |
| 222 | - | Msg("entered resize mode") | |
| 155 | + | Msg("entered " + target.String() + " mode") | |
| 223 | 156 | ||
| 224 | 157 | if m.onModeChange != nil { | |
| 225 | - | m.onModeChange(oldMode, m.mode) | |
| 158 | + | m.onModeChange(oldMode, target) | |
| 226 | 159 | } | |
| 227 | 160 | } | |
| 228 | 161 | ||
| @@ -278,17 +211,39 @@ func (m *ModalState) cancelTimerLocked() { | |||
| 278 | 211 | if m.timer != nil { | |
| 279 | 212 | m.timer.Stop() | |
| 280 | 213 | m.timer = nil | |
| 214 | + | m.timerGen++ | |
| 281 | 215 | } | |
| 282 | 216 | } | |
| 283 | 217 | ||
| 218 | + | // startTimeoutLocked starts a new timeout timer that exits the mode via the | |
| 219 | + | // main thread scheduler. Timer goroutines must not call ExitMode directly | |
| 220 | + | // because onModeChange may make GTK calls (e.g., switching controller phase). | |
| 221 | + | // Must be called with m.mu held. | |
| 222 | + | func (m *ModalState) startTimeoutLocked(ctx context.Context, timeout time.Duration) { | |
| 223 | + | m.timerGen++ | |
| 224 | + | gen := m.timerGen | |
| 225 | + | schedule := m.scheduleOnMainThread | |
| 226 | + | m.timer = time.AfterFunc(timeout, func() { | |
| 227 | + | schedule(func() { | |
| 228 | + | // Ignore stale callback: time.Timer.Stop can race with the | |
| 229 | + | // timer firing, so a canceled timer's callback may still run. | |
| 230 | + | m.mu.RLock() | |
| 231 | + | stale := m.timerGen != gen | |
| 232 | + | m.mu.RUnlock() | |
| 233 | + | if stale { | |
| 234 | + | return | |
| 235 | + | } | |
| 236 | + | m.ExitMode(ctx) | |
| 237 | + | }) | |
| 238 | + | }) | |
| 239 | + | } | |
| 240 | + | ||
| 284 | 241 | // resetTimeoutLocked resets the timeout timer. | |
| 285 | 242 | // Must be called with m.mu held. | |
| 286 | 243 | func (m *ModalState) resetTimeoutLocked(ctx context.Context, timeout time.Duration) { | |
| 287 | 244 | m.cancelTimerLocked() | |
| 288 | 245 | m.timeout = timeout | |
| 289 | 246 | if timeout > 0 { | |
| 290 | - | m.timer = time.AfterFunc(timeout, func() { | |
| 291 | - | m.ExitMode(ctx) | |
| 292 | - | }) | |
| 247 | + | m.startTimeoutLocked(ctx, timeout) | |
| 293 | 248 | } | |
| 294 | 249 | } | |
| @@ -60,13 +60,21 @@ func (m Mode) DisplayName() string { | |||||
| 60 | 60 | ||||
| 61 | // ModalState manages the current input mode with optional timeout. | 61 | // ModalState manages the current input mode with optional timeout. | ||
| 62 | type ModalState struct { | 62 | type ModalState struct { | ||
| 63 | - | mode Mode | 63 | + | mode Mode |
| 64 | - | timeout time.Duration | 64 | + | timeout time.Duration |
| 65 | - | timer *time.Timer | 65 | + | timer *time.Timer |
| 66 | + | timerGen int64 // incremented each time a timer is started; used to ignore stale callbacks | |||
| 66 | 67 | ||||
| 67 | // Callback for mode changes (called synchronously under lock). | 68 | // Callback for mode changes (called synchronously under lock). | ||
| 68 | onModeChange func(from, to Mode) | 69 | onModeChange func(from, to Mode) | ||
| 69 | 70 | ||||
| 71 | + | // scheduleOnMainThread dispatches a function to the GTK main thread. | |||
| 72 | + | // Timer callbacks use this so that onModeChange (which may make GTK | |||
| 73 | + | // calls like switching controller phase) always runs on the GTK thread. | |||
| 74 | + | // Defaults to direct execution (suitable for tests without a GTK loop). | |||
| 75 | + | // The app sets this to a glib.IdleAdd wrapper via SetMainThreadScheduler. | |||
| 76 | + | scheduleOnMainThread func(fn func()) | |||
| 77 | + | ||||
| 70 | mu sync.RWMutex | 78 | mu sync.RWMutex | ||
| 71 | } | 79 | } | ||
| 72 | 80 | ||||
| @@ -75,10 +83,23 @@ func NewModalState(ctx context.Context) *ModalState { | |||||
| 75 | log := logging.FromContext(ctx) | 83 | log := logging.FromContext(ctx) | ||
| 76 | log.Debug().Msg("creating modal state") | 84 | log.Debug().Msg("creating modal state") | ||
| 77 | return &ModalState{ | 85 | return &ModalState{ | ||
| 78 | - | mode: ModeNormal, | 86 | + | mode: ModeNormal, |
| 87 | + | scheduleOnMainThread: func(fn func()) { fn() }, | |||
| 79 | } | 88 | } | ||
| 80 | } | 89 | } | ||
| 81 | 90 | ||||
| 91 | + | // SetMainThreadScheduler sets the function used to dispatch timer callbacks | |||
| 92 | + | // to the GTK main thread. In the real app this should be a glib.IdleAdd | |||
| 93 | + | // wrapper. In tests, the default (direct execution) is used. | |||
| 94 | + | func (m *ModalState) SetMainThreadScheduler(fn func(func())) { | |||
| 95 | + | m.mu.Lock() | |||
| 96 | + | defer m.mu.Unlock() | |||
| 97 | + | if fn == nil { | |||
| 98 | + | return | |||
| 99 | + | } | |||
| 100 | + | m.scheduleOnMainThread = fn | |||
| 101 | + | } | |||
| 102 | + | ||||
| 82 | // Mode returns the current mode (thread-safe). | 103 | // Mode returns the current mode (thread-safe). | ||
| 83 | func (m *ModalState) Mode() Mode { | 104 | func (m *ModalState) Mode() Mode { | ||
| 84 | m.mu.RLock() | 105 | m.mu.RLock() | ||
| @@ -87,142 +108,54 @@ func (m *ModalState) Mode() Mode { | |||||
| 87 | } | 108 | } | ||
| 88 | 109 | ||||
| 89 | // EnterTabMode switches to tab mode with an optional timeout. | 110 | // EnterTabMode switches to tab mode with an optional timeout. | ||
| 90 | - | // If timeout is 0, the mode stays until explicitly exited. | |||
| 91 | func (m *ModalState) EnterTabMode(ctx context.Context, timeout time.Duration) { | 111 | func (m *ModalState) EnterTabMode(ctx context.Context, timeout time.Duration) { | ||
| 92 | - | log := logging.FromContext(ctx) | 112 | + | m.enterMode(ctx, ModeTab, timeout) |
| 93 | - | m.mu.Lock() | |||
| 94 | - | defer m.mu.Unlock() | |||
| 95 | - | ||||
| 96 | - | if m.mode == ModeTab { | |||
| 97 | - | // Already in tab mode, just reset timeout | |||
| 98 | - | m.resetTimeoutLocked(ctx, timeout) | |||
| 99 | - | return | |||
| 100 | - | } | |||
| 101 | - | ||||
| 102 | - | m.cancelTimerLocked() | |||
| 103 | - | oldMode := m.mode | |||
| 104 | - | m.mode = ModeTab | |||
| 105 | - | m.timeout = timeout | |||
| 106 | - | ||||
| 107 | - | if timeout > 0 { | |||
| 108 | - | m.timer = time.AfterFunc(timeout, func() { | |||
| 109 | - | m.ExitMode(ctx) | |||
| 110 | - | }) | |||
| 111 | - | } | |||
| 112 | - | ||||
| 113 | - | log.Debug(). | |||
| 114 | - | Str("from", oldMode.String()). | |||
| 115 | - | Str("to", m.mode.String()). | |||
| 116 | - | Dur("timeout", timeout). | |||
| 117 | - | Msg("entered tab mode") | |||
| 118 | - | ||||
| 119 | - | if m.onModeChange != nil { | |||
| 120 | - | m.onModeChange(oldMode, m.mode) | |||
| 121 | - | } | |||
| 122 | } | 113 | } | ||
| 123 | 114 | ||||
| 124 | // EnterPaneMode switches to pane mode with an optional timeout. | 115 | // EnterPaneMode switches to pane mode with an optional timeout. | ||
| 125 | - | // If timeout is 0, the mode stays until explicitly exited. | |||
| 126 | func (m *ModalState) EnterPaneMode(ctx context.Context, timeout time.Duration) { | 116 | func (m *ModalState) EnterPaneMode(ctx context.Context, timeout time.Duration) { | ||
| 127 | - | log := logging.FromContext(ctx) | 117 | + | m.enterMode(ctx, ModePane, timeout) |
| 128 | - | m.mu.Lock() | |||
| 129 | - | defer m.mu.Unlock() | |||
| 130 | - | ||||
| 131 | - | if m.mode == ModePane { | |||
| 132 | - | // Already in pane mode, just reset timeout | |||
| 133 | - | m.resetTimeoutLocked(ctx, timeout) | |||
| 134 | - | return | |||
| 135 | - | } | |||
| 136 | - | ||||
| 137 | - | m.cancelTimerLocked() | |||
| 138 | - | oldMode := m.mode | |||
| 139 | - | m.mode = ModePane | |||
| 140 | - | m.timeout = timeout | |||
| 141 | - | ||||
| 142 | - | if timeout > 0 { | |||
| 143 | - | m.timer = time.AfterFunc(timeout, func() { | |||
| 144 | - | m.ExitMode(ctx) | |||
| 145 | - | }) | |||
| 146 | - | } | |||
| 147 | - | ||||
| 148 | - | log.Debug(). | |||
| 149 | - | Str("from", oldMode.String()). | |||
| 150 | - | Str("to", m.mode.String()). | |||
| 151 | - | Dur("timeout", timeout). | |||
| 152 | - | Msg("entered pane mode") | |||
| 153 | - | ||||
| 154 | - | if m.onModeChange != nil { | |||
| 155 | - | m.onModeChange(oldMode, m.mode) | |||
| 156 | - | } | |||
| 157 | } | 118 | } | ||
| 158 | 119 | ||||
| 159 | // EnterSessionMode switches to session mode with an optional timeout. | 120 | // EnterSessionMode switches to session mode with an optional timeout. | ||
| 160 | - | // If timeout is 0, the mode stays until explicitly exited. | |||
| 161 | func (m *ModalState) EnterSessionMode(ctx context.Context, timeout time.Duration) { | 121 | func (m *ModalState) EnterSessionMode(ctx context.Context, timeout time.Duration) { | ||
| 162 | - | log := logging.FromContext(ctx) | 122 | + | m.enterMode(ctx, ModeSession, timeout) |
| 163 | - | m.mu.Lock() | |||
| 164 | - | defer m.mu.Unlock() | |||
| 165 | - | ||||
| 166 | - | if m.mode == ModeSession { | |||
| 167 | - | // Already in session mode, just reset timeout | |||
| 168 | - | m.resetTimeoutLocked(ctx, timeout) | |||
| 169 | - | return | |||
| 170 | - | } | |||
| 171 | - | ||||
| 172 | - | m.cancelTimerLocked() | |||
| 173 | - | oldMode := m.mode | |||
| 174 | - | m.mode = ModeSession | |||
| 175 | - | m.timeout = timeout | |||
| 176 | - | ||||
| 177 | - | if timeout > 0 { | |||
| 178 | - | m.timer = time.AfterFunc(timeout, func() { | |||
| 179 | - | m.ExitMode(ctx) | |||
| 180 | - | }) | |||
| 181 | - | } | |||
| 182 | - | ||||
| 183 | - | log.Debug(). | |||
| 184 | - | Str("from", oldMode.String()). | |||
| 185 | - | Str("to", m.mode.String()). | |||
| 186 | - | Dur("timeout", timeout). | |||
| 187 | - | Msg("entered session mode") | |||
| 188 | - | ||||
| 189 | - | if m.onModeChange != nil { | |||
| 190 | - | m.onModeChange(oldMode, m.mode) | |||
| 191 | - | } | |||
| 192 | } | 123 | } | ||
| 193 | 124 | ||||
| 194 | // EnterResizeMode switches to resize mode with an optional timeout. | 125 | // EnterResizeMode switches to resize mode with an optional timeout. | ||
| 195 | - | // If timeout is 0, the mode stays until explicitly exited. | |||
| 196 | func (m *ModalState) EnterResizeMode(ctx context.Context, timeout time.Duration) { | 126 | func (m *ModalState) EnterResizeMode(ctx context.Context, timeout time.Duration) { | ||
| 127 | + | m.enterMode(ctx, ModeResize, timeout) | |||
| 128 | + | } | |||
| 129 | + | ||||
| 130 | + | // enterMode is the shared implementation for all mode-enter methods. | |||
| 131 | + | // If already in the target mode, it resets the timeout instead. | |||
| 132 | + | func (m *ModalState) enterMode(ctx context.Context, target Mode, timeout time.Duration) { | |||
| 197 | log := logging.FromContext(ctx) | 133 | log := logging.FromContext(ctx) | ||
| 198 | m.mu.Lock() | 134 | m.mu.Lock() | ||
| 199 | defer m.mu.Unlock() | 135 | defer m.mu.Unlock() | ||
| 200 | 136 | ||||
| 201 | - | if m.mode == ModeResize { | 137 | + | if m.mode == target { |
| 202 | - | // Already in resize mode, just reset timeout | |||
| 203 | m.resetTimeoutLocked(ctx, timeout) | 138 | m.resetTimeoutLocked(ctx, timeout) | ||
| 204 | return | 139 | return | ||
| 205 | } | 140 | } | ||
| 206 | 141 | ||||
| 207 | m.cancelTimerLocked() | 142 | m.cancelTimerLocked() | ||
| 208 | oldMode := m.mode | 143 | oldMode := m.mode | ||
| 209 | - | m.mode = ModeResize | 144 | + | m.mode = target |
| 210 | m.timeout = timeout | 145 | m.timeout = timeout | ||
| 211 | 146 | ||||
| 212 | if timeout > 0 { | 147 | if timeout > 0 { | ||
| 213 | - | m.timer = time.AfterFunc(timeout, func() { | 148 | + | m.startTimeoutLocked(ctx, timeout) |
| 214 | - | m.ExitMode(ctx) | |||
| 215 | - | }) | |||
| 216 | } | 149 | } | ||
| 217 | 150 | ||||
| 218 | log.Debug(). | 151 | log.Debug(). | ||
| 219 | Str("from", oldMode.String()). | 152 | Str("from", oldMode.String()). | ||
| 220 | - | Str("to", m.mode.String()). | 153 | + | Str("to", target.String()). |
| 221 | Dur("timeout", timeout). | 154 | Dur("timeout", timeout). | ||
| 222 | - | Msg("entered resize mode") | 155 | + | Msg("entered " + target.String() + " mode") |
| 223 | 156 | ||||
| 224 | if m.onModeChange != nil { | 157 | if m.onModeChange != nil { | ||
| 225 | - | m.onModeChange(oldMode, m.mode) | 158 | + | m.onModeChange(oldMode, target) |
| 226 | } | 159 | } | ||
| 227 | } | 160 | } | ||
| 228 | 161 | ||||
| @@ -278,17 +211,39 @@ func (m *ModalState) cancelTimerLocked() { | |||||
| 278 | if m.timer != nil { | 211 | if m.timer != nil { | ||
| 279 | m.timer.Stop() | 212 | m.timer.Stop() | ||
| 280 | m.timer = nil | 213 | m.timer = nil | ||
| 214 | + | m.timerGen++ | |||
| 281 | } | 215 | } | ||
| 282 | } | 216 | } | ||
| 283 | 217 | ||||
| 218 | + | // startTimeoutLocked starts a new timeout timer that exits the mode via the | |||
| 219 | + | // main thread scheduler. Timer goroutines must not call ExitMode directly | |||
| 220 | + | // because onModeChange may make GTK calls (e.g., switching controller phase). | |||
| 221 | + | // Must be called with m.mu held. | |||
| 222 | + | func (m *ModalState) startTimeoutLocked(ctx context.Context, timeout time.Duration) { | |||
| 223 | + | m.timerGen++ | |||
| 224 | + | gen := m.timerGen | |||
| 225 | + | schedule := m.scheduleOnMainThread | |||
| 226 | + | m.timer = time.AfterFunc(timeout, func() { | |||
| 227 | + | schedule(func() { | |||
| 228 | + | // Ignore stale callback: time.Timer.Stop can race with the | |||
| 229 | + | // timer firing, so a canceled timer's callback may still run. | |||
| 230 | + | m.mu.RLock() | |||
| 231 | + | stale := m.timerGen != gen | |||
| 232 | + | m.mu.RUnlock() | |||
| 233 | + | if stale { | |||
| 234 | + | return | |||
| 235 | + | } | |||
| 236 | + | m.ExitMode(ctx) | |||
| 237 | + | }) | |||
| 238 | + | }) | |||
| 239 | + | } | |||
| 240 | + | ||||
| 284 | // resetTimeoutLocked resets the timeout timer. | 241 | // resetTimeoutLocked resets the timeout timer. | ||
| 285 | // Must be called with m.mu held. | 242 | // Must be called with m.mu held. | ||
| 286 | func (m *ModalState) resetTimeoutLocked(ctx context.Context, timeout time.Duration) { | 243 | func (m *ModalState) resetTimeoutLocked(ctx context.Context, timeout time.Duration) { | ||
| 287 | m.cancelTimerLocked() | 244 | m.cancelTimerLocked() | ||
| 288 | m.timeout = timeout | 245 | m.timeout = timeout | ||
| 289 | if timeout > 0 { | 246 | if timeout > 0 { | ||
| 290 | - | m.timer = time.AfterFunc(timeout, func() { | 247 | + | m.startTimeoutLocked(ctx, timeout) |
| 291 | - | m.ExitMode(ctx) | |||
| 292 | - | }) | |||
| 293 | } | 248 | } | ||
| 294 | } | 249 | } | ||