f5762b8 fix: address code review findings across engine abstraction · dumber

M cmd/dumber/main.go +2 -3


@@ -28,7 +28,6 @@ import (
28	28		"github.com/bnema/dumber/internal/infrastructure/snapshot"
29	29		"github.com/bnema/dumber/internal/infrastructure/textinput"
30	30		"github.com/bnema/dumber/internal/infrastructure/updater"
31		-	"github.com/bnema/dumber/internal/infrastructure/webkit"
32	31		"github.com/bnema/dumber/internal/infrastructure/xdg"
33	32		"github.com/bnema/dumber/internal/logging"
34	33		"github.com/bnema/dumber/internal/ui"

@@ -516,8 +515,8 @@ func buildUIDependencies(
516	515		startupCrashReports []string,
517	516		) *ui.Dependencies {
518	517		var filterManager port.FilterManager
519		-	if wkEngine, ok := engine.(*webkit.Engine); ok {
520		-	filterManager = wkEngine.InternalFilterManager()
	518	+	if fmp, ok := engine.(port.FilterManagerProvider); ok {
	519	+	filterManager = fmp.InternalFilterManager()
521	520		}
522	521
523	522		focusProvider := textinput.NewFocusProvider()

 	"github.com/bnema/dumber/internal/infrastructure/snapshot"
 	"github.com/bnema/dumber/internal/infrastructure/textinput"
 	"github.com/bnema/dumber/internal/infrastructure/updater"
-	"github.com/bnema/dumber/internal/infrastructure/webkit"
 	"github.com/bnema/dumber/internal/infrastructure/xdg"
 	"github.com/bnema/dumber/internal/logging"
 	"github.com/bnema/dumber/internal/ui"
 	startupCrashReports []string,
 ) *ui.Dependencies {
 	var filterManager port.FilterManager
-	if wkEngine, ok := engine.(*webkit.Engine); ok {
+	if fmp, ok := engine.(port.FilterManagerProvider); ok {
-		filterManager = wkEngine.InternalFilterManager()
+		filterManager = fmp.InternalFilterManager()
 	}
 	focusProvider := textinput.NewFocusProvider()

M internal/application/port/engine.go +7 -0


@@ -221,6 +221,13 @@ type ScriptRefresher interface {
221	221		RefreshAll(ctx context.Context, webviews []WebView)
222	222		}
223	223
	224	+	// FilterManagerProvider is an optional capability interface for engines that
	225	+	// support content filter management (e.g. ad blocking). Callers should type-assert
	226	+	// the Engine to FilterManagerProvider rather than a concrete engine type.
	227	+	type FilterManagerProvider interface {
	228	+	InternalFilterManager() FilterManager
	229	+	}
	230	+
224	231		// NativeWidgetProvider is an optional capability for WebViews that can provide
225	232		// a native widget pointer for embedding into the host toolkit's layout system.
226	233		// For GTK-based engines this returns a *gtk.Widget pointer; other engines


@@ -221,6 +221,13 @@ type ScriptRefresher interface {
221	RefreshAll(ctx context.Context, webviews []WebView)	221		RefreshAll(ctx context.Context, webviews []WebView)
222	}	222		}
223		223
		224	+	// FilterManagerProvider is an optional capability interface for engines that
		225	+	// support content filter management (e.g. ad blocking). Callers should type-assert
		226	+	// the Engine to FilterManagerProvider rather than a concrete engine type.
		227	+	type FilterManagerProvider interface {
		228	+	InternalFilterManager() FilterManager
		229	+	}
		230	+
224	// NativeWidgetProvider is an optional capability for WebViews that can provide	231		// NativeWidgetProvider is an optional capability for WebViews that can provide
225	// a native widget pointer for embedding into the host toolkit's layout system.	232		// a native widget pointer for embedding into the host toolkit's layout system.
226	// For GTK-based engines this returns a *gtk.Widget pointer; other engines	233		// For GTK-based engines this returns a *gtk.Widget pointer; other engines

M internal/application/port/rendering_env.go +2 -2


@@ -22,8 +22,8 @@ const (
22	22		// Environment variables must be set BEFORE GTK/WebKit initialization.
23	23		//
24	24		// Note: ApplyEnvironment was removed from this interface because rendering
25		-	// settings are engine-specific (WebKit/GTK/GStreamer) and have moved to
26		-	// internal/infrastructure/env.RenderingSettings to avoid port bloat.
	25	+	// settings are engine-specific and are handled by concrete implementations
	26	+	// (e.g. WebKit/GTK/GStreamer) to avoid port bloat.
27	27		type RenderingEnvManager interface {
28	28		// DetectGPUVendor identifies the primary GPU vendor from system info.
29	29		// Uses /sys/class/drm/card*/device/vendor as primary detection method.


@@ -22,8 +22,8 @@ const (
22		// Environment variables must be set BEFORE GTK/WebKit initialization.	22		// Environment variables must be set BEFORE GTK/WebKit initialization.
23		//	23		//
24		// Note: ApplyEnvironment was removed from this interface because rendering	24		// Note: ApplyEnvironment was removed from this interface because rendering
25	-	// settings are engine-specific (WebKit/GTK/GStreamer) and have moved to	25	+	// settings are engine-specific and are handled by concrete implementations
26	-	// internal/infrastructure/env.RenderingSettings to avoid port bloat.	26	+	// (e.g. WebKit/GTK/GStreamer) to avoid port bloat.
27		type RenderingEnvManager interface {	27		type RenderingEnvManager interface {
28		// DetectGPUVendor identifies the primary GPU vendor from system info.	28		// DetectGPUVendor identifies the primary GPU vendor from system info.
29		// Uses /sys/class/drm/card*/device/vendor as primary detection method.	29		// Uses /sys/class/drm/card*/device/vendor as primary detection method.

M internal/infrastructure/webkit/handlers/homepage/folders.go +1 -1


@@ -114,7 +114,7 @@ type updateFolderRequest struct {
114	114		}
115	115
116	116		// HandleUpdate handles folder_update messages.
117		-	// NOTE: This requires UpdateFolder() method to be added to ManageFavoritesUseCase.
	117	+	// It delegates to the port.HomepageFavorites interface's UpdateFolder method.
118	118		func (h *FolderHandlers) HandleUpdate() webkit.MessageHandler {
119	119		return webkit.MessageHandlerFunc(func(ctx context.Context, _ webkit.WebViewID, payload json.RawMessage) (any, error) {
120	120		log := logging.FromContext(ctx)


@@ -114,7 +114,7 @@ type updateFolderRequest struct {
114		}	114		}
115			115
116		// HandleUpdate handles folder_update messages.	116		// HandleUpdate handles folder_update messages.
117	-	// NOTE: This requires UpdateFolder() method to be added to ManageFavoritesUseCase.	117	+	// It delegates to the port.HomepageFavorites interface's UpdateFolder method.
118		func (h *FolderHandlers) HandleUpdate() webkit.MessageHandler {	118		func (h *FolderHandlers) HandleUpdate() webkit.MessageHandler {
119		return webkit.MessageHandlerFunc(func(ctx context.Context, _ webkit.WebViewID, payload json.RawMessage) (any, error) {	119		return webkit.MessageHandlerFunc(func(ctx context.Context, _ webkit.WebViewID, payload json.RawMessage) (any, error) {
120		log := logging.FromContext(ctx)	120		log := logging.FromContext(ctx)

M internal/infrastructure/webkit/settings.go +1 -8


@@ -175,14 +175,7 @@ func (sm SettingsManager) ApplyToWebView(ctx context.Context, wv webkit.WebVie
175	175		return
176	176		}
177	177
178		-	settings := sm.CreateSettings(ctx)
179		-	if settings == nil {
180		-	return
181		-	}
182		-
183		-	// WebView.GetSettings() returns the current settings - we can modify them directly
184		-	// For a full replacement, we'd need webkit_web_view_set_settings which may not be available
185		-	// Instead, we apply to the existing settings object
	178	+	// Apply to the existing settings object attached to the WebView
186	179		existingSettings := wv.GetSettings()
187	180		if existingSettings != nil {
188	181		sm.mu.RLock()

 		return
 	}
-	settings := sm.CreateSettings(ctx)
+	// Apply to the existing settings object attached to the WebView
-	if settings == nil {
-		return
-	}
-	// WebView.GetSettings() returns the current settings - we can modify them directly
-	// For a full replacement, we'd need webkit_web_view_set_settings which may not be available
-	// Instead, we apply to the existing settings object
 	existingSettings := wv.GetSettings()
 	if existingSettings != nil {
 		sm.mu.RLock()

M internal/infrastructure/webkit/webview.go +10 -0


@@ -1607,6 +1607,10 @@ func (wv WebView) SetCallbacks(callbacks port.WebViewCallbacks) {
1607	1607		wv.OnLinkHover = nil
1608	1608		wv.OnWebProcessTerminated = nil
1609	1609		wv.OnPermissionRequest = nil
	1610	+	wv.OnLinkMiddleClick = nil
	1611	+	wv.OnEnterFullscreen = nil
	1612	+	wv.OnLeaveFullscreen = nil
	1613	+	wv.OnAudioStateChanged = nil
1610	1614		return
1611	1615		}
1612	1616

@@ -1733,6 +1737,9 @@ func (wv *WebView) Close() {
1733	1737		// It composes fn with any existing OnClose handler so multiple callers can register
1734	1738		// close callbacks without overwriting one another.
1735	1739		func (wv *WebView) AddCloseCallback(fn func()) {
	1740	+	if fn == nil {
	1741	+	return
	1742	+	}
1736	1743		if wv.OnClose == nil {
1737	1744		wv.OnClose = fn
1738	1745		return

@@ -1748,6 +1755,9 @@ func (wv *WebView) AddCloseCallback(fn func()) {
1748	1755		// fn is invoked when the URI changes (OnURIChanged) and when a page load is committed
1749	1756		// (OnLoadChanged with LoadCommitted), covering both redirect-based and postMessage OAuth flows.
1750	1757		func (wv *WebView) AddNavigationCallback(fn func(uri string)) {
	1758	+	if fn == nil {
	1759	+	return
	1760	+	}
1751	1761		// Compose with existing OnURIChanged.
1752	1762		if wv.OnURIChanged == nil {
1753	1763		wv.OnURIChanged = fn


@@ -1607,6 +1607,10 @@ func (wv WebView) SetCallbacks(callbacks port.WebViewCallbacks) {
1607	wv.OnLinkHover = nil	1607		wv.OnLinkHover = nil
1608	wv.OnWebProcessTerminated = nil	1608		wv.OnWebProcessTerminated = nil
1609	wv.OnPermissionRequest = nil	1609		wv.OnPermissionRequest = nil
		1610	+	wv.OnLinkMiddleClick = nil
		1611	+	wv.OnEnterFullscreen = nil
		1612	+	wv.OnLeaveFullscreen = nil
		1613	+	wv.OnAudioStateChanged = nil
1610	return	1614		return
1611	}	1615		}
1612		1616

@@ -1733,6 +1737,9 @@ func (wv *WebView) Close() {
1733	// It composes fn with any existing OnClose handler so multiple callers can register	1737		// It composes fn with any existing OnClose handler so multiple callers can register
1734	// close callbacks without overwriting one another.	1738		// close callbacks without overwriting one another.
1735	func (wv *WebView) AddCloseCallback(fn func()) {	1739		func (wv *WebView) AddCloseCallback(fn func()) {
		1740	+	if fn == nil {
		1741	+	return
		1742	+	}
1736	if wv.OnClose == nil {	1743		if wv.OnClose == nil {
1737	wv.OnClose = fn	1744		wv.OnClose = fn
1738	return	1745		return

@@ -1748,6 +1755,9 @@ func (wv *WebView) AddCloseCallback(fn func()) {
1748	// fn is invoked when the URI changes (OnURIChanged) and when a page load is committed	1755		// fn is invoked when the URI changes (OnURIChanged) and when a page load is committed
1749	// (OnLoadChanged with LoadCommitted), covering both redirect-based and postMessage OAuth flows.	1756		// (OnLoadChanged with LoadCommitted), covering both redirect-based and postMessage OAuth flows.
1750	func (wv *WebView) AddNavigationCallback(fn func(uri string)) {	1757		func (wv *WebView) AddNavigationCallback(fn func(uri string)) {
		1758	+	if fn == nil {
		1759	+	return
		1760	+	}
1751	// Compose with existing OnURIChanged.	1761		// Compose with existing OnURIChanged.
1752	if wv.OnURIChanged == nil {	1762		if wv.OnURIChanged == nil {
1753	wv.OnURIChanged = fn	1763		wv.OnURIChanged = fn

M internal/ui/app.go +3 -1


@@ -3111,7 +3111,9 @@ func (a *App) applyAppearanceConfig(ctx context.Context) {
3111	3111		cfg := a.deps.Config
3112	3112
3113	3113		if a.engine != nil {
3114		-	_ = a.engine.UpdateSettings(ctx, port.EngineSettingsUpdate{Raw: cfg})
	3114	+	if err := a.engine.UpdateSettings(ctx, port.EngineSettingsUpdate{Raw: cfg}); err != nil {
	3115	+	log.Warn().Err(err).Msg("failed to apply engine settings update")
	3116	+	}
3115	3117		}
3116	3118
3117	3119		// Apply settings to existing webviews via coordinator


@@ -3111,7 +3111,9 @@ func (a *App) applyAppearanceConfig(ctx context.Context) {
3111		cfg := a.deps.Config	3111		cfg := a.deps.Config
3112			3112
3113		if a.engine != nil {	3113		if a.engine != nil {
3114	-	_ = a.engine.UpdateSettings(ctx, port.EngineSettingsUpdate{Raw: cfg})	3114	+	if err := a.engine.UpdateSettings(ctx, port.EngineSettingsUpdate{Raw: cfg}); err != nil {
			3115	+	log.Warn().Err(err).Msg("failed to apply engine settings update")
			3116	+	}
3115		}	3117		}
3116			3118
3117		// Apply settings to existing webviews via coordinator	3119		// Apply settings to existing webviews via coordinator

M internal/ui/coordinator/content/appearance.go +1 -1


@@ -108,7 +108,7 @@ func buildWebUIThemeScript(prefersDark bool, cssText string) (string, error) {
108	108		if (!meta) {
109	109		meta = document.createElement('meta');
110	110		meta.name = 'color-scheme';
111		-	document.documentElement.appendChild(meta);
	111	+	(document.head \|\| document.documentElement).appendChild(meta);
112	112		}
113	113		meta.content = prefersDark ? 'dark light' : 'light dark';
114	114		} catch (e) {


@@ -108,7 +108,7 @@ func buildWebUIThemeScript(prefersDark bool, cssText string) (string, error) {
108		if (!meta) {	108		if (!meta) {
109		meta = document.createElement('meta');	109		meta = document.createElement('meta');
110		meta.name = 'color-scheme';	110		meta.name = 'color-scheme';
111	-	document.documentElement.appendChild(meta);	111	+	(document.head \|\| document.documentElement).appendChild(meta);
112		}	112		}
113		meta.content = prefersDark ? 'dark light' : 'light dark';	113		meta.content = prefersDark ? 'dark light' : 'light dark';
114		} catch (e) {	114		} catch (e) {

M internal/ui/coordinator/content/oauth.go +2 -3


@@ -114,8 +114,7 @@ func ShouldAutoClose(url string) bool {
114	114		//
115	115		// Safety-timeout force close is disabled for stability. OAuth popups close on
116	116		// callback detection or manual user action.
117		-	func shouldForceCloseOnSafetyTimeout(url string) bool {
118		-	_ = url
	117	+	func shouldForceCloseOnSafetyTimeout() bool {
119	118		return false
120	119		}
121	120

@@ -256,7 +255,7 @@ func (c *Coordinator) setupOAuthAutoClose(
256	255		cb := glib.SourceFunc(func(_ uintptr) bool {
257	256		if wv != nil && !wv.IsDestroyed() {
258	257		uri := wv.URI()
259		-	if shouldForceCloseOnSafetyTimeout(uri) {
	258	+	if shouldForceCloseOnSafetyTimeout() {
260	259		log.Warn().Str("pane", string(paneID)).Msg("oauth safety timeout, closing stuck popup")
261	260		oauthWV.Close()
262	261		return false

 //
 // Safety-timeout force close is disabled for stability. OAuth popups close on
 // callback detection or manual user action.
-func shouldForceCloseOnSafetyTimeout(url string) bool {
+func shouldForceCloseOnSafetyTimeout() bool {
-	_ = url
 	return false
 }
 			cb := glib.SourceFunc(func(_ uintptr) bool {
 				if wv != nil && !wv.IsDestroyed() {
 					uri := wv.URI()
-					if shouldForceCloseOnSafetyTimeout(uri) {
+					if shouldForceCloseOnSafetyTimeout() {
 						log.Warn().Str("pane", string(paneID)).Msg("oauth safety timeout, closing stuck popup")
 						oauthWV.Close()
 						return false

M internal/ui/coordinator/content/oauth_test.go +2 -4


@@ -109,13 +109,11 @@ func TestIsOAuthURL_DetectsOAuthAuthorizeRequest(t *testing.T) {
109	109
110	110		func TestShouldForceCloseOnSafetyTimeout(t *testing.T) {
111	111		t.Run("does not force close during in-progress auth challenge", func(t *testing.T) {
112		-	url := "https://accounts.google.com/v3/signin/challenge/dp?TL=AHU8sQabc"
113		-	assert.False(t, shouldForceCloseOnSafetyTimeout(url))
	112	+	assert.False(t, shouldForceCloseOnSafetyTimeout())
114	113		})
115	114
116	115		t.Run("does not force close when callback is reached", func(t *testing.T) {
117		-	url := "http://localhost:1455/auth/callback?code=abc123&state=xyz"
118		-	assert.False(t, shouldForceCloseOnSafetyTimeout(url))
	116	+	assert.False(t, shouldForceCloseOnSafetyTimeout())
119	117		})
120	118		}
121	119

 func TestShouldForceCloseOnSafetyTimeout(t *testing.T) {
 	t.Run("does not force close during in-progress auth challenge", func(t *testing.T) {
-		url := "https://accounts.google.com/v3/signin/challenge/dp?TL=AHU8sQabc"
+		assert.False(t, shouldForceCloseOnSafetyTimeout())
-		assert.False(t, shouldForceCloseOnSafetyTimeout(url))
 	})
 	t.Run("does not force close when callback is reached", func(t *testing.T) {
-		url := "http://localhost:1455/auth/callback?code=abc123&state=xyz"
+		assert.False(t, shouldForceCloseOnSafetyTimeout())
-		assert.False(t, shouldForceCloseOnSafetyTimeout(url))
 	})
 }

M internal/ui/coordinator/content/popup_test.go +17 -18


@@ -9,7 +9,6 @@ import (
9	9
10	10		"github.com/bnema/dumber/internal/application/port"
11	11		"github.com/bnema/dumber/internal/domain/entity"
12		-	"github.com/bnema/dumber/internal/infrastructure/config"
13	12		)
14	13
15	14		// ---------------------------------------------------------------------------

@@ -63,52 +62,52 @@ func TestPopupTypeString_Unknown(t *testing.T) {
63	62		func TestGetBehavior_NilConfigReturnsSplit(t *testing.T) {
64	63		t.Parallel()
65	64
66		-	assert.Equal(t, config.PopupBehaviorSplit, GetBehavior(PopupTypeTab, nil))
67		-	assert.Equal(t, config.PopupBehaviorSplit, GetBehavior(PopupTypePopup, nil))
	65	+	assert.Equal(t, entity.PopupBehaviorSplit, GetBehavior(PopupTypeTab, nil))
	66	+	assert.Equal(t, entity.PopupBehaviorSplit, GetBehavior(PopupTypePopup, nil))
68	67		}
69	68
70	69		func TestGetBehavior_TabPopup_DefaultConfig(t *testing.T) {
71	70		t.Parallel()
72	71
73	72		// BlankTargetBehavior is empty → falls through to default "stacked"
74		-	cfg := &config.PopupBehaviorConfig{}
75		-	assert.Equal(t, config.PopupBehaviorStacked, GetBehavior(PopupTypeTab, cfg))
	73	+	cfg := &entity.PopupBehaviorConfig{}
	74	+	assert.Equal(t, entity.PopupBehaviorStacked, GetBehavior(PopupTypeTab, cfg))
76	75		}
77	76
78	77		func TestGetBehavior_TabPopup_SplitConfig(t *testing.T) {
79	78		t.Parallel()
80	79
81		-	cfg := &config.PopupBehaviorConfig{BlankTargetBehavior: "split"}
82		-	assert.Equal(t, config.PopupBehaviorSplit, GetBehavior(PopupTypeTab, cfg))
	80	+	cfg := &entity.PopupBehaviorConfig{BlankTargetBehavior: "split"}
	81	+	assert.Equal(t, entity.PopupBehaviorSplit, GetBehavior(PopupTypeTab, cfg))
83	82		}
84	83
85	84		func TestGetBehavior_TabPopup_StackedConfig(t *testing.T) {
86	85		t.Parallel()
87	86
88		-	cfg := &config.PopupBehaviorConfig{BlankTargetBehavior: "stacked"}
89		-	assert.Equal(t, config.PopupBehaviorStacked, GetBehavior(PopupTypeTab, cfg))
	87	+	cfg := &entity.PopupBehaviorConfig{BlankTargetBehavior: "stacked"}
	88	+	assert.Equal(t, entity.PopupBehaviorStacked, GetBehavior(PopupTypeTab, cfg))
90	89		}
91	90
92	91		func TestGetBehavior_TabPopup_TabbedConfig(t *testing.T) {
93	92		t.Parallel()
94	93
95		-	cfg := &config.PopupBehaviorConfig{BlankTargetBehavior: "tabbed"}
96		-	assert.Equal(t, config.PopupBehaviorTabbed, GetBehavior(PopupTypeTab, cfg))
	94	+	cfg := &entity.PopupBehaviorConfig{BlankTargetBehavior: "tabbed"}
	95	+	assert.Equal(t, entity.PopupBehaviorTabbed, GetBehavior(PopupTypeTab, cfg))
97	96		}
98	97
99	98		func TestGetBehavior_JSPopup_UsesBehaviorField(t *testing.T) {
100	99		t.Parallel()
101	100
102		-	cfg := &config.PopupBehaviorConfig{Behavior: config.PopupBehaviorWindowed}
103		-	assert.Equal(t, config.PopupBehaviorWindowed, GetBehavior(PopupTypePopup, cfg))
	101	+	cfg := &entity.PopupBehaviorConfig{Behavior: entity.PopupBehaviorWindowed}
	102	+	assert.Equal(t, entity.PopupBehaviorWindowed, GetBehavior(PopupTypePopup, cfg))
104	103		}
105	104
106	105		func TestGetBehavior_JSPopup_DefaultConfig(t *testing.T) {
107	106		t.Parallel()
108	107
109	108		// Behavior zero-value is empty string; GetBehavior returns it as-is.
110		-	cfg := &config.PopupBehaviorConfig{}
111		-	assert.Equal(t, config.PopupBehavior(""), GetBehavior(PopupTypePopup, cfg))
	109	+	cfg := &entity.PopupBehaviorConfig{}
	110	+	assert.Equal(t, entity.PopupBehavior(""), GetBehavior(PopupTypePopup, cfg))
112	111		}
113	112
114	113		func TestGetBehavior_TabPopup_BlocksWhenOpenInNewPaneFalse(t *testing.T) {

@@ -116,11 +115,11 @@ func TestGetBehavior_TabPopup_BlocksWhenOpenInNewPaneFalse(t *testing.T) {
116	115
117	116		// GetBehavior itself does not honor OpenInNewPane — that is enforced by
118	117		// handlePopupCreate. GetBehavior still returns the configured value.
119		-	cfg := &config.PopupBehaviorConfig{
	118	+	cfg := &entity.PopupBehaviorConfig{
120	119		OpenInNewPane: false,
121	120		BlankTargetBehavior: "split",
122	121		}
123		-	assert.Equal(t, config.PopupBehaviorSplit, GetBehavior(PopupTypeTab, cfg))
	122	+	assert.Equal(t, entity.PopupBehaviorSplit, GetBehavior(PopupTypeTab, cfg))
124	123		}
125	124
126	125		// ---------------------------------------------------------------------------

@@ -220,7 +219,7 @@ func TestSetPopupConfig_SetsFields(t *testing.T) {
220	219		c := &Coordinator{}
221	220
222	221		factory := &stubFactory{}
223		-	cfg := &config.PopupBehaviorConfig{Behavior: config.PopupBehaviorSplit}
	222	+	cfg := &entity.PopupBehaviorConfig{Behavior: entity.PopupBehaviorSplit}
224	223		genID := func() string { return "test-id" }
225	224
226	225		c.SetPopupConfig(factory, cfg, genID)

 	"github.com/bnema/dumber/internal/application/port"
 	"github.com/bnema/dumber/internal/domain/entity"
-	"github.com/bnema/dumber/internal/infrastructure/config"
 )
 // ---------------------------------------------------------------------------
 func TestGetBehavior_NilConfigReturnsSplit(t *testing.T) {
 	t.Parallel()
-	assert.Equal(t, config.PopupBehaviorSplit, GetBehavior(PopupTypeTab, nil))
+	assert.Equal(t, entity.PopupBehaviorSplit, GetBehavior(PopupTypeTab, nil))
-	assert.Equal(t, config.PopupBehaviorSplit, GetBehavior(PopupTypePopup, nil))
+	assert.Equal(t, entity.PopupBehaviorSplit, GetBehavior(PopupTypePopup, nil))
 }
 func TestGetBehavior_TabPopup_DefaultConfig(t *testing.T) {
 	t.Parallel()
 	// BlankTargetBehavior is empty → falls through to default "stacked"
-	cfg := &config.PopupBehaviorConfig{}
+	cfg := &entity.PopupBehaviorConfig{}
-	assert.Equal(t, config.PopupBehaviorStacked, GetBehavior(PopupTypeTab, cfg))
+	assert.Equal(t, entity.PopupBehaviorStacked, GetBehavior(PopupTypeTab, cfg))
 }
 func TestGetBehavior_TabPopup_SplitConfig(t *testing.T) {
 	t.Parallel()
-	cfg := &config.PopupBehaviorConfig{BlankTargetBehavior: "split"}
+	cfg := &entity.PopupBehaviorConfig{BlankTargetBehavior: "split"}
-	assert.Equal(t, config.PopupBehaviorSplit, GetBehavior(PopupTypeTab, cfg))
+	assert.Equal(t, entity.PopupBehaviorSplit, GetBehavior(PopupTypeTab, cfg))
 }
 func TestGetBehavior_TabPopup_StackedConfig(t *testing.T) {
 	t.Parallel()
-	cfg := &config.PopupBehaviorConfig{BlankTargetBehavior: "stacked"}
+	cfg := &entity.PopupBehaviorConfig{BlankTargetBehavior: "stacked"}
-	assert.Equal(t, config.PopupBehaviorStacked, GetBehavior(PopupTypeTab, cfg))
+	assert.Equal(t, entity.PopupBehaviorStacked, GetBehavior(PopupTypeTab, cfg))
 }
 func TestGetBehavior_TabPopup_TabbedConfig(t *testing.T) {
 	t.Parallel()
-	cfg := &config.PopupBehaviorConfig{BlankTargetBehavior: "tabbed"}
+	cfg := &entity.PopupBehaviorConfig{BlankTargetBehavior: "tabbed"}
-	assert.Equal(t, config.PopupBehaviorTabbed, GetBehavior(PopupTypeTab, cfg))
+	assert.Equal(t, entity.PopupBehaviorTabbed, GetBehavior(PopupTypeTab, cfg))
 }
 func TestGetBehavior_JSPopup_UsesBehaviorField(t *testing.T) {
 	t.Parallel()
-	cfg := &config.PopupBehaviorConfig{Behavior: config.PopupBehaviorWindowed}
+	cfg := &entity.PopupBehaviorConfig{Behavior: entity.PopupBehaviorWindowed}
-	assert.Equal(t, config.PopupBehaviorWindowed, GetBehavior(PopupTypePopup, cfg))
+	assert.Equal(t, entity.PopupBehaviorWindowed, GetBehavior(PopupTypePopup, cfg))
 }
 func TestGetBehavior_JSPopup_DefaultConfig(t *testing.T) {
 	t.Parallel()
 	// Behavior zero-value is empty string; GetBehavior returns it as-is.
-	cfg := &config.PopupBehaviorConfig{}
+	cfg := &entity.PopupBehaviorConfig{}
-	assert.Equal(t, config.PopupBehavior(""), GetBehavior(PopupTypePopup, cfg))
+	assert.Equal(t, entity.PopupBehavior(""), GetBehavior(PopupTypePopup, cfg))
 }
 func TestGetBehavior_TabPopup_BlocksWhenOpenInNewPaneFalse(t *testing.T) {
 	// GetBehavior itself does not honor OpenInNewPane — that is enforced by
 	// handlePopupCreate. GetBehavior still returns the configured value.
-	cfg := &config.PopupBehaviorConfig{
+	cfg := &entity.PopupBehaviorConfig{
 		OpenInNewPane:       false,
 		BlankTargetBehavior: "split",
 	}
-	assert.Equal(t, config.PopupBehaviorSplit, GetBehavior(PopupTypeTab, cfg))
+	assert.Equal(t, entity.PopupBehaviorSplit, GetBehavior(PopupTypeTab, cfg))
 }
 // ---------------------------------------------------------------------------
 	c := &Coordinator{}
 	factory := &stubFactory{}
-	cfg := &config.PopupBehaviorConfig{Behavior: config.PopupBehaviorSplit}
+	cfg := &entity.PopupBehaviorConfig{Behavior: entity.PopupBehaviorSplit}
 	genID := func() string { return "test-id" }
 	c.SetPopupConfig(factory, cfg, genID)

M internal/ui/coordinator/update.go +1 -1


@@ -45,7 +45,7 @@ func NewUpdateCoordinator(
45	45		}
46	46		}
47	47
48		-	// CheckOnStartup triggers an update check if enabled by config.
	48	+	// CheckOnStartup triggers an update check if enabled via the enableOnStartup constructor flag.
49	49		func (c *UpdateCoordinator) CheckOnStartup(ctx context.Context) {
50	50		log := logging.FromContext(ctx)
51	51


@@ -45,7 +45,7 @@ func NewUpdateCoordinator(
45		}	45		}
46		}	46		}
47			47
48	-	// CheckOnStartup triggers an update check if enabled by config.	48	+	// CheckOnStartup triggers an update check if enabled via the enableOnStartup constructor flag.
49		func (c *UpdateCoordinator) CheckOnStartup(ctx context.Context) {	49		func (c *UpdateCoordinator) CheckOnStartup(ctx context.Context) {
50		log := logging.FromContext(ctx)	50		log := logging.FromContext(ctx)
51			51

M internal/ui/coordinator/workspace.go +4 -0


@@ -1862,6 +1862,8 @@ func (c *WorkspaceCoordinator) insertPopupSplit(ctx context.Context, input conte
1862	1862		c.attachPopupWebView(ctx, wsView, input)
1863	1863		}
1864	1864
	1865	+	c.notifyStateChanged()
	1866	+
1865	1867		log.Info().
1866	1868		Str("popup_pane", string(input.PopupPane.ID)).
1867	1869		Str("direction", string(direction)).

@@ -1977,6 +1979,8 @@ func (c *WorkspaceCoordinator) insertPopupStacked(ctx context.Context, input con
1977	1979		return err
1978	1980		}
1979	1981
	1982	+	c.notifyStateChanged()
	1983	+
1980	1984		log.Info().
1981	1985		Str("popup_pane", string(input.PopupPane.ID)).
1982	1986		Int("stack_size", len(stackNode.Children)).


@@ -1862,6 +1862,8 @@ func (c *WorkspaceCoordinator) insertPopupSplit(ctx context.Context, input conte
1862	c.attachPopupWebView(ctx, wsView, input)	1862		c.attachPopupWebView(ctx, wsView, input)
1863	}	1863		}
1864		1864
		1865	+	c.notifyStateChanged()
		1866	+
1865	log.Info().	1867		log.Info().
1866	Str("popup_pane", string(input.PopupPane.ID)).	1868		Str("popup_pane", string(input.PopupPane.ID)).
1867	Str("direction", string(direction)).	1869		Str("direction", string(direction)).

@@ -1977,6 +1979,8 @@ func (c *WorkspaceCoordinator) insertPopupStacked(ctx context.Context, input con
1977	return err	1979		return err
1978	}	1980		}
1979		1981
		1982	+	c.notifyStateChanged()
		1983	+
1980	log.Info().	1984		log.Info().
1981	Str("popup_pane", string(input.PopupPane.ID)).	1985		Str("popup_pane", string(input.PopupPane.ID)).
1982	Int("stack_size", len(stackNode.Children)).	1986		Int("stack_size", len(stackNode.Children)).

M internal/ui/dialog/permission.go +86 -59


@@ -4,6 +4,7 @@ package dialog
4	4		import (
5	5		"context"
6	6		"fmt"
	7	+	"strings"
7	8		"sync"
8	9
9	10		"github.com/bnema/dumber/internal/application/port"

@@ -125,88 +126,114 @@ func (d *PermissionDialog) showNextQueuedRequest() {
125	126		d.showRequest(next)
126	127		}
127	128
128		-	// buildHeading creates the dialog heading based on permission types.
129		-	func (d *PermissionDialog) buildHeading(permTypes []entity.PermissionType) string {
130		-	hasMic := false
131		-	hasCam := false
132		-	hasDisplay := false
133		-	hasDataAccess := false
	129	+	// permFlags holds parsed permission type flags.
	130	+	type permFlags struct {
	131	+	mic, cam, display, dataAccess bool
	132	+	}
134	133
	134	+	// parsePermFlags extracts boolean flags from permission types.
	135	+	func parsePermFlags(permTypes []entity.PermissionType) permFlags {
	136	+	var f permFlags
135	137		for _, pt := range permTypes {
136	138		switch pt {
137	139		case entity.PermissionTypeMicrophone:
138		-	hasMic = true
	140	+	f.mic = true
139	141		case entity.PermissionTypeCamera:
140		-	hasCam = true
	142	+	f.cam = true
141	143		case entity.PermissionTypeDisplay:
142		-	hasDisplay = true
	144	+	f.display = true
143	145		case entity.PermissionTypeWebsiteDataAccess:
144		-	hasDataAccess = true
	146	+	f.dataAccess = true
145	147		}
146	148		}
	149	+	return f
	150	+	}
	151	+
	152	+	// joinPermissionLabels joins labels with commas and "and".
	153	+	func joinPermissionLabels(labels []string) string {
	154	+	switch len(labels) {
	155	+	case 0:
	156	+	return ""
	157	+	case 1:
	158	+	return labels[0]
	159	+	case 2:
	160	+	return labels[0] + " and " + labels[1]
	161	+	default:
	162	+	return strings.Join(labels[:len(labels)-1], ", ") +
	163	+	", and " + labels[len(labels)-1]
	164	+	}
	165	+	}
147	166
	167	+	// buildHeading creates the dialog heading based on permission types.
	168	+	func (d *PermissionDialog) buildHeading(
	169	+	permTypes []entity.PermissionType,
	170	+	) string {
	171	+	f := parsePermFlags(permTypes)
	172	+	var labels []string
	173	+	if f.mic {
	174	+	labels = append(labels, "Microphone")
	175	+	}
	176	+	if f.cam {
	177	+	labels = append(labels, "Camera")
	178	+	}
	179	+	if f.display {
	180	+	labels = append(labels, "Screen Sharing")
	181	+	}
	182	+	if f.dataAccess {
	183	+	labels = append(labels, "Data Access")
	184	+	}
148	185		switch {
149		-	case hasMic && hasCam && hasDisplay:
150		-	return "Allow Microphone, Camera, and Screen Sharing?"
151		-	case hasMic && hasDisplay:
152		-	return "Allow Microphone and Screen Sharing?"
153		-	case hasCam && hasDisplay:
154		-	return "Allow Camera and Screen Sharing?"
155		-	case hasMic && hasCam:
156		-	return "Allow Microphone and Camera?"
157		-	case hasMic:
158		-	return "Allow Microphone Access?"
159		-	case hasCam:
160		-	return "Allow Camera Access?"
161		-	case hasDisplay:
162		-	return "Allow Screen Sharing?"
163		-	case hasDataAccess:
	186	+	case len(labels) == 0:
	187	+	return "Allow Permission?"
	188	+	case len(labels) == 1 && f.dataAccess:
164	189		return "Allow Third-Party Data Access?"
	190	+	case len(labels) == 1 && f.display:
	191	+	return "Allow Screen Sharing?"
	192	+	case len(labels) == 1:
	193	+	return "Allow " + labels[0] + " Access?"
165	194		default:
166		-	return "Allow Permission?"
	195	+	return "Allow " + joinPermissionLabels(labels) + "?"
167	196		}
168	197		}
169	198
170	199		// buildBody creates the dialog body text.
171		-	func (d *PermissionDialog) buildBody(origin string, permTypes []entity.PermissionType) string {
172		-	hasMic := false
173		-	hasCam := false
174		-	hasDisplay := false
175		-	hasDataAccess := false
176		-
177		-	for _, pt := range permTypes {
178		-	switch pt {
179		-	case entity.PermissionTypeMicrophone:
180		-	hasMic = true
181		-	case entity.PermissionTypeCamera:
182		-	hasCam = true
183		-	case entity.PermissionTypeDisplay:
184		-	hasDisplay = true
185		-	case entity.PermissionTypeWebsiteDataAccess:
186		-	hasDataAccess = true
	200	+	func (d *PermissionDialog) buildBody(
	201	+	origin string, permTypes []entity.PermissionType,
	202	+	) string {
	203	+	f := parsePermFlags(permTypes)
	204	+	var parts []string
	205	+	if f.mic && f.cam {
	206	+	parts = append(parts, "access your microphone and camera")
	207	+	} else if f.mic {
	208	+	parts = append(parts, "access your microphone")
	209	+	} else if f.cam {
	210	+	parts = append(parts, "access your camera")
	211	+	}
	212	+	if f.display {
	213	+	parts = append(parts, "share your screen")
	214	+	}
	215	+	if f.dataAccess {
	216	+	if len(parts) == 0 {
	217	+	parts = append(parts,
	218	+	"access its stored data while you browse this site")
	219	+	} else {
	220	+	parts = append(parts, "access its stored data")
187	221		}
188	222		}
189	223
190	224		var action string
191	225		switch {
192		-	case hasMic && hasCam && hasDisplay:
193		-	action = "access your microphone and camera, and share your screen"
194		-	case hasMic && hasDisplay:
195		-	action = "access your microphone and share your screen"
196		-	case hasCam && hasDisplay:
197		-	action = "access your camera and share your screen"
198		-	case hasMic && hasCam:
199		-	action = "access your microphone and camera"
200		-	case hasMic:
201		-	action = "access your microphone"
202		-	case hasCam:
203		-	action = "access your camera"
204		-	case hasDisplay:
205		-	action = "share your screen"
206		-	case hasDataAccess:
207		-	action = "access its stored data while you browse this site"
208		-	default:
	226	+	case len(parts) == 0:
209	227		action = "access your device"
	228	+	case len(parts) == 1:
	229	+	action = parts[0]
	230	+	case f.mic && f.cam:
	231	+	// When mic+cam are combined into one phrase containing "and",
	232	+	// use ", and" before the next part for clarity.
	233	+	action = strings.Join(parts[:len(parts)-1], ", ") +
	234	+	", and " + parts[len(parts)-1]
	235	+	default:
	236	+	action = parts[0] + " and " + parts[1]
210	237		}
211	238
212	239		return fmt.Sprintf("%s wants to %s.", origin, action)


@@ -4,6 +4,7 @@ package dialog
4		import (	4		import (
5		"context"	5		"context"
6		"fmt"	6		"fmt"
			7	+	"strings"
7		"sync"	8		"sync"
8			9
9		"github.com/bnema/dumber/internal/application/port"	10		"github.com/bnema/dumber/internal/application/port"

@@ -125,88 +126,114 @@ func (d *PermissionDialog) showNextQueuedRequest() {
125		d.showRequest(next)	126		d.showRequest(next)
126		}	127		}
127			128
128	-	// buildHeading creates the dialog heading based on permission types.	129	+	// permFlags holds parsed permission type flags.
129	-	func (d *PermissionDialog) buildHeading(permTypes []entity.PermissionType) string {	130	+	type permFlags struct {
130	-	hasMic := false	131	+	mic, cam, display, dataAccess bool
131	-	hasCam := false	132	+	}
132	-	hasDisplay := false
133	-	hasDataAccess := false
134			133
			134	+	// parsePermFlags extracts boolean flags from permission types.
			135	+	func parsePermFlags(permTypes []entity.PermissionType) permFlags {
			136	+	var f permFlags
135		for _, pt := range permTypes {	137		for _, pt := range permTypes {
136		switch pt {	138		switch pt {
137		case entity.PermissionTypeMicrophone:	139		case entity.PermissionTypeMicrophone:
138	-	hasMic = true	140	+	f.mic = true
139		case entity.PermissionTypeCamera:	141		case entity.PermissionTypeCamera:
140	-	hasCam = true	142	+	f.cam = true
141		case entity.PermissionTypeDisplay:	143		case entity.PermissionTypeDisplay:
142	-	hasDisplay = true	144	+	f.display = true
143		case entity.PermissionTypeWebsiteDataAccess:	145		case entity.PermissionTypeWebsiteDataAccess:
144	-	hasDataAccess = true	146	+	f.dataAccess = true
145		}	147		}
146		}	148		}
			149	+	return f
			150	+	}
			151	+
			152	+	// joinPermissionLabels joins labels with commas and "and".
			153	+	func joinPermissionLabels(labels []string) string {
			154	+	switch len(labels) {
			155	+	case 0:
			156	+	return ""
			157	+	case 1:
			158	+	return labels[0]
			159	+	case 2:
			160	+	return labels[0] + " and " + labels[1]
			161	+	default:
			162	+	return strings.Join(labels[:len(labels)-1], ", ") +
			163	+	", and " + labels[len(labels)-1]
			164	+	}
			165	+	}
147			166
			167	+	// buildHeading creates the dialog heading based on permission types.
			168	+	func (d *PermissionDialog) buildHeading(
			169	+	permTypes []entity.PermissionType,
			170	+	) string {
			171	+	f := parsePermFlags(permTypes)
			172	+	var labels []string
			173	+	if f.mic {
			174	+	labels = append(labels, "Microphone")
			175	+	}
			176	+	if f.cam {
			177	+	labels = append(labels, "Camera")
			178	+	}
			179	+	if f.display {
			180	+	labels = append(labels, "Screen Sharing")
			181	+	}
			182	+	if f.dataAccess {
			183	+	labels = append(labels, "Data Access")
			184	+	}
148		switch {	185		switch {
149	-	case hasMic && hasCam && hasDisplay:	186	+	case len(labels) == 0:
150	-	return "Allow Microphone, Camera, and Screen Sharing?"	187	+	return "Allow Permission?"
151	-	case hasMic && hasDisplay:	188	+	case len(labels) == 1 && f.dataAccess:
152	-	return "Allow Microphone and Screen Sharing?"
153	-	case hasCam && hasDisplay:
154	-	return "Allow Camera and Screen Sharing?"
155	-	case hasMic && hasCam:
156	-	return "Allow Microphone and Camera?"
157	-	case hasMic:
158	-	return "Allow Microphone Access?"
159	-	case hasCam:
160	-	return "Allow Camera Access?"
161	-	case hasDisplay:
162	-	return "Allow Screen Sharing?"
163	-	case hasDataAccess:
164		return "Allow Third-Party Data Access?"	189		return "Allow Third-Party Data Access?"
			190	+	case len(labels) == 1 && f.display:
			191	+	return "Allow Screen Sharing?"
			192	+	case len(labels) == 1:
			193	+	return "Allow " + labels[0] + " Access?"
165		default:	194		default:
166	-	return "Allow Permission?"	195	+	return "Allow " + joinPermissionLabels(labels) + "?"
167		}	196		}
168		}	197		}
169			198
170		// buildBody creates the dialog body text.	199		// buildBody creates the dialog body text.
171	-	func (d *PermissionDialog) buildBody(origin string, permTypes []entity.PermissionType) string {	200	+	func (d *PermissionDialog) buildBody(
172	-	hasMic := false	201	+	origin string, permTypes []entity.PermissionType,
173	-	hasCam := false	202	+	) string {
174	-	hasDisplay := false	203	+	f := parsePermFlags(permTypes)
175	-	hasDataAccess := false	204	+	var parts []string
176	-		205	+	if f.mic && f.cam {
177	-	for _, pt := range permTypes {	206	+	parts = append(parts, "access your microphone and camera")
178	-	switch pt {	207	+	} else if f.mic {
179	-	case entity.PermissionTypeMicrophone:	208	+	parts = append(parts, "access your microphone")
180	-	hasMic = true	209	+	} else if f.cam {
181	-	case entity.PermissionTypeCamera:	210	+	parts = append(parts, "access your camera")
182	-	hasCam = true	211	+	}
183	-	case entity.PermissionTypeDisplay:	212	+	if f.display {
184	-	hasDisplay = true	213	+	parts = append(parts, "share your screen")
185	-	case entity.PermissionTypeWebsiteDataAccess:	214	+	}
186	-	hasDataAccess = true	215	+	if f.dataAccess {
			216	+	if len(parts) == 0 {
			217	+	parts = append(parts,
			218	+	"access its stored data while you browse this site")
			219	+	} else {
			220	+	parts = append(parts, "access its stored data")
187		}	221		}
188		}	222		}
189			223
190		var action string	224		var action string
191		switch {	225		switch {
192	-	case hasMic && hasCam && hasDisplay:	226	+	case len(parts) == 0:
193	-	action = "access your microphone and camera, and share your screen"
194	-	case hasMic && hasDisplay:
195	-	action = "access your microphone and share your screen"
196	-	case hasCam && hasDisplay:
197	-	action = "access your camera and share your screen"
198	-	case hasMic && hasCam:
199	-	action = "access your microphone and camera"
200	-	case hasMic:
201	-	action = "access your microphone"
202	-	case hasCam:
203	-	action = "access your camera"
204	-	case hasDisplay:
205	-	action = "share your screen"
206	-	case hasDataAccess:
207	-	action = "access its stored data while you browse this site"
208	-	default:
209		action = "access your device"	227		action = "access your device"
			228	+	case len(parts) == 1:
			229	+	action = parts[0]
			230	+	case f.mic && f.cam:
			231	+	// When mic+cam are combined into one phrase containing "and",
			232	+	// use ", and" before the next part for clarity.
			233	+	action = strings.Join(parts[:len(parts)-1], ", ") +
			234	+	", and " + parts[len(parts)-1]
			235	+	default:
			236	+	action = parts[0] + " and " + parts[1]
210		}	237		}
211			238
212		return fmt.Sprintf("%s wants to %s.", origin, action)	239		return fmt.Sprintf("%s wants to %s.", origin, action)