f9984f3 fix: address copilot review (Focus delegation, signal cleanup, changelog) · dumber

M CHANGELOG.md +4 -2


@@ -6,17 +6,19 @@ All notable changes to this project will be documented in this file.
6	6
7	7		### Added
8	8		- Auto-copy on selection: Text selected in web pages is automatically copied to the clipboard (like zellij/tmux). Shows brief "Copied to clipboard" toast notification. Configurable via `clipboard.auto_copy_on_selection` (default: true). Fixes #83.
	9	+	- Stacked pane close buttons: Close buttons in stacked pane title bars allow closing individual panes directly. Includes proper signal cleanup on removal.
9	10
10	11		### Changed
11	12		- Flatpak runtime: Bumped to GNOME 49 runtime.
	13	+	- Focus method refactored: `Focus()` now delegates to `ApplyFocusChange()` to ensure stack index is updated when focusing stacked panes.
12	14
13	15		### Fixed
14	16		- Flatpak locale crash: Attempted fix for crash on systems with certain locale configurations by adding proper locale data to the Flatpak bundle.
15	17		- Stacked pane vertical splits: Vertical splits from stacked panes now correctly split around the entire stack instead of just the active pane.
16		-	- Pane border focus: Fixed stale border remaining on previously hovered pane when using keyboard navigation to switch focus.
	18	+	- Pane border focus: Fixed stale border remaining on previously hovered pane when using keyboard navigation to switch focus. Fixes #89.
17	19		- Geometric focus navigation: Fixed focus jumping to wrong pane when navigating left/right in complex layouts. Navigation now prioritizes panes at the same vertical level (for left/right) or horizontal level (for up/down).
18	20		- Link status overlay auto-hide: Link URL overlay now automatically hides after 10 seconds of inactivity instead of staying visible indefinitely. Timer resets when hovering new links. Fixes #82.
19		-	- Omnibox multi-word search: History search now matches all words in query (AND logic) and searches both URL and title fields. Prefix matching enabled (e.g., "git iss" matches "github.com/issues"). Fixes #87.
	21	+	- Omnibox multi-word search: History search now matches all words in query (AND logic) and searches both URL and title fields. Prefix matching enabled (e.g., "git iss" matches "github.com/issues"). FTS5 special characters are now sanitized to prevent syntax errors. Results interleave URL and title matches for better balance. Fixes #87.
20	22
21	23		## [0.23.2] - 2025-12-31
22	24

 ### Added
 - **Auto-copy on selection**: Text selected in web pages is automatically copied to the clipboard (like zellij/tmux). Shows brief "Copied to clipboard" toast notification. Configurable via `clipboard.auto_copy_on_selection` (default: true). Fixes #83.
+- **Stacked pane close buttons**: Close buttons in stacked pane title bars allow closing individual panes directly. Includes proper signal cleanup on removal.
 ### Changed
 - **Flatpak runtime**: Bumped to GNOME 49 runtime.
+- **Focus method refactored**: `Focus()` now delegates to `ApplyFocusChange()` to ensure stack index is updated when focusing stacked panes.
 ### Fixed
 - **Flatpak locale crash**: Attempted fix for crash on systems with certain locale configurations by adding proper locale data to the Flatpak bundle.
 - **Stacked pane vertical splits**: Vertical splits from stacked panes now correctly split around the entire stack instead of just the active pane.
-- **Pane border focus**: Fixed stale border remaining on previously hovered pane when using keyboard navigation to switch focus.
+- **Pane border focus**: Fixed stale border remaining on previously hovered pane when using keyboard navigation to switch focus. Fixes #89.
 - **Geometric focus navigation**: Fixed focus jumping to wrong pane when navigating left/right in complex layouts. Navigation now prioritizes panes at the same vertical level (for left/right) or horizontal level (for up/down).
 - **Link status overlay auto-hide**: Link URL overlay now automatically hides after 10 seconds of inactivity instead of staying visible indefinitely. Timer resets when hovering new links. Fixes #82.
