refactor(browser): integrate FaviconService, remove legacy favicon handlers

- Initialize FaviconService in createWebView()
- Add GetFaviconDatabase() to WebView for favicon access
- Rewrite stacked panes title bar favicon loading with async callbacks
- Remove legacy favicon handlers from setupWebViewIntegration
- Use callback-based favicon display in title bars
- Replace Show() with WidgetSetVisible() for consistency
- Fix workspace_pane_ops whitespace

commit: c8f47a30dcef6aa462289d962d969abad2075afa
author: bnema · 12mo ago · 2025-10-18 10:54 UTC
parent: a9d5550
signature: GPG Unverified2B82207D370127A0 The key is unknown

4 files · +68 -80

M internal/app/browser/browser.go +1 -0


@@ -36,6 +36,7 @@ type BrowserApp struct {
36	36		// Services
37	37		parserService *services.ParserService
38	38		browserService *services.BrowserService
	39	+	faviconService *services.FaviconService
39	40
40	41		// WebView and controllers
41	42		webView *webkit.WebView


@@ -36,6 +36,7 @@ type BrowserApp struct {
36	// Services	36		// Services
37	parserService *services.ParserService	37		parserService *services.ParserService
38	browserService *services.BrowserService	38		browserService *services.BrowserService
		39	+	faviconService *services.FaviconService
39		40
40	// WebView and controllers	41		// WebView and controllers
41	webView *webkit.WebView	42		webView *webkit.WebView

M internal/app/browser/stacked_panes.go +33 -70

@@ -1,15 +1,14 @@
1	1		package browser
2	2
3	3		import (
4		-	"context"
5	4		"errors"
6	5		"fmt"
7	6		"log"
8	7		"sync/atomic"
9	8		"time"
10	9
11		-	"github.com/bnema/dumber/internal/cache"
12	10		"github.com/bnema/dumber/pkg/webkit"
	11	+	"github.com/diamondburned/gotk4/pkg/gdk/v4"
13	12		"github.com/diamondburned/gotk4/pkg/gtk/v4"
14	13		)
15	14

@@ -664,75 +663,39 @@ func (spm *StackedPaneManager) createTitleBarWithTitle(title string, pageURL str
664	663		titleBox.SetHExpand(true)
665	664		titleBox.SetVExpand(false)
666	665
667		-	// Add favicon placeholder for debugging (always visible)
668		-	var faviconImg gtk.Widgetter
669		-	var debugReason string
670		-
671		-	if pageURL == "" {
672		-	debugReason = "empty pageURL"
673		-	} else if spm.wm == nil {
674		-	debugReason = "nil workspace manager"
675		-	} else if spm.wm.app == nil {
676		-	debugReason = "nil app"
677		-	} else if spm.wm.app.queries == nil {
678		-	debugReason = "nil queries"
679		-	} else {
680		-	// Query database for favicon URL
681		-	ctx := context.Background()
682		-	entry, err := spm.wm.app.queries.GetHistoryEntry(ctx, pageURL)
683		-	if err != nil {
684		-	debugReason = fmt.Sprintf("DB query failed: %v", err)
685		-	log.Printf("[favicon] Failed to get history entry for %s: %v", pageURL, err)
686		-	} else if !entry.FaviconUrl.Valid \|\| entry.FaviconUrl.String == "" {
687		-	debugReason = "no favicon URL in DB"
688		-	log.Printf("[favicon] No favicon URL in DB for %s", pageURL)
689		-	} else {
690		-	faviconURL := entry.FaviconUrl.String
691		-	log.Printf("[favicon] Found favicon URL in DB for %s: %s", pageURL, faviconURL)
	666	+	placeholderLabel := gtk.NewLabel(faviconPlaceholder)
	667	+	if placeholderLabel != nil {
	668	+	placeholderLabel.AddCSSClass("stacked-pane-favicon-placeholder")
	669	+	titleBox.Append(placeholderLabel)
	670	+	webkit.WidgetSetVisible(placeholderLabel, true)
	671	+	}
692	672
693		-	// Use favicon cache with favicon URL (same as dmenu)
694		-	faviconCache, err := cache.NewFaviconCache()
	673	+	if pageURL != "" && spm.wm != nil && spm.wm.app != nil && spm.wm.app.faviconService != nil {
	674	+	spm.wm.app.faviconService.GetFaviconTexture(pageURL, func(texture *gdk.Texture, err error) {
695	675		if err != nil {
696		-	debugReason = fmt.Sprintf("cache init failed: %v", err)
697		-	log.Printf("[favicon] Failed to create cache: %v", err)
698		-	} else {
699		-	faviconPath := faviconCache.GetCachedPath(faviconURL)
700		-	if faviconPath != "" {
701		-	log.Printf("[favicon] Loading cached favicon from: %s", faviconPath)
702		-	faviconImg = gtk.NewImageFromFile(faviconPath)
703		-	if faviconImg != nil {
704		-	if img, ok := faviconImg.(*gtk.Image); ok {
705		-	img.SetPixelSize(16)
706		-	}
707		-	webkit.WidgetAddCSSClass(faviconImg, "stacked-pane-favicon")
708		-	log.Printf("[favicon] Successfully created favicon image widget")
709		-	} else {
710		-	debugReason = "image widget creation failed"
711		-	log.Printf("[favicon] Failed to create image widget from %s", faviconPath)
712		-	}
713		-	} else {
714		-	debugReason = "not cached yet"
715		-	log.Printf("[favicon] Favicon not cached yet, starting download: %s", faviconURL)
716		-	// Start async download for next time (same as dmenu)
717		-	faviconCache.CacheAsync(faviconURL)
718		-	}
	676	+	log.Printf("[favicon] Failed to load favicon for title bar: %s - %v", pageURL, err)
	677	+	return
719	678		}
720		-	}
721		-	}
722	679
723		-	// Add favicon or placeholder
724		-	if faviconImg != nil {
725		-	titleBox.Append(faviconImg)
726		-	webkit.WidgetShow(faviconImg)
727		-	} else {
728		-	// DEBUG: Add visible placeholder label to show code is running
729		-	placeholderLabel := gtk.NewLabel(faviconPlaceholder)
730		-	if placeholderLabel != nil {
731		-	placeholderLabel.AddCSSClass("stacked-pane-favicon-placeholder")
732		-	titleBox.Append(placeholderLabel)
733		-	placeholderLabel.Show()
734		-	log.Printf("[favicon] Using placeholder for %s (reason: %s)", pageURL, debugReason)
735		-	}
	680	+	if texture != nil && titleBox != nil {
	681	+	log.Printf("[favicon] Favicon ready to display in title bar for: %s", pageURL)
	682	+
	683	+	// Create favicon image
	684	+	faviconImg := gtk.NewImageFromPaintable(texture)
	685	+	faviconImg.SetPixelSize(16)
	686	+	webkit.WidgetAddCSSClass(faviconImg, "stacked-pane-favicon")
	687	+
	688	+	// IDEMPOTENT: Remove placeholder only if it still has titleBox as parent
	689	+	// This handles the case where multiple async callbacks fire for the same title bar
	690	+	if placeholderLabel != nil && webkit.WidgetGetParent(placeholderLabel) != nil {
	691	+	titleBox.Remove(placeholderLabel)
	692	+	}
	693	+
	694	+	// Prepend favicon image (multiple calls are safe - GTK handles it)
	695	+	titleBox.Prepend(faviconImg)
	696	+	webkit.WidgetSetVisible(faviconImg, true)
	697	+	}
	698	+	})
736	699		}
737	700
738	701		// Create title label with the specified title

@@ -742,11 +705,11 @@ func (spm *StackedPaneManager) createTitleBarWithTitle(title string, pageURL str
742	705		}
743	706
744	707		titleLabel.AddCSSClass("stacked-pane-title-text")
745		-	titleLabel.SetEllipsize(2) // pango.EllipsizeEnd = 2
	708	+	titleLabel.SetEllipsize(2)
746	709
747	710		titleBox.Append(titleLabel)
748		-	titleBox.Show()
749		-	titleLabel.Show()
	711	+	webkit.WidgetSetVisible(titleBox, true)
	712	+	webkit.WidgetSetVisible(titleLabel, true)
750	713
751	714		return titleBox
752	715		}

@@ -1,15 +1,14 @@
1		package browser	1		package browser
2			2
3		import (	3		import (
4	-	"context"
5		"errors"	4		"errors"
6		"fmt"	5		"fmt"
7		"log"	6		"log"
8		"sync/atomic"	7		"sync/atomic"
9		"time"	8		"time"
10			9
11	-	"github.com/bnema/dumber/internal/cache"
12		"github.com/bnema/dumber/pkg/webkit"	10		"github.com/bnema/dumber/pkg/webkit"
			11	+	"github.com/diamondburned/gotk4/pkg/gdk/v4"
13		"github.com/diamondburned/gotk4/pkg/gtk/v4"	12		"github.com/diamondburned/gotk4/pkg/gtk/v4"
14		)	13		)
15			14

@@ -664,75 +663,39 @@ func (spm *StackedPaneManager) createTitleBarWithTitle(title string, pageURL str
664		titleBox.SetHExpand(true)	663		titleBox.SetHExpand(true)
665		titleBox.SetVExpand(false)	664		titleBox.SetVExpand(false)
666			665
667	-	// Add favicon placeholder for debugging (always visible)	666	+	placeholderLabel := gtk.NewLabel(faviconPlaceholder)
668	-	var faviconImg gtk.Widgetter	667	+	if placeholderLabel != nil {
669	-	var debugReason string	668	+	placeholderLabel.AddCSSClass("stacked-pane-favicon-placeholder")
670	-		669	+	titleBox.Append(placeholderLabel)
671	-	if pageURL == "" {	670	+	webkit.WidgetSetVisible(placeholderLabel, true)
672	-	debugReason = "empty pageURL"	671	+	}
673	-	} else if spm.wm == nil {
674	-	debugReason = "nil workspace manager"
675	-	} else if spm.wm.app == nil {
676	-	debugReason = "nil app"
677	-	} else if spm.wm.app.queries == nil {
678	-	debugReason = "nil queries"
679	-	} else {
680	-	// Query database for favicon URL
681	-	ctx := context.Background()
682	-	entry, err := spm.wm.app.queries.GetHistoryEntry(ctx, pageURL)
683	-	if err != nil {
684	-	debugReason = fmt.Sprintf("DB query failed: %v", err)
685	-	log.Printf("[favicon] Failed to get history entry for %s: %v", pageURL, err)
686	-	} else if !entry.FaviconUrl.Valid \|\| entry.FaviconUrl.String == "" {
687	-	debugReason = "no favicon URL in DB"
688	-	log.Printf("[favicon] No favicon URL in DB for %s", pageURL)
689	-	} else {
690	-	faviconURL := entry.FaviconUrl.String
691	-	log.Printf("[favicon] Found favicon URL in DB for %s: %s", pageURL, faviconURL)
692			672
693	-	// Use favicon cache with favicon URL (same as dmenu)	673	+	if pageURL != "" && spm.wm != nil && spm.wm.app != nil && spm.wm.app.faviconService != nil {
694	-	faviconCache, err := cache.NewFaviconCache()	674	+	spm.wm.app.faviconService.GetFaviconTexture(pageURL, func(texture *gdk.Texture, err error) {
695		if err != nil {	675		if err != nil {
696	-	debugReason = fmt.Sprintf("cache init failed: %v", err)	676	+	log.Printf("[favicon] Failed to load favicon for title bar: %s - %v", pageURL, err)
697	-	log.Printf("[favicon] Failed to create cache: %v", err)	677	+	return
698	-	} else {
699	-	faviconPath := faviconCache.GetCachedPath(faviconURL)
700	-	if faviconPath != "" {
701	-	log.Printf("[favicon] Loading cached favicon from: %s", faviconPath)
702	-	faviconImg = gtk.NewImageFromFile(faviconPath)
703	-	if faviconImg != nil {
704	-	if img, ok := faviconImg.(*gtk.Image); ok {
705	-	img.SetPixelSize(16)
706	-	}
707	-	webkit.WidgetAddCSSClass(faviconImg, "stacked-pane-favicon")
708	-	log.Printf("[favicon] Successfully created favicon image widget")
709	-	} else {
710	-	debugReason = "image widget creation failed"
711	-	log.Printf("[favicon] Failed to create image widget from %s", faviconPath)
712	-	}
713	-	} else {
714	-	debugReason = "not cached yet"
715	-	log.Printf("[favicon] Favicon not cached yet, starting download: %s", faviconURL)
716	-	// Start async download for next time (same as dmenu)
717	-	faviconCache.CacheAsync(faviconURL)
718	-	}
719		}	678		}
720	-	}
721	-	}
722			679
723	-	// Add favicon or placeholder	680	+	if texture != nil && titleBox != nil {
724	-	if faviconImg != nil {	681	+	log.Printf("[favicon] Favicon ready to display in title bar for: %s", pageURL)
725	-	titleBox.Append(faviconImg)	682	+
726	-	webkit.WidgetShow(faviconImg)	683	+	// Create favicon image
727	-	} else {	684	+	faviconImg := gtk.NewImageFromPaintable(texture)
728	-	// DEBUG: Add visible placeholder label to show code is running	685	+	faviconImg.SetPixelSize(16)
729	-	placeholderLabel := gtk.NewLabel(faviconPlaceholder)	686	+	webkit.WidgetAddCSSClass(faviconImg, "stacked-pane-favicon")
730	-	if placeholderLabel != nil {	687	+
731	-	placeholderLabel.AddCSSClass("stacked-pane-favicon-placeholder")	688	+	// IDEMPOTENT: Remove placeholder only if it still has titleBox as parent
732	-	titleBox.Append(placeholderLabel)	689	+	// This handles the case where multiple async callbacks fire for the same title bar
733	-	placeholderLabel.Show()	690	+	if placeholderLabel != nil && webkit.WidgetGetParent(placeholderLabel) != nil {
734	-	log.Printf("[favicon] Using placeholder for %s (reason: %s)", pageURL, debugReason)	691	+	titleBox.Remove(placeholderLabel)
735	-	}	692	+	}
			693	+
			694	+	// Prepend favicon image (multiple calls are safe - GTK handles it)
			695	+	titleBox.Prepend(faviconImg)
			696	+	webkit.WidgetSetVisible(faviconImg, true)
			697	+	}
			698	+	})
736		}	699		}
737			700
738		// Create title label with the specified title	701		// Create title label with the specified title

@@ -742,11 +705,11 @@ func (spm *StackedPaneManager) createTitleBarWithTitle(title string, pageURL str
742		}	705		}
743			706
744		titleLabel.AddCSSClass("stacked-pane-title-text")	707		titleLabel.AddCSSClass("stacked-pane-title-text")
745	-	titleLabel.SetEllipsize(2) // pango.EllipsizeEnd = 2	708	+	titleLabel.SetEllipsize(2)
746			709
747		titleBox.Append(titleLabel)	710		titleBox.Append(titleLabel)
748	-	titleBox.Show()	711	+	webkit.WidgetSetVisible(titleBox, true)
749	-	titleLabel.Show()	712	+	webkit.WidgetSetVisible(titleLabel, true)
750			713
751		return titleBox	714		return titleBox
752		}	715		}

