refactor(webkit): fix workspace WebView architecture for GTK4

PROBLEM:
Split panes were failing with GTK error "widget already has parent".
Every WebView was being created with its own GtkWindow, making it
impossible to reparent into workspace paned containers.

SOLUTION:
Introduced CreateWindow config flag to separate standalone vs embedded
WebView creation:

- Added Config.CreateWindow field (default: true)
- Refactored NewWebView to create GtkBox container holding WebView
- Window creation now conditional based on CreateWindow flag
- RootWidget() returns container (can be reparented) not WebView
- Initial WebView uses CreateWindow=true (needs window)
- Workspace/popup WebViews use CreateWindow=false (embedded)

This follows the same pattern as main branch (webkit_cgo.go:1118-1121)
where workspace WebViews are created without windows to allow GTK
reparenting into paned containers.

IMPACT:
- Split panes (Ctrl+P) now work correctly with gotk4
- Popups can be embedded in workspace panes
- No more "widget already has parent" GTK assertions
- WebView containers can be freely reparented by workspace manager

commit: a1a9d2dc8c7ebf489c70e6a8c3a1b9f337cc4961
author: bamen · 12mo ago · 2025-10-10 05:29 UTC
parent: 3562591
signature: unsigned

5 files · +62 -20

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


@@ -52,6 +52,7 @@ func (app BrowserApp) buildWebkitConfig() (webkit.Config, error) {
52	52		DataDir: webkitData,
53	53		CacheDir: webkitCache,
54	54		AppearanceConfigJSON: app.buildAppearanceConfigJSON(),
	55	+	CreateWindow: true, // Default to creating a window for standalone WebViews
55	56		}
56	57
57	58		return cfg, nil


@@ -52,6 +52,7 @@ func (app BrowserApp) buildWebkitConfig() (webkit.Config, error) {
52	DataDir: webkitData,	52		DataDir: webkitData,
53	CacheDir: webkitCache,	53		CacheDir: webkitCache,
54	AppearanceConfigJSON: app.buildAppearanceConfigJSON(),	54		AppearanceConfigJSON: app.buildAppearanceConfigJSON(),
		55	+	CreateWindow: true, // Default to creating a window for standalone WebViews
55	}	56		}
56		57
57	return cfg, nil	58		return cfg, nil

M internal/app/browser/workspace_manager.go +4 -3


@@ -115,9 +115,10 @@ func NewWorkspaceManager(app BrowserApp, rootPane BrowserPane) *WorkspaceManag
115	115		if err != nil {
116	116		return nil, err
117	117		}
118		-	// Pane WebViews are embedded inside the main application window, so they do not
119		-	// need their own toplevel GtkWindow. In gotk4, this is the default behavior when
120		-	// using webkit.NewWebView() without creating a separate window.
	118	+	// CRITICAL: Pane WebViews are embedded inside the workspace paned containers,
	119	+	// so they do not need their own toplevel GtkWindow. Setting CreateWindow=false
	120	+	// prevents GTK "widget already has parent" errors when adding to paned containers.
	121	+	cfg.CreateWindow = false
121	122		return webkit.NewWebView(cfg)
122	123		}
123	124		manager.createPaneFn = func(view webkit.WebView) (BrowserPane, error) {

 		if err != nil {
 			return nil, err
 		}
-		// Pane WebViews are embedded inside the main application window, so they do not
+		// CRITICAL: Pane WebViews are embedded inside the workspace paned containers,
-		// need their own toplevel GtkWindow. In gotk4, this is the default behavior when
+		// so they do not need their own toplevel GtkWindow. Setting CreateWindow=false
-		// using webkit.NewWebView() without creating a separate window.
+		// prevents GTK "widget already has parent" errors when adding to paned containers.
+		cfg.CreateWindow = false
 		return webkit.NewWebView(cfg)
 	}
 	manager.createPaneFn = func(view *webkit.WebView) (*BrowserPane, error) {

M internal/app/browser/workspace_popup.go +10 -0


@@ -70,6 +70,8 @@ func (wm WorkspaceManager) HandlePopup(source webkit.WebView, url string) *web
70	70		log.Printf("[workspace] failed to build webkit config: %v - allowing native popup", err)
71	71		return nil
72	72		}
	73	+	// Popups are embedded in workspace paned containers - no separate window needed
	74	+	webkitCfg.CreateWindow = false