-- **Omnibox multi-word search**: History search now matches all words in query (AND logic) and searches both URL and title fields. Prefix matching enabled (e.g., "git iss" matches "github.com/issues"). Fixes #87.
+- **Omnibox multi-word search**: History search now matches all words in query (AND logic) and searches both URL and title fields. Prefix matching enabled (e.g., "git iss" matches "github.com/issues"). FTS5 special characters are now sanitized to prevent syntax errors. Results interleave URL and title matches for better balance. Fixes #87.
 ## [0.23.2] - 2025-12-31

M internal/application/usecase/manage_panes.go +3 -21


@@ -540,31 +540,13 @@ func (uc ManagePanesUseCase) Close(ctx context.Context, ws entity.Workspace, p
540	540		}
541	541
542	542		// Focus sets the active pane in the workspace.
	543	+	// Delegates to ApplyFocusChange to ensure stack index is updated when focusing a stacked pane.
543	544		func (uc ManagePanesUseCase) Focus(ctx context.Context, ws entity.Workspace, paneID entity.PaneID) error {
544	545		log := logging.FromContext(ctx)
545		-	if uc == nil {
546		-	return fmt.Errorf("manage panes use case is nil")
547		-	}
548	546		log.Debug().Str("pane_id", string(paneID)).Msg("focusing pane")
549	547
550		-	if ws == nil {
551		-	return fmt.Errorf("workspace is required")
552		-	}
553		-
554		-	node := ws.FindPane(paneID)
555		-	if node == nil {
556		-	return fmt.Errorf("pane not found: %s", paneID)
557		-	}
558		-
559		-	oldActive := ws.ActivePaneID
560		-	ws.ActivePaneID = paneID
561		-
562		-	log.Info().
563		-	Str("from", string(oldActive)).
564		-	Str("to", string(paneID)).
565		-	Msg("focus changed")
566		-
567		-	return nil
	548	+	_, err := uc.ApplyFocusChange(ctx, ws, paneID)
	549	+	return err
568	550		}
569	551
570	552		// NavigateFocus moves focus to an adjacent pane.


@@ -540,31 +540,13 @@ func (uc ManagePanesUseCase) Close(ctx context.Context, ws entity.Workspace, p
540		}	540		}
541			541
542		// Focus sets the active pane in the workspace.	542		// Focus sets the active pane in the workspace.
			543	+	// Delegates to ApplyFocusChange to ensure stack index is updated when focusing a stacked pane.
543		func (uc ManagePanesUseCase) Focus(ctx context.Context, ws entity.Workspace, paneID entity.PaneID) error {	544		func (uc ManagePanesUseCase) Focus(ctx context.Context, ws entity.Workspace, paneID entity.PaneID) error {
544		log := logging.FromContext(ctx)	545		log := logging.FromContext(ctx)
545	-	if uc == nil {
546	-	return fmt.Errorf("manage panes use case is nil")
547	-	}
548		log.Debug().Str("pane_id", string(paneID)).Msg("focusing pane")	546		log.Debug().Str("pane_id", string(paneID)).Msg("focusing pane")
549			547
550	-	if ws == nil {	548	+	_, err := uc.ApplyFocusChange(ctx, ws, paneID)
551	-	return fmt.Errorf("workspace is required")	549	+	return err
552	-	}
553	-
554	-	node := ws.FindPane(paneID)
555	-	if node == nil {
556	-	return fmt.Errorf("pane not found: %s", paneID)
557	-	}
558	-
559	-	oldActive := ws.ActivePaneID
560	-	ws.ActivePaneID = paneID
561	-
562	-	log.Info().
563	-	Str("from", string(oldActive)).
564	-	Str("to", string(paneID)).
565	-	Msg("focus changed")
566	-
567	-	return nil
568		}	550		}
569			551
570		// NavigateFocus moves focus to an adjacent pane.	552		// NavigateFocus moves focus to an adjacent pane.

M internal/ui/layout/stacked.go +70 -23


@@ -6,6 +6,7 @@ import (
6	6		"sync"
7	7
8	8		"github.com/bnema/dumber/internal/logging"
	9	+	"github.com/jwijenbergh/puregotk/v4/gobject"
9	10		)
10	11
11	12		// ErrStackEmpty is returned when operating on an empty stack.

