detached 4f57229

fix(shortcuts): implement Ctrl+W pane closing with active WebView detection and segfault prevention

commit
4f572297a546c974a5663b30443bf261c602cfba
author
bamen · · 2025-09-22 21:38 UTC
committer
bamen · · 2025-09-22 17:45 UTC
parent
681ce9a
signature
unsigned
5 files · +92 -23
M CHANGELOG.md +2 -0
@@ -8,6 +8,8 @@ All notable changes to this project will be documented in this file.
88 - **Print functionality**: Added Ctrl+Shift+P shortcut to open native print dialogs from any pane. Ctrl+P is blocked at WebKit level to prevent conflicts with pane mode shortcuts
99
1010 ### Fixed
11+- **Ctrl+W pane closing**: Fixed Ctrl+W shortcut to properly close active panes including popups. Implemented active WebView detection and proper popup vs regular pane handling via OnWorkspaceMessage for consistent cleanup
12+- **WebView script injection segfault**: Fixed segmentation violation when injecting JavaScript into destroyed WebViews during message handling. Added IsDestroyed() safety check to prevent GTK widget corruption
1113 - **Alt+Arrow workspace navigation**: Fixed Alt+Arrow keys not working for pane navigation after WebView focus changes. Implemented C-level to GTK4 shortcut bridge that ensures workspace navigation works consistently across all panes
1214 - **Global shortcuts protection**: Fixed window shortcuts (Ctrl+L, Ctrl+F, F12) not working in new panes due to overly aggressive webpage protection. Window shortcuts now properly bubble up to GTK while pane shortcuts remain blocked from webpages. Resolves omnibox and developer tools not responding to keyboard shortcuts in newly created panes
1315 - **Omnibox keyboard event isolation**: Fixed keyboard event leakage from omnibox to underlying webpages that caused unintended page actions (e.g., typing 's' in omnibox triggering GitHub search). Implemented WebKit-level main-world event blocking that activates when omnibox opens, preventing page JavaScript from receiving keyboard events while preserving native GTK shortcuts (Ctrl+L, Ctrl+F, etc.) and omnibox functionality
@@ -8,6 +8,8 @@ All notable changes to this project will be documented in this file.
8 - **Print functionality**: Added Ctrl+Shift+P shortcut to open native print dialogs from any pane. Ctrl+P is blocked at WebKit level to prevent conflicts with pane mode shortcuts8 - **Print functionality**: Added Ctrl+Shift+P shortcut to open native print dialogs from any pane. Ctrl+P is blocked at WebKit level to prevent conflicts with pane mode shortcuts
9 9
10 ### Fixed10 ### Fixed
11+- **Ctrl+W pane closing**: Fixed Ctrl+W shortcut to properly close active panes including popups. Implemented active WebView detection and proper popup vs regular pane handling via OnWorkspaceMessage for consistent cleanup
12+- **WebView script injection segfault**: Fixed segmentation violation when injecting JavaScript into destroyed WebViews during message handling. Added IsDestroyed() safety check to prevent GTK widget corruption
11 - **Alt+Arrow workspace navigation**: Fixed Alt+Arrow keys not working for pane navigation after WebView focus changes. Implemented C-level to GTK4 shortcut bridge that ensures workspace navigation works consistently across all panes13 - **Alt+Arrow workspace navigation**: Fixed Alt+Arrow keys not working for pane navigation after WebView focus changes. Implemented C-level to GTK4 shortcut bridge that ensures workspace navigation works consistently across all panes
12 - **Global shortcuts protection**: Fixed window shortcuts (Ctrl+L, Ctrl+F, F12) not working in new panes due to overly aggressive webpage protection. Window shortcuts now properly bubble up to GTK while pane shortcuts remain blocked from webpages. Resolves omnibox and developer tools not responding to keyboard shortcuts in newly created panes14 - **Global shortcuts protection**: Fixed window shortcuts (Ctrl+L, Ctrl+F, F12) not working in new panes due to overly aggressive webpage protection. Window shortcuts now properly bubble up to GTK while pane shortcuts remain blocked from webpages. Resolves omnibox and developer tools not responding to keyboard shortcuts in newly created panes
13 - **Omnibox keyboard event isolation**: Fixed keyboard event leakage from omnibox to underlying webpages that caused unintended page actions (e.g., typing 's' in omnibox triggering GitHub search). Implemented WebKit-level main-world event blocking that activates when omnibox opens, preventing page JavaScript from receiving keyboard events while preserving native GTK shortcuts (Ctrl+L, Ctrl+F, etc.) and omnibox functionality15 - **Omnibox keyboard event isolation**: Fixed keyboard event leakage from omnibox to underlying webpages that caused unintended page actions (e.g., typing 's' in omnibox triggering GitHub search). Implemented WebKit-level main-world event blocking that activates when omnibox opens, preventing page JavaScript from receiving keyboard events while preserving native GTK shortcuts (Ctrl+L, Ctrl+F, etc.) and omnibox functionality
M internal/app/browser/window_shortcuts.go +35 -6
@@ -6,6 +6,7 @@ import (
66 "sync"
77 "time"
88
9+ "github.com/bnema/dumber/internal/app/messaging"
910 "github.com/bnema/dumber/pkg/webkit"
1011 )
1112
@@ -313,15 +314,43 @@ func (h *WindowShortcutHandler) handleClosePane() {
313314 return
314315 }
315316
316- if h.app.activePane == nil {
317- log.Printf("[window-shortcuts] No active pane to close")
318- return
317+ // Find the currently active WebView
318+ var activeWebView *webkit.WebView
319+ log.Printf("[window-shortcuts] Searching for active WebView among %d WebViews", len(h.app.workspace.viewToNode))
320+ for webView := range h.app.workspace.viewToNode {
321+ if webView != nil {
322+ isActive := webView.IsActive()
323+ log.Printf("[window-shortcuts] WebView %s: IsActive=%t", webView.ID(), isActive)
324+ if isActive {
325+ activeWebView = webView
326+ log.Printf("[window-shortcuts] Found active WebView: %s", webView.ID())
327+ break
328+ }
329+ }
319330 }
320331
321- log.Printf("[window-shortcuts] Closing current pane")
332+ if activeWebView == nil {
333+ log.Printf("[window-shortcuts] No active WebView found, using workspace closeCurrentPane")
334+ h.app.workspace.closeCurrentPane()
335+ return
336+ }
322337
323- // Use the existing closeCurrentPane method from workspace manager
324- h.app.workspace.closeCurrentPane()
338+ // Check if the active WebView is a popup
339+ node := h.app.workspace.viewToNode[activeWebView]
340+ if node != nil && node.isPopup {
341+ log.Printf("[window-shortcuts] Closing popup via OnWorkspaceMessage")
342+ // Use the proper close-popup message for popups
343+ msg := messaging.Message{
344+ Event: "close-popup",
345+ WebViewID: activeWebView.ID(),
346+ Reason: "user-ctrl-w",
347+ }
348+ h.app.workspace.OnWorkspaceMessage(activeWebView, msg)
349+ } else {
350+ log.Printf("[window-shortcuts] Closing regular pane via workspace closeCurrentPane")
351+ // Use regular close for non-popup panes
352+ h.app.workspace.closeCurrentPane()
353+ }
325354 }
326355
327356 // Cleanup releases resources
@@ -6,6 +6,7 @@ import (
6 "sync"6 "sync"
7 "time"7 "time"
8 8
9+ "github.com/bnema/dumber/internal/app/messaging"
9 "github.com/bnema/dumber/pkg/webkit"10 "github.com/bnema/dumber/pkg/webkit"
10 )11 )
11 12
@@ -313,15 +314,43 @@ func (h *WindowShortcutHandler) handleClosePane() {
313 return314 return
314 }315 }
315 316
316- if h.app.activePane == nil {317+ // Find the currently active WebView
317- log.Printf("[window-shortcuts] No active pane to close")318+ var activeWebView *webkit.WebView
318- return319+ log.Printf("[window-shortcuts] Searching for active WebView among %d WebViews", len(h.app.workspace.viewToNode))
320+ for webView := range h.app.workspace.viewToNode {
321+ if webView != nil {
322+ isActive := webView.IsActive()
323+ log.Printf("[window-shortcuts] WebView %s: IsActive=%t", webView.ID(), isActive)
324+ if isActive {
325+ activeWebView = webView
326+ log.Printf("[window-shortcuts] Found active WebView: %s", webView.ID())
327+ break
328+ }
329+ }
319 }330 }
320 331
321- log.Printf("[window-shortcuts] Closing current pane")332+ if activeWebView == nil {
333+ log.Printf("[window-shortcuts] No active WebView found, using workspace closeCurrentPane")
334+ h.app.workspace.closeCurrentPane()
335+ return
336+ }
322 337
323- // Use the existing closeCurrentPane method from workspace manager338+ // Check if the active WebView is a popup
324- h.app.workspace.closeCurrentPane()339+ node := h.app.workspace.viewToNode[activeWebView]
340+ if node != nil && node.isPopup {
341+ log.Printf("[window-shortcuts] Closing popup via OnWorkspaceMessage")
342+ // Use the proper close-popup message for popups
343+ msg := messaging.Message{
344+ Event: "close-popup",
345+ WebViewID: activeWebView.ID(),
346+ Reason: "user-ctrl-w",
347+ }
348+ h.app.workspace.OnWorkspaceMessage(activeWebView, msg)
349+ } else {
350+ log.Printf("[window-shortcuts] Closing regular pane via workspace closeCurrentPane")
351+ // Use regular close for non-popup panes
352+ h.app.workspace.closeCurrentPane()
353+ }
325 }354 }
326 355
327 // Cleanup releases resources356 // Cleanup releases resources
M internal/app/messaging/handler.go +6 -2
@@ -95,13 +95,11 @@ func (h *Handler) SetWorkspaceObserver(observer WorkspaceObserver) {
9595
9696 // Handle processes incoming script messages
9797 func (h *Handler) Handle(payload string) {
98- log.Printf("[DEBUG] Received message: %s", payload)
9998 var msg Message
10099 if err := json.Unmarshal([]byte(payload), &msg); err != nil {
101100 log.Printf("[ERROR] Failed to unmarshal message: %v", err)
102101 return
103102 }
104- log.Printf("[DEBUG] Parsed message type: %s", msg.Type)
105103
106104 switch msg.Type {
107105 case "navigate":
@@ -612,6 +610,12 @@ func (h *Handler) handleWebViewIDRequest(msg Message) {
612610 return
613611 }
614612
613+ // Check if WebView is destroyed before attempting any operations
614+ if h.webView.IsDestroyed() {
615+ log.Printf("[messaging] Cannot provide webview ID - webview is destroyed")
616+ return
617+ }
618+
615619 webViewID := h.webView.ID()
616620 log.Printf("[messaging] Sending webview ID %s to JavaScript", webViewID)
617621
@@ -95,13 +95,11 @@ func (h *Handler) SetWorkspaceObserver(observer WorkspaceObserver) {
95 95
96 // Handle processes incoming script messages96 // Handle processes incoming script messages
97 func (h *Handler) Handle(payload string) {97 func (h *Handler) Handle(payload string) {
98- log.Printf("[DEBUG] Received message: %s", payload)
99 var msg Message98 var msg Message
100 if err := json.Unmarshal([]byte(payload), &msg); err != nil {99 if err := json.Unmarshal([]byte(payload), &msg); err != nil {
101 log.Printf("[ERROR] Failed to unmarshal message: %v", err)100 log.Printf("[ERROR] Failed to unmarshal message: %v", err)
102 return101 return
103 }102 }
104- log.Printf("[DEBUG] Parsed message type: %s", msg.Type)
105 103
106 switch msg.Type {104 switch msg.Type {
107 case "navigate":105 case "navigate":
@@ -612,6 +610,12 @@ func (h *Handler) handleWebViewIDRequest(msg Message) {
612 return610 return
613 }611 }
614 612
613+ // Check if WebView is destroyed before attempting any operations
614+ if h.webView.IsDestroyed() {
615+ log.Printf("[messaging] Cannot provide webview ID - webview is destroyed")
616+ return
617+ }
618+
615 webViewID := h.webView.ID()619 webViewID := h.webView.ID()
616 log.Printf("[messaging] Sending webview ID %s to JavaScript", webViewID)620 log.Printf("[messaging] Sending webview ID %s to JavaScript", webViewID)
617 621
M pkg/webkit/keyboard_cgo.go +44 -15
@@ -153,6 +153,36 @@ func goOnKeyPress(id C.ulong, keyval C.guint, state C.GdkModifierType) C.gboolea
153153 return C.gboolean(0)
154154 }
155155
156+// getCLevelBridgeShortcut returns the shortcut string for shortcuts that need C-level bridge handling.
157+// These shortcuts are blocked at the webpage level and have their callbacks triggered directly from C code.
158+// Returns empty string if the shortcut doesn't need C-level handling.
159+func getCLevelBridgeShortcut(ctrl, alt, shift bool, keyName string) string {
160+ // Alt+Arrow shortcuts - for workspace pane navigation
161+ if alt && !ctrl && !shift {
162+ switch keyName {
163+ case "ArrowLeft":
164+ return "alt+arrowleft"
165+ case "ArrowRight":
166+ return "alt+arrowright"
167+ case "ArrowUp":
168+ return "alt+arrowup"
169+ case "ArrowDown":
170+ return "alt+arrowdown"
171+ }
172+ }
173+
174+ // Ctrl+W shortcut - for closing current pane via workspace manager
175+ if ctrl && !alt && !shift && keyName == "w" {
176+ return "ctrl+w"
177+ }
178+
179+ // Add other C-level bridge shortcuts here as needed
180+ // Note: Most shortcuts (Ctrl+L, Ctrl+F, zoom) should NOT be here
181+ // They work fine through normal GTK4 handling without C-level intervention
182+
183+ return ""
184+}
185+
156186 func dispatchAccelerator(uid uintptr, keyval uint, state uint) bool {
157187 ctrl := (state & uint(C.GDK_CONTROL_MASK)) != 0
158188 alt := (state & uint(C.GDK_ALT_MASK)) != 0
@@ -199,6 +229,8 @@ func dispatchAccelerator(uid uintptr, keyval uint, state uint) bool {
199229 keyName = "f"
200230 case uint(C.GDK_KEY_p), uint(C.GDK_KEY_P):
201231 keyName = "p"
232+ case uint(C.GDK_KEY_w), uint(C.GDK_KEY_W):
233+ keyName = "w"
202234 default:
203235 // Filter out modifier keys themselves (like Ctrl, Alt, Shift)
204236 // These keyvals shouldn't be logged as misses
@@ -220,21 +252,18 @@ func dispatchAccelerator(uid uintptr, keyval uint, state uint) bool {
220252 return false
221253 }
222254
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- }
255+ // Check for shortcuts that need C-level handling and trigger if found
256+ shortcut := getCLevelBridgeShortcut(ctrl, alt, shift, keyName)
257+ if shortcut != "" {
258+ // For all C-level bridge shortcuts (Alt+Arrow and Ctrl+W), use global window shortcut handler
259+ // The window shortcut handler now properly handles both popup and regular panes
260+ shortcutsRegistryMu.RLock()
261+ handle, exists := globalShortcutHandles[shortcut]
262+ shortcutsRegistryMu.RUnlock()
263+
264+ if exists && handle != 0 {
265+ invokeWindowShortcutCallback(handle)
266+ return true
238267 }
239268 }
240269
@@ -153,6 +153,36 @@ func goOnKeyPress(id C.ulong, keyval C.guint, state C.GdkModifierType) C.gboolea
153 return C.gboolean(0)153 return C.gboolean(0)
154 }154 }
155 155
156+// getCLevelBridgeShortcut returns the shortcut string for shortcuts that need C-level bridge handling.
157+// These shortcuts are blocked at the webpage level and have their callbacks triggered directly from C code.
158+// Returns empty string if the shortcut doesn't need C-level handling.
159+func getCLevelBridgeShortcut(ctrl, alt, shift bool, keyName string) string {
160+ // Alt+Arrow shortcuts - for workspace pane navigation
161+ if alt && !ctrl && !shift {
162+ switch keyName {
163+ case "ArrowLeft":
164+ return "alt+arrowleft"
165+ case "ArrowRight":
166+ return "alt+arrowright"
167+ case "ArrowUp":
168+ return "alt+arrowup"
169+ case "ArrowDown":
170+ return "alt+arrowdown"
171+ }
172+ }
173+
174+ // Ctrl+W shortcut - for closing current pane via workspace manager
175+ if ctrl && !alt && !shift && keyName == "w" {
176+ return "ctrl+w"
177+ }
178+
179+ // Add other C-level bridge shortcuts here as needed
180+ // Note: Most shortcuts (Ctrl+L, Ctrl+F, zoom) should NOT be here
181+ // They work fine through normal GTK4 handling without C-level intervention
182+
183+ return ""
184+}
185+
156 func dispatchAccelerator(uid uintptr, keyval uint, state uint) bool {186 func dispatchAccelerator(uid uintptr, keyval uint, state uint) bool {
157 ctrl := (state & uint(C.GDK_CONTROL_MASK)) != 0187 ctrl := (state & uint(C.GDK_CONTROL_MASK)) != 0
158 alt := (state & uint(C.GDK_ALT_MASK)) != 0188 alt := (state & uint(C.GDK_ALT_MASK)) != 0
@@ -199,6 +229,8 @@ func dispatchAccelerator(uid uintptr, keyval uint, state uint) bool {
199 keyName = "f"229 keyName = "f"
200 case uint(C.GDK_KEY_p), uint(C.GDK_KEY_P):230 case uint(C.GDK_KEY_p), uint(C.GDK_KEY_P):
201 keyName = "p"231 keyName = "p"
232+ case uint(C.GDK_KEY_w), uint(C.GDK_KEY_W):
233+ keyName = "w"
202 default:234 default:
203 // Filter out modifier keys themselves (like Ctrl, Alt, Shift)235 // Filter out modifier keys themselves (like Ctrl, Alt, Shift)
204 // These keyvals shouldn't be logged as misses236 // These keyvals shouldn't be logged as misses
@@ -220,21 +252,18 @@ func dispatchAccelerator(uid uintptr, keyval uint, state uint) bool {
220 return false252 return false
221 }253 }
222 254
223- // Check for Alt+Arrow shortcuts that need C-level handling255+ // Check for shortcuts that need C-level handling and trigger if found
224- if alt && !ctrl && !shift {256+ shortcut := getCLevelBridgeShortcut(ctrl, alt, shift, keyName)
225- if keyName == "ArrowLeft" || keyName == "ArrowRight" || keyName == "ArrowUp" || keyName == "ArrowDown" {257+ if shortcut != "" {
226- // Build shortcut string for Alt+Arrow258+ // For all C-level bridge shortcuts (Alt+Arrow and Ctrl+W), use global window shortcut handler
227- shortcutStr := "alt+arrow" + strings.ToLower(strings.TrimPrefix(keyName, "Arrow"))259+ // The window shortcut handler now properly handles both popup and regular panes
228-260+ shortcutsRegistryMu.RLock()
229- // Look up callback handle and trigger if found261+ handle, exists := globalShortcutHandles[shortcut]
230- shortcutsRegistryMu.RLock()262+ shortcutsRegistryMu.RUnlock()
231- handle, exists := globalShortcutHandles[shortcutStr]263+
232- shortcutsRegistryMu.RUnlock()264+ if exists && handle != 0 {
233-265+ invokeWindowShortcutCallback(handle)
234- if exists && handle != 0 {266+ return true
235- invokeWindowShortcutCallback(handle)
236- return true
237- }
238 }267 }
239 }268 }
240 269
M pkg/webkit/webview_cgo.go +5 -0
@@ -2617,6 +2617,11 @@ func (w *WebView) IsActive() bool {
26172617 return w != nil && w.isActive
26182618 }
26192619
2620+// IsDestroyed returns whether this WebView has been destroyed
2621+func (w *WebView) IsDestroyed() bool {
2622+ return w == nil || w.destroyed
2623+}
2624+
26202625 // ID returns the unique identifier for this WebView as a string
26212626 func (w *WebView) ID() string {
26222627 if w == nil {
@@ -2617,6 +2617,11 @@ func (w *WebView) IsActive() bool {
2617 return w != nil && w.isActive2617 return w != nil && w.isActive
2618 }2618 }
2619 2619
2620+// IsDestroyed returns whether this WebView has been destroyed
2621+func (w *WebView) IsDestroyed() bool {
2622+ return w == nil || w.destroyed
2623+}
2624+
2620 // ID returns the unique identifier for this WebView as a string2625 // ID returns the unique identifier for this WebView as a string
2621 func (w *WebView) ID() string {2626 func (w *WebView) ID() string {
2622 if w == nil {2627 if w == nil {

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 8

Tags 42

↑↓ moveenter openesc close