M internal/app/browser/webview.go +33 -9


@@ -14,6 +14,7 @@ import (
14	14		"github.com/bnema/dumber/internal/app/messaging"
15	15		"github.com/bnema/dumber/internal/config"
16	16		"github.com/bnema/dumber/internal/filtering"
	17	+	"github.com/bnema/dumber/internal/services"
17	18		"github.com/bnema/dumber/pkg/webkit"
18	19		)
19	20

@@ -132,15 +133,6 @@ func (app BrowserApp) buildPane(view webkit.WebView) (*BrowserPane, error) {
132	133		app.workspace.RegisterNavigationHandler(view)
133	134		}
134	135
135		-	// Register favicon handlers for this webview (needed for each pane)
136		-	// This ensures favicons are detected and cached for all panes, not just the main webview
137		-	view.RegisterFaviconURIChangedHandler(func(pageURI, faviconURI string) {
138		-	// Call the private handler through a public wrapper or directly via browserService
139		-	// We can't call handleFaviconURIChanged directly as it's private, so we need to expose it
140		-	// For now, duplicate the minimal logic here
141		-	app.browserService.ProcessFaviconURI(pageURI, faviconURI)
142		-	})
143		-
144	136		return pane, nil
145	137		}
146	138

@@ -287,6 +279,12 @@ func (app *BrowserApp) createWebView() error {
287	279
288	280		app.setupWebViewIntegration()
289	281
	282	+	// Initialize FaviconService after WebView is created
	283	+	if err := app.initializeFaviconService(); err != nil {
	284	+	log.Printf("Warning: failed to initialize favicon service: %v", err)
	285	+	// Continue without favicon service - not critical
	286	+	}
	287	+
290	288		// Apply initial zoom and show window
291	289		app.zoomController.ApplyInitialZoom()
292	290

@@ -313,6 +311,32 @@ func (app *BrowserApp) setupWebViewIntegration() {
313	311		}
314	312		}
315	313
	314	+	// initializeFaviconService creates and initializes the FaviconService
	315	+	func (app *BrowserApp) initializeFaviconService() error {
	316	+	// Get the FaviconDatabase from the WebView
	317	+	faviconDB := app.webView.GetFaviconDatabase()
	318	+	if faviconDB == nil {
	319	+	return fmt.Errorf("favicon database not available")
	320	+	}
	321	+
	322	+	// Get the WebKit data directory
	323	+	dataDir, err := config.GetDataDir()
	324	+	if err != nil {
	325	+	return fmt.Errorf("failed to get data directory: %w", err)
	326	+	}
	327	+	webkitData := filepath.Join(dataDir, "webkit")
	328	+
	329	+	// Create FaviconService
	330	+	faviconService, err := services.NewFaviconService(faviconDB, app.queries, webkitData)
	331	+	if err != nil {
	332	+	return fmt.Errorf("failed to create favicon service: %w", err)
	333	+	}
	334	+
	335	+	app.faviconService = faviconService
	336	+	log.Printf("[favicon] FaviconService initialized successfully")
	337	+	return nil
	338	+	}
	339	+