73	75		// Create as related to avoid WindowFeatures crash; we'll decide final placement later
74	76		newView, err := webkit.NewWebViewWithRelated(webkitCfg, source)
75	77		if err != nil {

@@ -105,6 +107,8 @@ func (wm WorkspaceManager) HandlePopup(source webkit.WebView, url string) *web
105	107		log.Printf("[workspace] failed to build webkit config: %v - allowing native popup", err)
106	108		return nil
107	109		}
	110	+	// Popups are embedded in workspace paned containers - no separate window needed
	111	+	webkitCfg.CreateWindow = false
108	112		newView, err := webkit.NewWebViewWithRelated(webkitCfg, source)
109	113		if err != nil {
110	114		log.Printf("[workspace] failed to create placeholder WebView: %v - allowing native popup", err)

@@ -342,6 +346,8 @@ func (wm WorkspaceManager) handleIntentAsTab(sourceNode paneNode, url string,
342	346		log.Printf("[workspace] failed to build webkit config: %v - allowing native popup", err)
343	347		return nil
344	348		}
	349	+	// Tabs are embedded in workspace paned containers - no separate window needed
	350	+	webkitCfg.CreateWindow = false
345	351
346	352		newView, err := webkit.NewWebView(webkitCfg)
347	353		if err != nil {

@@ -398,6 +404,8 @@ func (wm WorkspaceManager) handleIntentAsPopup(sourceNode paneNode, url string
398	404		log.Printf("[workspace] failed to build webkit config: %v - allowing native popup", err)
399	405		return nil
400	406		}
	407	+	// Popups are embedded in workspace paned containers - no separate window needed
	408	+	webkitCfg.CreateWindow = false
401	409
402	410		newView, err := webkit.NewWebViewWithRelated(webkitCfg, sourceNode.pane.webView)
403	411		if err != nil {

@@ -792,6 +800,8 @@ func (wm WorkspaceManager) handleDetectedWindowType(sourceNode paneNode, webVi
792	800		log.Printf("[workspace] failed to build webkit config for tab: %v", err)
793	801		return
794	802		}
	803	+	// Tabs are embedded in workspace paned containers - no separate window needed
	804	+	webkitCfg.CreateWindow = false
795	805
796	806		// Create independent WebView like handleIntentAsTab does
797	807		independentView, err := webkit.NewWebView(webkitCfg)


@@ -70,6 +70,8 @@ func (wm WorkspaceManager) HandlePopup(source webkit.WebView, url string) *web
70	log.Printf("[workspace] failed to build webkit config: %v - allowing native popup", err)	70		log.Printf("[workspace] failed to build webkit config: %v - allowing native popup", err)
71	return nil	71		return nil
72	}	72		}
		73	+	// Popups are embedded in workspace paned containers - no separate window needed
		74	+	webkitCfg.CreateWindow = false
73	// Create as related to avoid WindowFeatures crash; we'll decide final placement later	75		// Create as related to avoid WindowFeatures crash; we'll decide final placement later
74	newView, err := webkit.NewWebViewWithRelated(webkitCfg, source)	76		newView, err := webkit.NewWebViewWithRelated(webkitCfg, source)
75	if err != nil {	77		if err != nil {

@@ -105,6 +107,8 @@ func (wm WorkspaceManager) HandlePopup(source webkit.WebView, url string) *web
105	log.Printf("[workspace] failed to build webkit config: %v - allowing native popup", err)	107		log.Printf("[workspace] failed to build webkit config: %v - allowing native popup", err)
106	return nil	108		return nil
107	}	109		}
		110	+	// Popups are embedded in workspace paned containers - no separate window needed
		111	+	webkitCfg.CreateWindow = false
108	newView, err := webkit.NewWebViewWithRelated(webkitCfg, source)	112		newView, err := webkit.NewWebViewWithRelated(webkitCfg, source)
109	if err != nil {	113		if err != nil {
110	log.Printf("[workspace] failed to create placeholder WebView: %v - allowing native popup", err)	114		log.Printf("[workspace] failed to create placeholder WebView: %v - allowing native popup", err)

@@ -342,6 +346,8 @@ func (wm WorkspaceManager) handleIntentAsTab(sourceNode paneNode, url string,
342	log.Printf("[workspace] failed to build webkit config: %v - allowing native popup", err)	346		log.Printf("[workspace] failed to build webkit config: %v - allowing native popup", err)
343	return nil	347		return nil
344	}	348		}
		349	+	// Tabs are embedded in workspace paned containers - no separate window needed
		350	+	webkitCfg.CreateWindow = false
345		351
346	newView, err := webkit.NewWebView(webkitCfg)	352		newView, err := webkit.NewWebView(webkitCfg)
347	if err != nil {	353		if err != nil {

@@ -398,6 +404,8 @@ func (wm WorkspaceManager) handleIntentAsPopup(sourceNode paneNode, url string
398	log.Printf("[workspace] failed to build webkit config: %v - allowing native popup", err)	404		log.Printf("[workspace] failed to build webkit config: %v - allowing native popup", err)
399	return nil	405		return nil
400	}	406		}
		407	+	// Popups are embedded in workspace paned containers - no separate window needed
		408	+	webkitCfg.CreateWindow = false
401		409
402	newView, err := webkit.NewWebViewWithRelated(webkitCfg, sourceNode.pane.webView)	410		newView, err := webkit.NewWebViewWithRelated(webkitCfg, sourceNode.pane.webView)
403	if err != nil {	411		if err != nil {

@@ -792,6 +800,8 @@ func (wm WorkspaceManager) handleDetectedWindowType(sourceNode paneNode, webVi
792	log.Printf("[workspace] failed to build webkit config for tab: %v", err)	800		log.Printf("[workspace] failed to build webkit config for tab: %v", err)
793	return	801		return
794	}	802		}
		803	+	// Tabs are embedded in workspace paned containers - no separate window needed
		804	+	webkitCfg.CreateWindow = false
795		805
796	// Create independent WebView like handleIntentAsTab does	806		// Create independent WebView like handleIntentAsTab does
797	independentView, err := webkit.NewWebView(webkitCfg)	807		independentView, err := webkit.NewWebView(webkitCfg)

M pkg/webkit/types.go +5 -0


@@ -54,6 +54,10 @@ type Config struct {
54	54		// AppearanceConfigJSON is the JSON string for appearance configuration
55	55		// This will be injected at document-start via UserContentManager
56	56		AppearanceConfigJSON string
	57	+
	58	+	// CreateWindow controls whether to create a standalone GTK Window for this WebView
	59	+	// Set to false for WebViews that will be embedded in workspace panes
	60	+	CreateWindow bool
57	61		}
58	62
59	63		// GetDefaultConfig returns a Config with sensible defaults

@@ -66,5 +70,6 @@ func GetDefaultConfig() *Config {
66	70		HardwareAcceleration: true,
67	71		DefaultFontSize: 16,
68	72		MinimumFontSize: 8,
	73	+	CreateWindow: true, // Default to standalone window
69	74		}
70	75		}


@@ -54,6 +54,10 @@ type Config struct {
54	// AppearanceConfigJSON is the JSON string for appearance configuration	54		// AppearanceConfigJSON is the JSON string for appearance configuration
55	// This will be injected at document-start via UserContentManager	55		// This will be injected at document-start via UserContentManager
56	AppearanceConfigJSON string	56		AppearanceConfigJSON string
		57	+
		58	+	// CreateWindow controls whether to create a standalone GTK Window for this WebView
		59	+	// Set to false for WebViews that will be embedded in workspace panes
		60	+	CreateWindow bool
57	}	61		}
58		62
59	// GetDefaultConfig returns a Config with sensible defaults	63		// GetDefaultConfig returns a Config with sensible defaults

@@ -66,5 +70,6 @@ func GetDefaultConfig() *Config {
66	HardwareAcceleration: true,	70		HardwareAcceleration: true,
67	DefaultFontSize: 16,	71		DefaultFontSize: 16,
68	MinimumFontSize: 8,	72		MinimumFontSize: 8,
		73	+	CreateWindow: true, // Default to standalone window
69	}	74		}
70	}	75		}