@@ -31,6 +32,12 @@ type stackedPane struct {
31	32		favicon ImageWidget
32	33		label LabelWidget
33	34		isActive bool
	35	+
	36	+	// Signal handler IDs for cleanup on removal
	37	+	titleClickSignalID uint32
	38	+	closeClickSignalID uint32
	39	+	titleButton ButtonWidget // stored for signal disconnection
	40	+	closeButton ButtonWidget // stored for signal disconnection
34	41		}
35	42
36	43		// StackedView manages a stack of panes where only one is visible at a time.

@@ -137,22 +144,26 @@ func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconNa
137	144		// Create title bar components
138	145		tb := sv.createTitleBar(title, faviconIconName)
139	146
	147	+	// Connect click handlers using paneID (not index, to handle removals)
	148	+	titleSignalID, closeSignalID := sv.connectTitleBarHandlers(tb, paneID)
	149	+
140	150		pane := &stackedPane{
141		-	paneID: paneID,
142		-	titleBar: tb.titleBar,
143		-	container: container,
144		-	title: title,
145		-	favicon: tb.favicon,
146		-	label: tb.label,
147		-	isActive: false,
	151	+	paneID: paneID,
	152	+	titleBar: tb.titleBar,
	153	+	container: container,
	154	+	title: title,
	155	+	favicon: tb.favicon,
	156	+	label: tb.label,
	157	+	isActive: false,
	158	+	titleClickSignalID: titleSignalID,
	159	+	closeClickSignalID: closeSignalID,
	160	+	titleButton: tb.titleButton,
	161	+	closeButton: tb.closeBtn,
148	162		}
149	163
150	164		index := len(sv.panes)
151	165		sv.panes = append(sv.panes, pane)
152	166
153		-	// Connect click handlers using paneID (not index, to handle removals)
154		-	sv.connectTitleBarHandlers(tb, paneID)
155		-
156	167		// Add to container
157	168		log.Debug().
158	169		Int("index", index).