316	340		// setupContentBlocking initializes the content blocking system with proper timing
317	341		func (app *BrowserApp) setupContentBlocking() error {
318	342		log.Printf("Initializing content blocking system...")

 	"github.com/bnema/dumber/internal/app/messaging"
 	"github.com/bnema/dumber/internal/config"
 	"github.com/bnema/dumber/internal/filtering"
+	"github.com/bnema/dumber/internal/services"
 	"github.com/bnema/dumber/pkg/webkit"
 )
 		app.workspace.RegisterNavigationHandler(view)
 	}
-	// Register favicon handlers for this webview (needed for each pane)
-	// This ensures favicons are detected and cached for all panes, not just the main webview
-	view.RegisterFaviconURIChangedHandler(func(pageURI, faviconURI string) {
-		// Call the private handler through a public wrapper or directly via browserService
-		// We can't call handleFaviconURIChanged directly as it's private, so we need to expose it
-		// For now, duplicate the minimal logic here
-		app.browserService.ProcessFaviconURI(pageURI, faviconURI)
-	})
 	return pane, nil
 }
 	app.setupWebViewIntegration()
+	// Initialize FaviconService after WebView is created
+	if err := app.initializeFaviconService(); err != nil {
+		log.Printf("Warning: failed to initialize favicon service: %v", err)
+		// Continue without favicon service - not critical
+	}
 	// Apply initial zoom and show window
 	app.zoomController.ApplyInitialZoom()
 	}
 }
