fix(input): prevent double accent detection and restrict JS events to editable targets
Fix bugs found by CodeRabbit review: - Split RoutePassToWidget from RouteAccentDetection so WebView pass-through skips window-level accent detection (JS bridge already handles it) - Route omnibox/find bar to PassToWidget instead of AccentDetection since widget-level capture controllers already call InsertAccentUseCase - Add isEditableTarget guard to JS accent script so non-editable elements don't arm the accent picker - Add stub AccentHandler to test that was exiting on nil instead of testing the actual no-accents path
- commit
- 0523844a47af18fd2bb51c3426350a4dccfa055c
- author
- brice · · 2026-03-11 06:04 UTC
- parent
- b05099c
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +42 -8
M internal/infrastructure/webkit/injector.go +15 -0
| @@ -150,7 +150,21 @@ const accentDetectionScript = `(function() { | |||
| 150 | 150 | // Track pressed key for release matching | |
| 151 | 151 | let pressedKey = null; | |
| 152 | 152 | ||
| 153 | + | // Check if the event target is an editable element (input, textarea, contenteditable) | |
| 154 | + | function isEditableTarget(target) { | |
| 155 | + | const el = target instanceof Element ? target : null; | |
| 156 | + | if (!el) return false; | |
| 157 | + | if (el.closest('[contenteditable=""], [contenteditable="true"]')) return true; | |
| 158 | + | if (el.closest('textarea')) return true; | |
| 159 | + | const input = el.closest('input'); | |
| 160 | + | 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); | |
| 163 | + | } | |
| 164 | + | ||
| 153 | 165 | document.addEventListener('keydown', function(e) { | |
| 166 | + | if (!isEditableTarget(e.target)) return; | |
| 167 | + | ||
| 154 | 168 | // Only handle a-z keys, with optional Shift | |
| 155 | 169 | // Skip if Ctrl or Alt are held (those are shortcuts) | |
| 156 | 170 | if (e.ctrlKey || e.altKey || e.metaKey) return; | |
| @@ -169,6 +183,7 @@ const accentDetectionScript = `(function() { | |||
| 169 | 183 | }, true); // capture phase to see events first | |
| 170 | 184 | ||
| 171 | 185 | document.addEventListener('keyup', function(e) { | |
| 186 | + | if (!isEditableTarget(e.target)) return; | |
| 172 | 187 | const key = e.key.toLowerCase(); | |
| 173 | 188 | if (key === pressedKey) { | |
| 174 | 189 | pressedKey = null; | |
| @@ -150,7 +150,21 @@ const accentDetectionScript = `(function() { | |||||
| 150 | // Track pressed key for release matching | 150 | // Track pressed key for release matching | ||
| 151 | let pressedKey = null; | 151 | let pressedKey = null; | ||
| 152 | 152 | ||||
| 153 | + | // Check if the event target is an editable element (input, textarea, contenteditable) | |||
| 154 | + | function isEditableTarget(target) { | |||
| 155 | + | const el = target instanceof Element ? target : null; | |||
| 156 | + | if (!el) return false; | |||
| 157 | + | if (el.closest('[contenteditable=""], [contenteditable="true"]')) return true; | |||
| 158 | + | if (el.closest('textarea')) return true; | |||
| 159 | + | const input = el.closest('input'); | |||
| 160 | + | 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); | |||
| 163 | + | } | |||
| 164 | + | ||||
| 153 | document.addEventListener('keydown', function(e) { | 165 | document.addEventListener('keydown', function(e) { | ||
| 166 | + | if (!isEditableTarget(e.target)) return; | |||
| 167 | + | ||||
| 154 | // Only handle a-z keys, with optional Shift | 168 | // Only handle a-z keys, with optional Shift | ||
| 155 | // Skip if Ctrl or Alt are held (those are shortcuts) | 169 | // Skip if Ctrl or Alt are held (those are shortcuts) | ||
| 156 | if (e.ctrlKey || e.altKey || e.metaKey) return; | 170 | if (e.ctrlKey || e.altKey || e.metaKey) return; | ||
| @@ -169,6 +183,7 @@ const accentDetectionScript = `(function() { | |||||
| 169 | }, true); // capture phase to see events first | 183 | }, true); // capture phase to see events first | ||
| 170 | 184 | ||||
| 171 | document.addEventListener('keyup', function(e) { | 185 | document.addEventListener('keyup', function(e) { | ||
| 186 | + | if (!isEditableTarget(e.target)) return; | |||
| 172 | const key = e.key.toLowerCase(); | 187 | const key = e.key.toLowerCase(); | ||
| 173 | if (key === pressedKey) { | 188 | if (key === pressedKey) { | ||
| 174 | pressedKey = null; | 189 | pressedKey = null; | ||
M internal/ui/app.go +3 -3
| @@ -671,12 +671,12 @@ func (a *App) initKeyboardHandler(ctx context.Context) { | |||
| 671 | 671 | ||
| 672 | 672 | if wsView.IsOmniboxVisible() || wsView.IsFindBarVisible() { | |
| 673 | 673 | // GTK Entry focused (omnibox or find bar): Alt-modified keys go to | |
| 674 | - | // shortcuts (pane navigation), text keys go to accent detection | |
| 675 | - | // for long-press accent picker | |
| 674 | + | // shortcuts (pane navigation), other keys pass through to the widget's | |
| 675 | + | // own capture-phase key controller (which handles accent detection) | |
| 676 | 676 | if kc.Modifiers&input.ModAlt != 0 { | |
| 677 | 677 | return input.RouteHandleShortcuts | |
| 678 | 678 | } | |
| 679 | - | return input.RouteAccentDetection | |
| 679 | + | return input.RoutePassToWidget | |
| 680 | 680 | } | |
| 681 | 681 | ||
| 682 | 682 | // WebView focused: text/dead keys pass through for native IM compose, | |
| @@ -671,12 +671,12 @@ func (a *App) initKeyboardHandler(ctx context.Context) { | |||||
| 671 | 671 | ||||
| 672 | if wsView.IsOmniboxVisible() || wsView.IsFindBarVisible() { | 672 | if wsView.IsOmniboxVisible() || wsView.IsFindBarVisible() { | ||
| 673 | // GTK Entry focused (omnibox or find bar): Alt-modified keys go to | 673 | // GTK Entry focused (omnibox or find bar): Alt-modified keys go to | ||
| 674 | - | // shortcuts (pane navigation), text keys go to accent detection | 674 | + | // shortcuts (pane navigation), other keys pass through to the widget's |
| 675 | - | // for long-press accent picker | 675 | + | // own capture-phase key controller (which handles accent detection) |
| 676 | if kc.Modifiers&input.ModAlt != 0 { | 676 | if kc.Modifiers&input.ModAlt != 0 { | ||
| 677 | return input.RouteHandleShortcuts | 677 | return input.RouteHandleShortcuts | ||
| 678 | } | 678 | } | ||
| 679 | - | return input.RouteAccentDetection | 679 | + | return input.RoutePassToWidget |
| 680 | } | 680 | } | ||
| 681 | 681 | ||||
| 682 | // WebView focused: text/dead keys pass through for native IM compose, | 682 | // WebView focused: text/dead keys pass through for native IM compose, | ||
M internal/ui/input/keyboard.go +9 -5
| @@ -239,15 +239,19 @@ func (h *KeyboardHandler) handleKeyPress(keyval, keycode uint, state gdk.Modifie | |||
| 239 | 239 | } | |
| 240 | 240 | ||
| 241 | 241 | switch route { | |
| 242 | - | case RoutePassToWidget, RouteAccentDetection: | |
| 243 | - | // Try long-press accent detection first. This enables the accent picker | |
| 244 | - | // universally -- for both GTK Entry widgets and WebView text inputs. | |
| 242 | + | case RoutePassToWidget: | |
| 243 | + | // Let the focused widget handle this key directly (WebView IM, etc.) | |
| 244 | + | // No accent detection here -- WebView has its own JS-based accent detection. | |
| 245 | + | log.Trace().Uint("keyval", keyval).Msg("routing key to focused widget") | |
| 246 | + | return false | |
| 247 | + | ||
| 248 | + | case RouteAccentDetection: | |
| 249 | + | // Try long-press accent detection for GTK Entry widgets (omnibox, find bar). | |
| 245 | 250 | // If the accent handler doesn't consume the key, pass it to the widget. | |
| 246 | 251 | if h.tryAccentDetection(accentHandler, keyval, modifiers) { | |
| 247 | 252 | return true | |
| 248 | 253 | } | |
| 249 | - | // Let the focused widget handle this key (WebView IM, GTK Entry, etc.) | |
| 250 | - | log.Trace().Uint("keyval", keyval).Msg("routing key to focused widget") | |
| 254 | + | log.Trace().Uint("keyval", keyval).Msg("accent detection declined, routing to widget") | |
| 251 | 255 | return false | |
| 252 | 256 | ||
| 253 | 257 | case RouteHandleShortcuts: | |
| @@ -239,15 +239,19 @@ func (h *KeyboardHandler) handleKeyPress(keyval, keycode uint, state gdk.Modifie | |||||
| 239 | } | 239 | } | ||
| 240 | 240 | ||||
| 241 | switch route { | 241 | switch route { | ||
| 242 | - | case RoutePassToWidget, RouteAccentDetection: | 242 | + | case RoutePassToWidget: |
| 243 | - | // Try long-press accent detection first. This enables the accent picker | 243 | + | // Let the focused widget handle this key directly (WebView IM, etc.) |
| 244 | - | // universally -- for both GTK Entry widgets and WebView text inputs. | 244 | + | // No accent detection here -- WebView has its own JS-based accent detection. |
| 245 | + | log.Trace().Uint("keyval", keyval).Msg("routing key to focused widget") | |||
| 246 | + | return false | |||
| 247 | + | ||||
| 248 | + | case RouteAccentDetection: | |||
| 249 | + | // Try long-press accent detection for GTK Entry widgets (omnibox, find bar). | |||
| 245 | // If the accent handler doesn't consume the key, pass it to the widget. | 250 | // If the accent handler doesn't consume the key, pass it to the widget. | ||
| 246 | if h.tryAccentDetection(accentHandler, keyval, modifiers) { | 251 | if h.tryAccentDetection(accentHandler, keyval, modifiers) { | ||
| 247 | return true | 252 | return true | ||
| 248 | } | 253 | } | ||
| 249 | - | // Let the focused widget handle this key (WebView IM, GTK Entry, etc.) | 254 | + | log.Trace().Uint("keyval", keyval).Msg("accent detection declined, routing to widget") |
| 250 | - | log.Trace().Uint("keyval", keyval).Msg("routing key to focused widget") | |||
| 251 | return false | 255 | return false | ||
| 252 | 256 | ||||
| 253 | case RouteHandleShortcuts: | 257 | case RouteHandleShortcuts: | ||
M internal/ui/input/keyboard_test.go +15 -0
| @@ -20,6 +20,18 @@ func newTestConfig() *config.Config { | |||
| 20 | 20 | return cfg | |
| 21 | 21 | } | |
| 22 | 22 | ||
| 23 | + | // stubAccentHandler is a test stub for the AccentHandler interface. | |
| 24 | + | type stubAccentHandler struct { | |
| 25 | + | onKeyPressedResult bool | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | func (s *stubAccentHandler) OnKeyPressed(_ context.Context, _ rune, _ bool) bool { | |
| 29 | + | return s.onKeyPressedResult | |
| 30 | + | } | |
| 31 | + | func (s *stubAccentHandler) OnKeyReleased(_ context.Context, _ rune) {} | |
| 32 | + | func (s *stubAccentHandler) IsPickerVisible() bool { return false } | |
| 33 | + | func (s *stubAccentHandler) Cancel(_ context.Context) {} | |
| 34 | + | ||
| 23 | 35 | func TestIsShortcutModified(t *testing.T) { | |
| 24 | 36 | tests := []struct { | |
| 25 | 37 | name string | |
| @@ -127,6 +139,9 @@ func TestHandleKeyPress_RouteAccentDetection_NoAccent(t *testing.T) { | |||
| 127 | 139 | ||
| 128 | 140 | h := NewKeyboardHandler(ctx, cfg) | |
| 129 | 141 | ||
| 142 | + | // Set a stub accent handler that always returns false (simulates "no accents for this key") | |
| 143 | + | h.SetAccentHandler(&stubAccentHandler{onKeyPressedResult: false}) | |
| 144 | + | ||
| 130 | 145 | // Route to accent detection | |
| 131 | 146 | h.SetRouteKey(func(kc KeyContext) KeyRoute { | |
| 132 | 147 | return RouteAccentDetection | |
| @@ -20,6 +20,18 @@ func newTestConfig() *config.Config { | |||||
| 20 | return cfg | 20 | return cfg | ||
| 21 | } | 21 | } | ||
| 22 | 22 | ||||
| 23 | + | // stubAccentHandler is a test stub for the AccentHandler interface. | |||
| 24 | + | type stubAccentHandler struct { | |||
| 25 | + | onKeyPressedResult bool | |||
| 26 | + | } | |||
| 27 | + | ||||
| 28 | + | func (s *stubAccentHandler) OnKeyPressed(_ context.Context, _ rune, _ bool) bool { | |||
| 29 | + | return s.onKeyPressedResult | |||
| 30 | + | } | |||
| 31 | + | func (s *stubAccentHandler) OnKeyReleased(_ context.Context, _ rune) {} | |||
| 32 | + | func (s *stubAccentHandler) IsPickerVisible() bool { return false } | |||
| 33 | + | func (s *stubAccentHandler) Cancel(_ context.Context) {} | |||
| 34 | + | ||||
| 23 | func TestIsShortcutModified(t *testing.T) { | 35 | func TestIsShortcutModified(t *testing.T) { | ||
| 24 | tests := []struct { | 36 | tests := []struct { | ||
| 25 | name string | 37 | name string | ||
| @@ -127,6 +139,9 @@ func TestHandleKeyPress_RouteAccentDetection_NoAccent(t *testing.T) { | |||||
| 127 | 139 | ||||
| 128 | h := NewKeyboardHandler(ctx, cfg) | 140 | h := NewKeyboardHandler(ctx, cfg) | ||
| 129 | 141 | ||||
| 142 | + | // Set a stub accent handler that always returns false (simulates "no accents for this key") | |||
| 143 | + | h.SetAccentHandler(&stubAccentHandler{onKeyPressedResult: false}) | |||
| 144 | + | ||||
| 130 | // Route to accent detection | 145 | // Route to accent detection | ||
| 131 | h.SetRouteKey(func(kc KeyContext) KeyRoute { | 146 | h.SetRouteKey(func(kc KeyContext) KeyRoute { | ||
| 132 | return RouteAccentDetection | 147 | return RouteAccentDetection | ||