@@ -175,9 +186,10 @@ func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconNa
175	186
176	187		// connectTitleBarHandlers connects the click handlers for a title bar.
177	188		// Uses paneID lookup instead of captured index to handle pane removals correctly.
178		-	func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID string) {
	189	+	// Returns the signal IDs for later disconnection.
	190	+	func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID string) (titleSignalID, closeSignalID uint32) {
179	191		// Connect title bar click handler for activation
180		-	tb.titleButton.ConnectClicked(func() {
	192	+	titleSignalID = tb.titleButton.ConnectClicked(func() {
181	193		sv.mu.RLock()
182	194		callback := sv.onActivate
183	195		currentIndex := sv.findPaneIndexInternal(paneID)

@@ -189,7 +201,7 @@ func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID str
189	201		})
190	202
191	203		// Connect close button click handler
192		-	tb.closeBtn.ConnectClicked(func() {
	204	+	closeSignalID = tb.closeBtn.ConnectClicked(func() {
193	205		sv.mu.RLock()
194	206		onClose := sv.onClosePane
195	207		sv.mu.RUnlock()

@@ -198,6 +210,34 @@ func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID str
198	210		onClose(paneID)
199	211		}
200	212		})
	213	+
	214	+	return titleSignalID, closeSignalID
	215	+	}
	216	+
	217	+	// disconnectPaneSignals disconnects signal handlers from a pane's buttons.
	218	+	// This prevents memory leaks when panes are removed from the stack.
	219	+	func (sv StackedView) disconnectPaneSignals(pane stackedPane) {
	220	+	if pane == nil {
	221	+	return
	222	+	}
	223	+
	224	+	// Disconnect title button click signal
	225	+	if pane.titleButton != nil && pane.titleClickSignalID != 0 {
	226	+	gtkWidget := pane.titleButton.GtkWidget()
	227	+	if gtkWidget != nil {
	228	+	obj := gobject.ObjectNewFromInternalPtr(gtkWidget.GoPointer())
	229	+	gobject.SignalHandlerDisconnect(obj, pane.titleClickSignalID)
	230	+	}
	231	+	}
	232	+
	233	+	// Disconnect close button click signal
	234	+	if pane.closeButton != nil && pane.closeClickSignalID != 0 {
	235	+	gtkWidget := pane.closeButton.GtkWidget()
	236	+	if gtkWidget != nil {
	237	+	obj := gobject.ObjectNewFromInternalPtr(gtkWidget.GoPointer())
	238	+	gobject.SignalHandlerDisconnect(obj, pane.closeClickSignalID)
	239	+	}
	240	+	}
201	241		}
202	242
203	243		// InsertPaneAfter inserts a new pane after the specified index position.

@@ -230,14 +270,21 @@ func (sv *StackedView) InsertPaneAfter(
230	270		// Create title bar components
231	271		tb := sv.createTitleBar(title, faviconIconName)
232	272
	273	+	// Connect click handlers using paneID (not index, to handle removals)
	274	+	titleSignalID, closeSignalID := sv.connectTitleBarHandlers(tb, paneID)
	275	+
233	276		pane := &stackedPane{
234		-	paneID: paneID,
235		-	titleBar: tb.titleBar,
236		-	container: container,
237		-	title: title,
238		-	favicon: tb.favicon,
239		-	label: tb.label,
240		-	isActive: false,
	277	+	paneID: paneID,
	278	+	titleBar: tb.titleBar,
	279	+	container: container,
	280	+	title: title,
	281	+	favicon: tb.favicon,
	282	+	label: tb.label,
	283	+	isActive: false,
	284	+	titleClickSignalID: titleSignalID,
	285	+	closeClickSignalID: closeSignalID,
	286	+	titleButton: tb.titleButton,
	287	+	closeButton: tb.closeBtn,
241	288		}
242	289
243	290		// Insert into slice at correct position

@@ -245,9 +292,6 @@ func (sv *StackedView) InsertPaneAfter(
245	292		copy(sv.panes[insertIndex+1:], sv.panes[insertIndex:])
246	293		sv.panes[insertIndex] = pane
247	294
248		-	// Connect click handlers using paneID (not index, to handle removals)
249		-	sv.connectTitleBarHandlers(tb, paneID)
250		-
251	295		// Insert widgets at correct position in GTK box
252	296		sv.insertTitleBarWidgets(tb.titleButton, container, insertIndex)
253	297

@@ -321,6 +365,9 @@ func (sv *StackedView) RemovePane(ctx context.Context, index int) error {
321	365
322	366		pane := sv.panes[index]
323	367
	368	+	// Disconnect signal handlers before removing widgets to prevent memory leaks
	369	+	sv.disconnectPaneSignals(pane)
	370	+
324	371		// Remove from container
325	372		if pane.titleBar != nil {
326	373		// The title bar is wrapped in a button, need to get parent

 	"sync"
 	"github.com/bnema/dumber/internal/logging"
+	"github.com/jwijenbergh/puregotk/v4/gobject"
 )
 // ErrStackEmpty is returned when operating on an empty stack.
 	favicon   ImageWidget
 	label     LabelWidget
 	isActive  bool
+	// Signal handler IDs for cleanup on removal
+	titleClickSignalID uint32
+	closeClickSignalID uint32
+	titleButton        ButtonWidget // stored for signal disconnection
+	closeButton        ButtonWidget // stored for signal disconnection
 }
 // StackedView manages a stack of panes where only one is visible at a time.
 	// Create title bar components
 	tb := sv.createTitleBar(title, faviconIconName)
+	// Connect click handlers using paneID (not index, to handle removals)
+	titleSignalID, closeSignalID := sv.connectTitleBarHandlers(tb, paneID)
 	pane := &stackedPane{
-		paneID:    paneID,
+		paneID:             paneID,
-		titleBar:  tb.titleBar,
+		titleBar:           tb.titleBar,
-		container: container,
+		container:          container,
-		title:     title,
+		title:              title,
-		favicon:   tb.favicon,
+		favicon:            tb.favicon,
-		label:     tb.label,
+		label:              tb.label,
-		isActive:  false,
+		isActive:           false,
+		titleClickSignalID: titleSignalID,
+		closeClickSignalID: closeSignalID,
+		titleButton:        tb.titleButton,
+		closeButton:        tb.closeBtn,
 	}
 	index := len(sv.panes)
 	sv.panes = append(sv.panes, pane)
-	// Connect click handlers using paneID (not index, to handle removals)
-	sv.connectTitleBarHandlers(tb, paneID)
 	// Add to container
 	log.Debug().
 		Int("index", index).
 // connectTitleBarHandlers connects the click handlers for a title bar.
 // Uses paneID lookup instead of captured index to handle pane removals correctly.
-func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID string) {
+// Returns the signal IDs for later disconnection.
+func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID string) (titleSignalID, closeSignalID uint32) {
 	// Connect title bar click handler for activation
-	tb.titleButton.ConnectClicked(func() {
+	titleSignalID = tb.titleButton.ConnectClicked(func() {
 		sv.mu.RLock()
 		callback := sv.onActivate
 		currentIndex := sv.findPaneIndexInternal(paneID)
 	})
 	// Connect close button click handler
-	tb.closeBtn.ConnectClicked(func() {
+	closeSignalID = tb.closeBtn.ConnectClicked(func() {
 		sv.mu.RLock()
 		onClose := sv.onClosePane
 		sv.mu.RUnlock()
 			onClose(paneID)
 		}
 	})
+	return titleSignalID, closeSignalID
+}
+// disconnectPaneSignals disconnects signal handlers from a pane's buttons.
+// This prevents memory leaks when panes are removed from the stack.
+func (sv *StackedView) disconnectPaneSignals(pane *stackedPane) {
+	if pane == nil {
+		return
+	}
+	// Disconnect title button click signal
+	if pane.titleButton != nil && pane.titleClickSignalID != 0 {
+		gtkWidget := pane.titleButton.GtkWidget()
+		if gtkWidget != nil {
+			obj := gobject.ObjectNewFromInternalPtr(gtkWidget.GoPointer())
+			gobject.SignalHandlerDisconnect(obj, pane.titleClickSignalID)
+		}
+	}
+	// Disconnect close button click signal
+	if pane.closeButton != nil && pane.closeClickSignalID != 0 {
+		gtkWidget := pane.closeButton.GtkWidget()
+		if gtkWidget != nil {
+			obj := gobject.ObjectNewFromInternalPtr(gtkWidget.GoPointer())
+			gobject.SignalHandlerDisconnect(obj, pane.closeClickSignalID)
+		}
+	}
 }
 // InsertPaneAfter inserts a new pane after the specified index position.
 	// Create title bar components
 	tb := sv.createTitleBar(title, faviconIconName)
+	// Connect click handlers using paneID (not index, to handle removals)
+	titleSignalID, closeSignalID := sv.connectTitleBarHandlers(tb, paneID)
 	pane := &stackedPane{
-		paneID:    paneID,
+		paneID:             paneID,
-		titleBar:  tb.titleBar,
+		titleBar:           tb.titleBar,
-		container: container,
+		container:          container,
-		title:     title,
+		title:              title,
-		favicon:   tb.favicon,
+		favicon:            tb.favicon,
-		label:     tb.label,
+		label:              tb.label,
-		isActive:  false,
+		isActive:           false,
+		titleClickSignalID: titleSignalID,
+		closeClickSignalID: closeSignalID,
+		titleButton:        tb.titleButton,
+		closeButton:        tb.closeBtn,
 	}
 	// Insert into slice at correct position
 	copy(sv.panes[insertIndex+1:], sv.panes[insertIndex:])
 	sv.panes[insertIndex] = pane
-	// Connect click handlers using paneID (not index, to handle removals)
-	sv.connectTitleBarHandlers(tb, paneID)
 	// Insert widgets at correct position in GTK box
 	sv.insertTitleBarWidgets(tb.titleButton, container, insertIndex)
 	pane := sv.panes[index]
+	// Disconnect signal handlers before removing widgets to prevent memory leaks
+	sv.disconnectPaneSignals(pane)
 	// Remove from container
 	if pane.titleBar != nil {
 		// The title bar is wrapped in a button, need to get parent