M pkg/webkit/webview.go +42 -17


@@ -20,9 +20,10 @@ var (
20	20
21	21		// WebView wraps a WebKitGTK WebView
22	22		type WebView struct {
23		-	view *webkit.WebView
24		-	window *Window
25		-	id uint64
	23	+	view *webkit.WebView
	24	+	container *gtk.Box // Container that holds the WebView
	25	+	window *Window // Optional window (only if CreateWindow=true)
	26	+	id uint64
26	27
27	28		// State
28	29		config *Config

@@ -50,16 +51,10 @@ func NewWebView(cfg Config) (WebView, error) {
50	51
51	52		// Generate unique ID immediately
52	53		id := atomic.AddUint64(&viewIDCounter, 1)
53		-	log.Printf("[webkit] Generated WebView ID: %d", id)
	54	+	log.Printf("[webkit] Generated WebView ID: %d (CreateWindow=%v)", id, cfg.CreateWindow)
54	55
55	56		InitMainThread()
56	57
57		-	// Create GTK Window
58		-	window, err := NewWindow("Dumb Browser")
59		-	if err != nil {
60		-	return nil, err
61		-	}
62		-
63	58		// Initialize persistent session - REQUIRED, no ephemeral fallback
64	59		// This must be done BEFORE creating the WebView
65	60		if cfg.DataDir == "" \|\| cfg.CacheDir == "" {

@@ -103,11 +98,25 @@ func NewWebView(cfg Config) (WebView, error) {
103	98		return nil, fmt.Errorf("failed to setup user content manager: %w", err)
104	99		}
105	100
	101	+	// Create container (GtkBox) to hold the WebView
	102	+	// This allows the WebView to be reparented into workspace panes
	103	+	container := gtk.NewBox(gtk.OrientationVertical, 0)
	104	+	container.SetHExpand(true)
	105	+	container.SetVExpand(true)
	106	+
	107	+	// Configure WebView widget for expansion
	108	+	wkView.SetHExpand(true)
	109	+	wkView.SetVExpand(true)
	110	+
	111	+	// Add WebView to container
	112	+	container.Append(wkView)
	113	+
106	114		wv := &WebView{
107		-	view: wkView,
108		-	window: window,
109		-	id: id,
110		-	config: cfg,
	115	+	view: wkView,
	116	+	container: container,
	117	+	window: nil, // Will be set below if CreateWindow is true
	118	+	id: id,
	119	+	config: cfg,
111	120		}
112	121
113	122		// Apply configuration

@@ -121,8 +130,20 @@ func NewWebView(cfg Config) (WebView, error) {
121	130		// Attach keyboard bridge to forward keyboard events to JavaScript
122	131		wv.AttachKeyboardBridge()
123	132
124		-	// Add WebView as child of window
125		-	window.SetChild(wkView)
	133	+	// Only create window if requested (standalone WebViews)
	134	+	// Workspace panes will set CreateWindow=false and manage the container themselves
	135	+	if cfg.CreateWindow {
	136	+	window, err := NewWindow("Dumber Browser")
	137	+	if err != nil {
	138	+	return nil, err
	139	+	}
	140	+	wv.window = window
	141	+	// Add container as child of window
	142	+	window.SetChild(container)
	143	+	log.Printf("[webkit] Created standalone window for WebView ID %d", id)
	144	+	} else {
	145	+	log.Printf("[webkit] Created WebView ID %d without window (for workspace embedding)", id)
	146	+	}
126	147
127	148		// Register in global registry
128	149		viewMu.Lock()

@@ -489,8 +510,12 @@ func (w *WebView) Widget() gtk.Widgetter {
489	510		}
490	511
491	512		// RootWidget returns the root container widget for this WebView
492		-	// In the new architecture, this is just the WebView itself
	513	+	// This is the GtkBox container that holds the WebView and can be reparented
493	514		func (w *WebView) RootWidget() gtk.Widgetter {
	515	+	if w.container != nil {
	516	+	return w.container
	517	+	}
	518	+	// Fallback to WebView if container is not available (shouldn't happen)
494	519		return w.AsWidget()
495	520		}
496	521

 // WebView wraps a WebKitGTK WebView
 type WebView struct {
-	view   *webkit.WebView
+	view      *webkit.WebView
-	window *Window
+	container *gtk.Box   // Container that holds the WebView
-	id     uint64
+	window    *Window    // Optional window (only if CreateWindow=true)
+	id        uint64
 	// State
 	config    *Config
 	// Generate unique ID immediately
 	id := atomic.AddUint64(&viewIDCounter, 1)
-	log.Printf("[webkit] Generated WebView ID: %d", id)
+	log.Printf("[webkit] Generated WebView ID: %d (CreateWindow=%v)", id, cfg.CreateWindow)
 	InitMainThread()
-	// Create GTK Window
-	window, err := NewWindow("Dumb Browser")
-	if err != nil {
-		return nil, err
-	}
 	// Initialize persistent session - REQUIRED, no ephemeral fallback
 	// This must be done BEFORE creating the WebView
 	if cfg.DataDir == "" || cfg.CacheDir == "" {
 		return nil, fmt.Errorf("failed to setup user content manager: %w", err)
 	}
+	// Create container (GtkBox) to hold the WebView
+	// This allows the WebView to be reparented into workspace panes
+	container := gtk.NewBox(gtk.OrientationVertical, 0)
+	container.SetHExpand(true)
+	container.SetVExpand(true)
+	// Configure WebView widget for expansion
+	wkView.SetHExpand(true)
+	wkView.SetVExpand(true)
+	// Add WebView to container
+	container.Append(wkView)
 	wv := &WebView{
-		view:   wkView,
+		view:      wkView,
-		window: window,
+		container: container,
-		id:     id,
+		window:    nil, // Will be set below if CreateWindow is true
-		config: cfg,
+		id:        id,
+		config:    cfg,
 	}
 	// Apply configuration
 	// Attach keyboard bridge to forward keyboard events to JavaScript
 	wv.AttachKeyboardBridge()
-	// Add WebView as child of window
+	// Only create window if requested (standalone WebViews)
-	window.SetChild(wkView)
+	// Workspace panes will set CreateWindow=false and manage the container themselves
+	if cfg.CreateWindow {
+		window, err := NewWindow("Dumber Browser")
+		if err != nil {
+			return nil, err
+		}
+		wv.window = window
+		// Add container as child of window
+		window.SetChild(container)
+		log.Printf("[webkit] Created standalone window for WebView ID %d", id)
+	} else {
+		log.Printf("[webkit] Created WebView ID %d without window (for workspace embedding)", id)
+	}
 	// Register in global registry
 	viewMu.Lock()
 }
 // RootWidget returns the root container widget for this WebView
-// In the new architecture, this is just the WebView itself
+// This is the GtkBox container that holds the WebView and can be reparented
 func (w *WebView) RootWidget() gtk.Widgetter {
+	if w.container != nil {
+		return w.container
+	}
+	// Fallback to WebView if container is not available (shouldn't happen)
 	return w.AsWidget()
 }