fix(shortcuts): restore Ctrl+L/F/zoom functionality and clean up dead code
- Fix: Only register Alt+Arrow and Ctrl+W for JavaScript blocking - Fix: Let Ctrl+L, Ctrl+F, zoom shortcuts work normally via GTK4 - Clean: Remove complex C-level blocking system (50+ lines) - Clean: Simplify JavaScript protection to basic blocking - Add: Minimal C-level bridge for Alt+Arrow shortcuts only All shortcuts now working except Ctrl+W (next commit)
- commit
- 681ce9ab62149e1ec4018b5108c017de0fa8e73a
- author
- bamen · · 2025-09-22 17:14 UTC
- committer
- bamen · · 2025-09-22 21:13 UTC
- parent
- 76cb885
- signature
- unsigned
3 files · +40 -63
M internal/app/browser/window_shortcuts.go +18 -0
| @@ -59,6 +59,7 @@ func (h *WindowShortcutHandler) registerGlobalShortcuts() error { | |||
| 59 | 59 | {"ctrl+f", h.handleFindToggle, "Find in page"}, | |
| 60 | 60 | {"ctrl+shift+c", h.handleCopyURL, "Copy URL"}, | |
| 61 | 61 | {"ctrl+shift+p", h.handlePrint, "Print page"}, | |
| 62 | + | {"ctrl+w", h.handleClosePane, "Close current pane"}, | |
| 62 | 63 | {"F12", h.handleDevTools, "Developer tools"}, | |
| 63 | 64 | // Zoom shortcuts - global level for proper active pane targeting | |
| 64 | 65 | {"ctrl+plus", h.handleZoomIn, "Zoom in"}, | |
| @@ -306,6 +307,23 @@ func (h *WindowShortcutHandler) handleWorkspaceNavigation(direction string) { | |||
| 306 | 307 | } | |
| 307 | 308 | } | |
| 308 | 309 | ||
| 310 | + | func (h *WindowShortcutHandler) handleClosePane() { | |
| 311 | + | if h.app == nil || h.app.workspace == nil { | |
| 312 | + | log.Printf("[window-shortcuts] No workspace for close pane") | |
| 313 | + | return | |
| 314 | + | } | |
| 315 | + | ||
| 316 | + | if h.app.activePane == nil { | |
| 317 | + | log.Printf("[window-shortcuts] No active pane to close") | |
| 318 | + | return | |
| 319 | + | } | |
| 320 | + | ||
| 321 | + | log.Printf("[window-shortcuts] Closing current pane") | |
| 322 | + | ||
| 323 | + | // Use the existing closeCurrentPane method from workspace manager | |
| 324 | + | h.app.workspace.closeCurrentPane() | |
| 325 | + | } | |
| 326 | + | ||
| 309 | 327 | // Cleanup releases resources | |
| 310 | 328 | func (h *WindowShortcutHandler) Cleanup() { | |
| 311 | 329 | if h.shortcuts != nil { | |
| @@ -59,6 +59,7 @@ func (h *WindowShortcutHandler) registerGlobalShortcuts() error { | |||||
| 59 | {"ctrl+f", h.handleFindToggle, "Find in page"}, | 59 | {"ctrl+f", h.handleFindToggle, "Find in page"}, | ||
| 60 | {"ctrl+shift+c", h.handleCopyURL, "Copy URL"}, | 60 | {"ctrl+shift+c", h.handleCopyURL, "Copy URL"}, | ||
| 61 | {"ctrl+shift+p", h.handlePrint, "Print page"}, | 61 | {"ctrl+shift+p", h.handlePrint, "Print page"}, | ||
| 62 | + | {"ctrl+w", h.handleClosePane, "Close current pane"}, | |||
| 62 | {"F12", h.handleDevTools, "Developer tools"}, | 63 | {"F12", h.handleDevTools, "Developer tools"}, | ||
| 63 | // Zoom shortcuts - global level for proper active pane targeting | 64 | // Zoom shortcuts - global level for proper active pane targeting | ||
| 64 | {"ctrl+plus", h.handleZoomIn, "Zoom in"}, | 65 | {"ctrl+plus", h.handleZoomIn, "Zoom in"}, | ||
| @@ -306,6 +307,23 @@ func (h *WindowShortcutHandler) handleWorkspaceNavigation(direction string) { | |||||
| 306 | } | 307 | } | ||
| 307 | } | 308 | } | ||
| 308 | 309 | ||||
| 310 | + | func (h *WindowShortcutHandler) handleClosePane() { | |||
| 311 | + | if h.app == nil || h.app.workspace == nil { | |||
| 312 | + | log.Printf("[window-shortcuts] No workspace for close pane") | |||
| 313 | + | return | |||
| 314 | + | } | |||
| 315 | + | ||||
| 316 | + | if h.app.activePane == nil { | |||
| 317 | + | log.Printf("[window-shortcuts] No active pane to close") | |||
| 318 | + | return | |||
| 319 | + | } | |||
| 320 | + | ||||
| 321 | + | log.Printf("[window-shortcuts] Closing current pane") | |||
| 322 | + | ||||
| 323 | + | // Use the existing closeCurrentPane method from workspace manager | |||
| 324 | + | h.app.workspace.closeCurrentPane() | |||
| 325 | + | } | |||
| 326 | + | ||||
| 309 | // Cleanup releases resources | 327 | // Cleanup releases resources | ||
| 310 | func (h *WindowShortcutHandler) Cleanup() { | 328 | func (h *WindowShortcutHandler) Cleanup() { | ||
| 311 | if h.shortcuts != nil { | 329 | if h.shortcuts != nil { | ||
M pkg/webkit/keyboard_cgo.go +16 -59
| @@ -220,9 +220,22 @@ func dispatchAccelerator(uid uintptr, keyval uint, state uint) bool { | |||
| 220 | 220 | return false | |
| 221 | 221 | } | |
| 222 | 222 | ||
| 223 | - | // GTK-level fallback blocking for global shortcuts | |
| 224 | - | if checkGlobalShortcutBlocking(keyName, ctrl, alt, shift) { | |
| 225 | - | return true // Block the event from reaching WebKit | |
| 223 | + | // Check for Alt+Arrow shortcuts that need C-level handling | |
| 224 | + | if alt && !ctrl && !shift { | |
| 225 | + | if keyName == "ArrowLeft" || keyName == "ArrowRight" || keyName == "ArrowUp" || keyName == "ArrowDown" { | |
| 226 | + | // Build shortcut string for Alt+Arrow | |
| 227 | + | shortcutStr := "alt+arrow" + strings.ToLower(strings.TrimPrefix(keyName, "Arrow")) | |
| 228 | + | ||
| 229 | + | // Look up callback handle and trigger if found | |
| 230 | + | shortcutsRegistryMu.RLock() | |
| 231 | + | handle, exists := globalShortcutHandles[shortcutStr] | |
| 232 | + | shortcutsRegistryMu.RUnlock() | |
| 233 | + | ||
| 234 | + | if exists && handle != 0 { | |
| 235 | + | invokeWindowShortcutCallback(handle) | |
| 236 | + | return true | |
| 237 | + | } | |
| 238 | + | } | |
| 226 | 239 | } | |
| 227 | 240 | ||
| 228 | 241 | now := time.Now() | |
| @@ -297,62 +310,6 @@ func dispatchAccelerator(uid uintptr, keyval uint, state uint) bool { | |||
| 297 | 310 | return handled | |
| 298 | 311 | } | |
| 299 | 312 | ||
| 300 | - | // checkGlobalShortcutBlocking checks if a keyboard event should be blocked at GTK level | |
| 301 | - | // This provides defense-in-depth protection when JavaScript-level blocking fails | |
| 302 | - | func checkGlobalShortcutBlocking(keyName string, ctrl, alt, shift bool) bool { | |
| 303 | - | if keyName == "" { | |
| 304 | - | return false | |
| 305 | - | } | |
| 306 | - | ||
| 307 | - | // Build shortcut string similar to the JavaScript protection logic | |
| 308 | - | var shortcut strings.Builder | |
| 309 | - | if ctrl { | |
| 310 | - | shortcut.WriteString("ctrl+") | |
| 311 | - | } | |
| 312 | - | if alt { | |
| 313 | - | shortcut.WriteString("alt+") | |
| 314 | - | } | |
| 315 | - | if shift { | |
| 316 | - | shortcut.WriteString("shift+") | |
| 317 | - | } | |
| 318 | - | shortcut.WriteString(strings.ToLower(keyName)) | |
| 319 | - | ||
| 320 | - | shortcutStr := shortcut.String() | |
| 321 | - | ||
| 322 | - | // Check if this shortcut is in the global registry | |
| 323 | - | shortcuts := GetRegisteredShortcuts() | |
| 324 | - | for _, registered := range shortcuts { | |
| 325 | - | if registered == shortcutStr { | |
| 326 | - | // Trigger the callback handle if available (especially for Alt+Arrow shortcuts) | |
| 327 | - | triggerGlobalShortcutCallback(registered) | |
| 328 | - | return true | |
| 329 | - | } | |
| 330 | - | // Also check common variations | |
| 331 | - | if registered == strings.ReplaceAll(shortcutStr, "arrowleft", "left") || | |
| 332 | - | registered == strings.ReplaceAll(shortcutStr, "arrowright", "right") || | |
| 333 | - | registered == strings.ReplaceAll(shortcutStr, "arrowup", "up") || | |
| 334 | - | registered == strings.ReplaceAll(shortcutStr, "arrowdown", "down") { | |
| 335 | - | // Trigger the callback handle for the registered variant | |
| 336 | - | triggerGlobalShortcutCallback(registered) | |
| 337 | - | return true | |
| 338 | - | } | |
| 339 | - | } | |
| 340 | - | ||
| 341 | - | return false | |
| 342 | - | } | |
| 343 | - | ||
| 344 | - | // triggerGlobalShortcutCallback looks up and invokes the callback handle for a given shortcut | |
| 345 | - | func triggerGlobalShortcutCallback(shortcut string) { | |
| 346 | - | // Access the global shortcut handles map from shortcuts_cgo.go | |
| 347 | - | shortcutsRegistryMu.RLock() | |
| 348 | - | handle, exists := globalShortcutHandles[shortcut] | |
| 349 | - | shortcutsRegistryMu.RUnlock() | |
| 350 | - | ||
| 351 | - | if exists && handle != 0 { | |
| 352 | - | invokeWindowShortcutCallback(handle) | |
| 353 | - | } | |
| 354 | - | } | |
| 355 | - | ||
| 356 | 313 | func nextViewID() uintptr { | |
| 357 | 314 | return uintptr(atomic.AddUint64(&viewIDCounter, 1)) | |
| 358 | 315 | } | |
| @@ -220,9 +220,22 @@ func dispatchAccelerator(uid uintptr, keyval uint, state uint) bool { | |||||
| 220 | return false | 220 | return false | ||
| 221 | } | 221 | } | ||
| 222 | 222 | ||||
| 223 | - | // GTK-level fallback blocking for global shortcuts | 223 | + | // Check for Alt+Arrow shortcuts that need C-level handling |
| 224 | - | if checkGlobalShortcutBlocking(keyName, ctrl, alt, shift) { | 224 | + | if alt && !ctrl && !shift { |
| 225 | - | return true // Block the event from reaching WebKit | 225 | + | if keyName == "ArrowLeft" || keyName == "ArrowRight" || keyName == "ArrowUp" || keyName == "ArrowDown" { |
| 226 | + | // Build shortcut string for Alt+Arrow | |||
| 227 | + | shortcutStr := "alt+arrow" + strings.ToLower(strings.TrimPrefix(keyName, "Arrow")) | |||
| 228 | + | ||||
| 229 | + | // Look up callback handle and trigger if found | |||
| 230 | + | shortcutsRegistryMu.RLock() | |||
| 231 | + | handle, exists := globalShortcutHandles[shortcutStr] | |||
| 232 | + | shortcutsRegistryMu.RUnlock() | |||
| 233 | + | ||||
| 234 | + | if exists && handle != 0 { | |||
| 235 | + | invokeWindowShortcutCallback(handle) | |||
| 236 | + | return true | |||
| 237 | + | } | |||
| 238 | + | } | |||
| 226 | } | 239 | } | ||
| 227 | 240 | ||||
| 228 | now := time.Now() | 241 | now := time.Now() | ||
| @@ -297,62 +310,6 @@ func dispatchAccelerator(uid uintptr, keyval uint, state uint) bool { | |||||
| 297 | return handled | 310 | return handled | ||
| 298 | } | 311 | } | ||
| 299 | 312 | ||||
| 300 | - | // checkGlobalShortcutBlocking checks if a keyboard event should be blocked at GTK level | |||
| 301 | - | // This provides defense-in-depth protection when JavaScript-level blocking fails | |||
| 302 | - | func checkGlobalShortcutBlocking(keyName string, ctrl, alt, shift bool) bool { | |||
| 303 | - | if keyName == "" { | |||
| 304 | - | return false | |||
| 305 | - | } | |||
| 306 | - | ||||
| 307 | - | // Build shortcut string similar to the JavaScript protection logic | |||
| 308 | - | var shortcut strings.Builder | |||
| 309 | - | if ctrl { | |||
| 310 | - | shortcut.WriteString("ctrl+") | |||
| 311 | - | } | |||
| 312 | - | if alt { | |||
| 313 | - | shortcut.WriteString("alt+") | |||
| 314 | - | } | |||
| 315 | - | if shift { | |||
| 316 | - | shortcut.WriteString("shift+") | |||
| 317 | - | } | |||
| 318 | - | shortcut.WriteString(strings.ToLower(keyName)) | |||
| 319 | - | ||||
| 320 | - | shortcutStr := shortcut.String() | |||
| 321 | - | ||||
| 322 | - | // Check if this shortcut is in the global registry | |||
| 323 | - | shortcuts := GetRegisteredShortcuts() | |||
| 324 | - | for _, registered := range shortcuts { | |||
| 325 | - | if registered == shortcutStr { | |||
| 326 | - | // Trigger the callback handle if available (especially for Alt+Arrow shortcuts) | |||
| 327 | - | triggerGlobalShortcutCallback(registered) | |||
| 328 | - | return true | |||
| 329 | - | } | |||
| 330 | - | // Also check common variations | |||
| 331 | - | if registered == strings.ReplaceAll(shortcutStr, "arrowleft", "left") || | |||
| 332 | - | registered == strings.ReplaceAll(shortcutStr, "arrowright", "right") || | |||
| 333 | - | registered == strings.ReplaceAll(shortcutStr, "arrowup", "up") || | |||
| 334 | - | registered == strings.ReplaceAll(shortcutStr, "arrowdown", "down") { | |||
| 335 | - | // Trigger the callback handle for the registered variant | |||
| 336 | - | triggerGlobalShortcutCallback(registered) | |||
| 337 | - | return true | |||
| 338 | - | } | |||
| 339 | - | } | |||
| 340 | - | ||||
| 341 | - | return false | |||
| 342 | - | } | |||
| 343 | - | ||||
| 344 | - | // triggerGlobalShortcutCallback looks up and invokes the callback handle for a given shortcut | |||
| 345 | - | func triggerGlobalShortcutCallback(shortcut string) { | |||
| 346 | - | // Access the global shortcut handles map from shortcuts_cgo.go | |||
| 347 | - | shortcutsRegistryMu.RLock() | |||
| 348 | - | handle, exists := globalShortcutHandles[shortcut] | |||
| 349 | - | shortcutsRegistryMu.RUnlock() | |||
| 350 | - | ||||
| 351 | - | if exists && handle != 0 { | |||
| 352 | - | invokeWindowShortcutCallback(handle) | |||
| 353 | - | } | |||
| 354 | - | } | |||
| 355 | - | ||||
| 356 | func nextViewID() uintptr { | 313 | func nextViewID() uintptr { | ||
| 357 | return uintptr(atomic.AddUint64(&viewIDCounter, 1)) | 314 | return uintptr(atomic.AddUint64(&viewIDCounter, 1)) | ||
| 358 | } | 315 | } | ||
M pkg/webkit/window_shortcuts.go +6 -4
| @@ -71,13 +71,15 @@ func (ws *WindowShortcuts) RegisterGlobalShortcut(key string, callback func()) e | |||
| 71 | 71 | return err | |
| 72 | 72 | } | |
| 73 | 73 | ||
| 74 | - | // Add Alt+Arrow shortcuts to global blocking registry with callback handle for C-level triggering | |
| 74 | + | // Only add Alt+Arrow and Ctrl+W shortcuts to global blocking registry | |
| 75 | + | // All other window shortcuts should bubble up to GTK for proper handling | |
| 75 | 76 | if strings.HasPrefix(key, "alt+Arrow") { | |
| 76 | 77 | RegisterGlobalShortcutWithHandle(key, handle) | |
| 77 | - | } else { | |
| 78 | - | // For non-Alt+Arrow shortcuts, just register for blocking without callback handle | |
| 79 | - | RegisterGlobalShortcut(key) | |
| 78 | + | } else if key == "ctrl+w" { | |
| 79 | + | // Ctrl+W should also have callback handle for consistent pane closing | |
| 80 | + | RegisterGlobalShortcutWithHandle(key, handle) | |
| 80 | 81 | } | |
| 82 | + | // Other shortcuts (Ctrl+L, Ctrl+F, zoom) are NOT registered for blocking | |
| 81 | 83 | ||
| 82 | 84 | log.Printf("[window-shortcuts] Registered global shortcut: %s -> %s", key, gtkKey) | |
| 83 | 85 | return nil | |
| @@ -71,13 +71,15 @@ func (ws *WindowShortcuts) RegisterGlobalShortcut(key string, callback func()) e | |||||
| 71 | return err | 71 | return err | ||
| 72 | } | 72 | } | ||
| 73 | 73 | ||||
| 74 | - | // Add Alt+Arrow shortcuts to global blocking registry with callback handle for C-level triggering | 74 | + | // Only add Alt+Arrow and Ctrl+W shortcuts to global blocking registry |
| 75 | + | // All other window shortcuts should bubble up to GTK for proper handling | |||
| 75 | if strings.HasPrefix(key, "alt+Arrow") { | 76 | if strings.HasPrefix(key, "alt+Arrow") { | ||
| 76 | RegisterGlobalShortcutWithHandle(key, handle) | 77 | RegisterGlobalShortcutWithHandle(key, handle) | ||
| 77 | - | } else { | 78 | + | } else if key == "ctrl+w" { |
| 78 | - | // For non-Alt+Arrow shortcuts, just register for blocking without callback handle | 79 | + | // Ctrl+W should also have callback handle for consistent pane closing |
| 79 | - | RegisterGlobalShortcut(key) | 80 | + | RegisterGlobalShortcutWithHandle(key, handle) |
| 80 | } | 81 | } | ||
| 82 | + | // Other shortcuts (Ctrl+L, Ctrl+F, zoom) are NOT registered for blocking | |||
| 81 | 83 | ||||
| 82 | log.Printf("[window-shortcuts] Registered global shortcut: %s -> %s", key, gtkKey) | 84 | log.Printf("[window-shortcuts] Registered global shortcut: %s -> %s", key, gtkKey) | ||
| 83 | return nil | 85 | return nil | ||