fix(webkit): refresh settings on pooled webviews
- commit
- a0ed55e8e2f1b6f15ff54e4c62b7ea5184bf4537
- author
- brice · · 2026-06-23 16:28 UTC
- committer
- brice · · 2026-06-23 20:01 UTC
- parent
- 1396c95
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +55 -10
M internal/infrastructure/webkit/engine_runtime_test.go +0 -9
| @@ -2,7 +2,6 @@ package webkit | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | - | "os" | |
| 6 | 5 | "testing" | |
| 7 | 6 | ||
| 8 | 7 | "github.com/bnema/dumber/internal/application/port" | |
| @@ -91,11 +90,3 @@ func TestEngineConfigureContentInjectorAutoCopyGetterReadsCurrentPayload(t *test | |||
| 91 | 90 | ||
| 92 | 91 | require.True(t, injector.autoCopyConfigGetter()) | |
| 93 | 92 | } | |
| 94 | - | ||
| 95 | - | func TestNewEngineAutoCopyGetterUsesRuntimeSettingsPayload(t *testing.T) { | |
| 96 | - | source, err := os.ReadFile("engine_init.go") | |
| 97 | - | require.NoError(t, err) | |
| 98 | - | globalGetter := "config." + "Get()" + ".Clipboard.AutoCopyOnSelection" | |
| 99 | - | require.NotContains(t, string(source), globalGetter) | |
| 100 | - | require.Contains(t, string(source), "settings.current().WebContent.AutoCopyOnSelection") | |
| 101 | - | } | |
| @@ -2,7 +2,6 @@ package webkit | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | - | "os" | |||
| 6 | "testing" | 5 | "testing" | ||
| 7 | 6 | ||||
| 8 | "github.com/bnema/dumber/internal/application/port" | 7 | "github.com/bnema/dumber/internal/application/port" | ||
| @@ -91,11 +90,3 @@ func TestEngineConfigureContentInjectorAutoCopyGetterReadsCurrentPayload(t *test | |||||
| 91 | 90 | ||||
| 92 | require.True(t, injector.autoCopyConfigGetter()) | 91 | require.True(t, injector.autoCopyConfigGetter()) | ||
| 93 | } | 92 | } | ||
| 94 | - | ||||
| 95 | - | func TestNewEngineAutoCopyGetterUsesRuntimeSettingsPayload(t *testing.T) { | |||
| 96 | - | source, err := os.ReadFile("engine_init.go") | |||
| 97 | - | require.NoError(t, err) | |||
| 98 | - | globalGetter := "config." + "Get()" + ".Clipboard.AutoCopyOnSelection" | |||
| 99 | - | require.NotContains(t, string(source), globalGetter) | |||
| 100 | - | require.Contains(t, string(source), "settings.current().WebContent.AutoCopyOnSelection") | |||
| 101 | - | } | |||
M internal/infrastructure/webkit/pool.go +7 -0
| @@ -173,6 +173,9 @@ func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | |||
| 173 | 173 | wv.inner.AddCssClass("webview-themed") | |
| 174 | 174 | // Keep pooled WebViews hidden until we explicitly reveal them. | |
| 175 | 175 | wv.inner.SetVisible(false) | |
| 176 | + | if shouldReapplySettingsOnAcquire(p.settings, wv) { | |
| 177 | + | p.settings.ApplyToWebView(ctx, wv.inner) | |
| 178 | + | } | |
| 176 | 179 | ||
| 177 | 180 | // Ensure frontend is attached even if this WebView was pooled before injection was configured. | |
| 178 | 181 | _ = wv.AttachFrontend(ctx, p.injector, p.router) | |
| @@ -237,6 +240,10 @@ func needsExplicitContextMenuSignalOnAcquire(wv *WebView, pipeline *contextMenuP | |||
| 237 | 240 | return wv != nil && pipeline != nil && len(wv.signalIDs) > 0 | |
| 238 | 241 | } | |
| 239 | 242 | ||
| 243 | + | func shouldReapplySettingsOnAcquire(settings *SettingsManager, wv *WebView) bool { | |
| 244 | + | return settings != nil && wv != nil && wv.inner != nil | |
| 245 | + | } | |
| 246 | + | ||
| 240 | 247 | func (p *WebViewPool) createWebView(ctx context.Context) (*WebView, error) { | |
| 241 | 248 | log := logging.FromContext(ctx) | |
| 242 | 249 | ||
| @@ -173,6 +173,9 @@ func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | |||||
| 173 | wv.inner.AddCssClass("webview-themed") | 173 | wv.inner.AddCssClass("webview-themed") | ||
| 174 | // Keep pooled WebViews hidden until we explicitly reveal them. | 174 | // Keep pooled WebViews hidden until we explicitly reveal them. | ||
| 175 | wv.inner.SetVisible(false) | 175 | wv.inner.SetVisible(false) | ||
| 176 | + | if shouldReapplySettingsOnAcquire(p.settings, wv) { | |||
| 177 | + | p.settings.ApplyToWebView(ctx, wv.inner) | |||
| 178 | + | } | |||
| 176 | 179 | ||||
| 177 | // Ensure frontend is attached even if this WebView was pooled before injection was configured. | 180 | // Ensure frontend is attached even if this WebView was pooled before injection was configured. | ||
| 178 | _ = wv.AttachFrontend(ctx, p.injector, p.router) | 181 | _ = wv.AttachFrontend(ctx, p.injector, p.router) | ||
| @@ -237,6 +240,10 @@ func needsExplicitContextMenuSignalOnAcquire(wv *WebView, pipeline *contextMenuP | |||||
| 237 | return wv != nil && pipeline != nil && len(wv.signalIDs) > 0 | 240 | return wv != nil && pipeline != nil && len(wv.signalIDs) > 0 | ||
| 238 | } | 241 | } | ||
| 239 | 242 | ||||
| 243 | + | func shouldReapplySettingsOnAcquire(settings *SettingsManager, wv *WebView) bool { | |||
| 244 | + | return settings != nil && wv != nil && wv.inner != nil | |||
| 245 | + | } | |||
| 246 | + | ||||
| 240 | func (p *WebViewPool) createWebView(ctx context.Context) (*WebView, error) { | 247 | func (p *WebViewPool) createWebView(ctx context.Context) (*WebView, error) { | ||
| 241 | log := logging.FromContext(ctx) | 248 | log := logging.FromContext(ctx) | ||
| 242 | 249 | ||||
M internal/infrastructure/webkit/pool_context_menu_test.go +48 -1
| @@ -1,6 +1,12 @@ | |||
| 1 | 1 | package webkit | |
| 2 | 2 | ||
| 3 | - | import "testing" | |
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | "github.com/bnema/dumber/internal/application/port" | |
| 8 | + | "github.com/bnema/puregotk/v4/webkit" | |
| 9 | + | ) | |
| 4 | 10 | ||
| 5 | 11 | func TestNeedsExplicitContextMenuSignalOnAcquire(t *testing.T) { | |
| 6 | 12 | t.Run("true for prewarmed view that already has other signals", func(t *testing.T) { | |
| @@ -24,3 +30,44 @@ func TestNeedsExplicitContextMenuSignalOnAcquire(t *testing.T) { | |||
| 24 | 30 | } | |
| 25 | 31 | }) | |
| 26 | 32 | } | |
| 33 | + | ||
| 34 | + | func TestShouldReapplySettingsOnAcquire(t *testing.T) { | |
| 35 | + | settings := NewSettingsManager(context.Background(), port.EngineSettingsPayload{}) | |
| 36 | + | ||
| 37 | + | tests := []struct { | |
| 38 | + | name string | |
| 39 | + | settings *SettingsManager | |
| 40 | + | wv *WebView | |
| 41 | + | want bool | |
| 42 | + | }{ | |
| 43 | + | { | |
| 44 | + | name: "true for current settings and native webview", | |
| 45 | + | settings: settings, | |
| 46 | + | wv: &WebView{inner: &webkit.WebView{}}, | |
| 47 | + | want: true, | |
| 48 | + | }, | |
| 49 | + | { | |
| 50 | + | name: "false without settings manager", | |
| 51 | + | settings: nil, | |
| 52 | + | wv: &WebView{inner: &webkit.WebView{}}, | |
| 53 | + | }, | |
| 54 | + | { | |
| 55 | + | name: "false without pooled webview", | |
| 56 | + | settings: settings, | |
| 57 | + | wv: nil, | |
| 58 | + | }, | |
| 59 | + | { | |
| 60 | + | name: "false before native webview exists", | |
| 61 | + | settings: settings, | |
| 62 | + | wv: &WebView{}, | |
| 63 | + | }, | |
| 64 | + | } | |
| 65 | + | ||
| 66 | + | for _, tt := range tests { | |
| 67 | + | t.Run(tt.name, func(t *testing.T) { | |
| 68 | + | if got := shouldReapplySettingsOnAcquire(tt.settings, tt.wv); got != tt.want { | |
| 69 | + | t.Fatalf("shouldReapplySettingsOnAcquire() = %v, want %v", got, tt.want) | |
| 70 | + | } | |
| 71 | + | }) | |
| 72 | + | } | |
| 73 | + | } | |
| @@ -1,6 +1,12 @@ | |||||
| 1 | package webkit | 1 | package webkit | ||
| 2 | 2 | ||||
| 3 | - | import "testing" | 3 | + | import ( |
| 4 | + | "context" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | "github.com/bnema/dumber/internal/application/port" | |||
| 8 | + | "github.com/bnema/puregotk/v4/webkit" | |||
| 9 | + | ) | |||
| 4 | 10 | ||||
| 5 | func TestNeedsExplicitContextMenuSignalOnAcquire(t *testing.T) { | 11 | func TestNeedsExplicitContextMenuSignalOnAcquire(t *testing.T) { | ||
| 6 | t.Run("true for prewarmed view that already has other signals", func(t *testing.T) { | 12 | t.Run("true for prewarmed view that already has other signals", func(t *testing.T) { | ||
| @@ -24,3 +30,44 @@ func TestNeedsExplicitContextMenuSignalOnAcquire(t *testing.T) { | |||||
| 24 | } | 30 | } | ||
| 25 | }) | 31 | }) | ||
| 26 | } | 32 | } | ||
| 33 | + | ||||
| 34 | + | func TestShouldReapplySettingsOnAcquire(t *testing.T) { | |||
| 35 | + | settings := NewSettingsManager(context.Background(), port.EngineSettingsPayload{}) | |||
| 36 | + | ||||
| 37 | + | tests := []struct { | |||
| 38 | + | name string | |||
| 39 | + | settings *SettingsManager | |||
| 40 | + | wv *WebView | |||
| 41 | + | want bool | |||
| 42 | + | }{ | |||
| 43 | + | { | |||
| 44 | + | name: "true for current settings and native webview", | |||
| 45 | + | settings: settings, | |||
| 46 | + | wv: &WebView{inner: &webkit.WebView{}}, | |||
| 47 | + | want: true, | |||
| 48 | + | }, | |||
| 49 | + | { | |||
| 50 | + | name: "false without settings manager", | |||
| 51 | + | settings: nil, | |||
| 52 | + | wv: &WebView{inner: &webkit.WebView{}}, | |||
| 53 | + | }, | |||
| 54 | + | { | |||
| 55 | + | name: "false without pooled webview", | |||
| 56 | + | settings: settings, | |||
| 57 | + | wv: nil, | |||
| 58 | + | }, | |||
| 59 | + | { | |||
| 60 | + | name: "false before native webview exists", | |||
| 61 | + | settings: settings, | |||
| 62 | + | wv: &WebView{}, | |||
| 63 | + | }, | |||
| 64 | + | } | |||
| 65 | + | ||||
| 66 | + | for _, tt := range tests { | |||
| 67 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 68 | + | if got := shouldReapplySettingsOnAcquire(tt.settings, tt.wv); got != tt.want { | |||
| 69 | + | t.Fatalf("shouldReapplySettingsOnAcquire() = %v, want %v", got, tt.want) | |||
| 70 | + | } | |||
| 71 | + | }) | |||
| 72 | + | } | |||
| 73 | + | } | |||