+// initializeFaviconService creates and initializes the FaviconService
+func (app *BrowserApp) initializeFaviconService() error {
+	// Get the FaviconDatabase from the WebView
+	faviconDB := app.webView.GetFaviconDatabase()
+	if faviconDB == nil {
+		return fmt.Errorf("favicon database not available")
+	}
+	// Get the WebKit data directory
+	dataDir, err := config.GetDataDir()
+	if err != nil {
+		return fmt.Errorf("failed to get data directory: %w", err)
+	}
+	webkitData := filepath.Join(dataDir, "webkit")
+	// Create FaviconService
+	faviconService, err := services.NewFaviconService(faviconDB, app.queries, webkitData)
+	if err != nil {
+		return fmt.Errorf("failed to create favicon service: %w", err)
+	}
+	app.faviconService = faviconService
+	log.Printf("[favicon] FaviconService initialized successfully")
+	return nil
+}
 // setupContentBlocking initializes the content blocking system with proper timing
 func (app *BrowserApp) setupContentBlocking() error {
 	log.Printf("Initializing content blocking system...")

M internal/app/browser/workspace_pane_ops.go +1 -1


@@ -908,7 +908,7 @@ func (wm WorkspaceManager) cleanupAndExit(node paneNode) (*paneNode, error) {
908	908		log.Printf("[workspace] closing final pane; exiting browser")
909	909		wm.cleanupPane(node, wm.nextCleanupGeneration())
910	910		wm.detachHover(node)
911		-
	911	+
912	912		// Gracefully quit the GTK main loop
913	913		webkit.QuitMainLoop()
914	914		return nil, nil


@@ -908,7 +908,7 @@ func (wm WorkspaceManager) cleanupAndExit(node paneNode) (*paneNode, error) {
908		log.Printf("[workspace] closing final pane; exiting browser")	908		log.Printf("[workspace] closing final pane; exiting browser")
909		wm.cleanupPane(node, wm.nextCleanupGeneration())	909		wm.cleanupPane(node, wm.nextCleanupGeneration())
910		wm.detachHover(node)	910		wm.detachHover(node)
911	-		911	+
912		// Gracefully quit the GTK main loop	912		// Gracefully quit the GTK main loop
913		webkit.QuitMainLoop()	913		webkit.QuitMainLoop()
914		return nil, nil	914		return nil, nil