Expand native popup content widgets for WebKit
Reason: WebKit native OAuth popups were opening a real GTK shell and loading the popup page, but the embedded popup WebView was attached without expansion in that host path. The result was a black native popup window even though Google OAuth navigation had succeeded. Scope: - prepare wrapped native popup content widgets before attaching them to the popup shell - enforce hexpand and vexpand for native popup content - add focused regression tests for native popup widget preparation Description: This commit factors native popup widget preparation into a small helper, applies expansion flags before SetContent, and adds tests for both the successful and invalid-widget cases. Manual retests confirmed Google OAuth popups now render correctly with the WebKit engine on x.com and Notion, while the existing CEF popup flow remains working.
- commit
- cf66830ed8df004a73076cb15cb2cd76f700514a
- author
- brice · · 2026-05-13 07:02 UTC
- parent
- 8354b8a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +42 -3
M internal/ui/native_popup_window.go +18 -3
| @@ -9,6 +9,7 @@ import ( | |||
| 9 | 9 | "github.com/bnema/dumber/internal/domain/entity" | |
| 10 | 10 | "github.com/bnema/dumber/internal/logging" | |
| 11 | 11 | "github.com/bnema/dumber/internal/ui/coordinator/content" | |
| 12 | + | "github.com/bnema/dumber/internal/ui/layout" | |
| 12 | 13 | "github.com/bnema/dumber/internal/ui/window" | |
| 13 | 14 | "github.com/bnema/puregotk/v4/gtk" | |
| 14 | 15 | ) | |
| @@ -35,6 +36,19 @@ func destroyFailedNativePopupSetup(popupShell nativePopupDestroyer, wv port.WebV | |||
| 35 | 36 | } | |
| 36 | 37 | } | |
| 37 | 38 | ||
| 39 | + | func prepareNativePopupContentWidget(widget layout.Widget) (*gtk.Widget, error) { | |
| 40 | + | if widget == nil { | |
| 41 | + | return nil, fmt.Errorf("failed to wrap native popup webview widget") | |
| 42 | + | } | |
| 43 | + | gtkWidget := widget.GtkWidget() | |
| 44 | + | if gtkWidget == nil { | |
| 45 | + | return nil, fmt.Errorf("failed to wrap native popup webview widget") | |
| 46 | + | } | |
| 47 | + | widget.SetHexpand(true) | |
| 48 | + | widget.SetVexpand(true) | |
| 49 | + | return gtkWidget, nil | |
| 50 | + | } | |
| 51 | + | ||
| 38 | 52 | func (a *App) ensureNativePopupWindows() { | |
| 39 | 53 | if a.nativePopupWindows == nil { | |
| 40 | 54 | a.nativePopupWindows = make(map[port.WebViewID]*nativePopupWindow) | |
| @@ -58,11 +72,12 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop | |||
| 58 | 72 | return fmt.Errorf("content coordinator not available for native popup") | |
| 59 | 73 | } | |
| 60 | 74 | widget := a.contentCoord.WrapWidget(ctx, input.PopupWebView) | |
| 61 | - | if widget == nil || widget.GtkWidget() == nil { | |
| 75 | + | gtkWidget, err := prepareNativePopupContentWidget(widget) | |
| 76 | + | if err != nil { | |
| 62 | 77 | destroyFailedNativePopupSetup(popupShell, input.PopupWebView) | |
| 63 | - | return fmt.Errorf("failed to wrap native popup webview widget") | |
| 78 | + | return err | |
| 64 | 79 | } | |
| 65 | - | popupShell.SetContent(widget.GtkWidget()) | |
| 80 | + | popupShell.SetContent(gtkWidget) | |
| 66 | 81 | ||
| 67 | 82 | parentWindowID := "" | |
| 68 | 83 | if bw := a.browserWindowForAnyPane(input.ParentPaneID); bw != nil { | |
| @@ -9,6 +9,7 @@ import ( | |||||
| 9 | "github.com/bnema/dumber/internal/domain/entity" | 9 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 10 | "github.com/bnema/dumber/internal/logging" | 10 | "github.com/bnema/dumber/internal/logging" | ||
| 11 | "github.com/bnema/dumber/internal/ui/coordinator/content" | 11 | "github.com/bnema/dumber/internal/ui/coordinator/content" | ||
| 12 | + | "github.com/bnema/dumber/internal/ui/layout" | |||
| 12 | "github.com/bnema/dumber/internal/ui/window" | 13 | "github.com/bnema/dumber/internal/ui/window" | ||
| 13 | "github.com/bnema/puregotk/v4/gtk" | 14 | "github.com/bnema/puregotk/v4/gtk" | ||
| 14 | ) | 15 | ) | ||
| @@ -35,6 +36,19 @@ func destroyFailedNativePopupSetup(popupShell nativePopupDestroyer, wv port.WebV | |||||
| 35 | } | 36 | } | ||
| 36 | } | 37 | } | ||
| 37 | 38 | ||||
| 39 | + | func prepareNativePopupContentWidget(widget layout.Widget) (*gtk.Widget, error) { | |||
| 40 | + | if widget == nil { | |||
| 41 | + | return nil, fmt.Errorf("failed to wrap native popup webview widget") | |||
| 42 | + | } | |||
| 43 | + | gtkWidget := widget.GtkWidget() | |||
| 44 | + | if gtkWidget == nil { | |||
| 45 | + | return nil, fmt.Errorf("failed to wrap native popup webview widget") | |||
| 46 | + | } | |||
| 47 | + | widget.SetHexpand(true) | |||
| 48 | + | widget.SetVexpand(true) | |||
| 49 | + | return gtkWidget, nil | |||
| 50 | + | } | |||
| 51 | + | ||||
| 38 | func (a *App) ensureNativePopupWindows() { | 52 | func (a *App) ensureNativePopupWindows() { | ||
| 39 | if a.nativePopupWindows == nil { | 53 | if a.nativePopupWindows == nil { | ||
| 40 | a.nativePopupWindows = make(map[port.WebViewID]*nativePopupWindow) | 54 | a.nativePopupWindows = make(map[port.WebViewID]*nativePopupWindow) | ||
| @@ -58,11 +72,12 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop | |||||
| 58 | return fmt.Errorf("content coordinator not available for native popup") | 72 | return fmt.Errorf("content coordinator not available for native popup") | ||
| 59 | } | 73 | } | ||
| 60 | widget := a.contentCoord.WrapWidget(ctx, input.PopupWebView) | 74 | widget := a.contentCoord.WrapWidget(ctx, input.PopupWebView) | ||
| 61 | - | if widget == nil || widget.GtkWidget() == nil { | 75 | + | gtkWidget, err := prepareNativePopupContentWidget(widget) |
| 76 | + | if err != nil { | |||
| 62 | destroyFailedNativePopupSetup(popupShell, input.PopupWebView) | 77 | destroyFailedNativePopupSetup(popupShell, input.PopupWebView) | ||
| 63 | - | return fmt.Errorf("failed to wrap native popup webview widget") | 78 | + | return err |
| 64 | } | 79 | } | ||
| 65 | - | popupShell.SetContent(widget.GtkWidget()) | 80 | + | popupShell.SetContent(gtkWidget) |
| 66 | 81 | ||||
| 67 | parentWindowID := "" | 82 | parentWindowID := "" | ||
| 68 | if bw := a.browserWindowForAnyPane(input.ParentPaneID); bw != nil { | 83 | if bw := a.browserWindowForAnyPane(input.ParentPaneID); bw != nil { | ||
M internal/ui/native_popup_window_test.go +24 -0
| @@ -5,7 +5,10 @@ import ( | |||
| 5 | 5 | ||
| 6 | 6 | "github.com/bnema/dumber/internal/application/port" | |
| 7 | 7 | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | |
| 8 | + | layoutmocks "github.com/bnema/dumber/internal/ui/layout/mocks" | |
| 9 | + | "github.com/bnema/puregotk/v4/gtk" | |
| 8 | 10 | "github.com/stretchr/testify/assert" | |
| 11 | + | "github.com/stretchr/testify/require" | |
| 9 | 12 | ) | |
| 10 | 13 | ||
| 11 | 14 | type popupDestroySpy struct { | |
| @@ -16,6 +19,27 @@ func (p *popupDestroySpy) Destroy() { | |||
| 16 | 19 | p.destroyed = true | |
| 17 | 20 | } | |
| 18 | 21 | ||
| 22 | + | func TestPrepareNativePopupContentWidget_ExpandsWrappedWidget(t *testing.T) { | |
| 23 | + | widget := layoutmocks.NewMockWidget(t) | |
| 24 | + | gtkWidget := >k.Widget{} | |
| 25 | + | widget.EXPECT().GtkWidget().Return(gtkWidget).Once() | |
| 26 | + | widget.EXPECT().SetHexpand(true).Once() | |
| 27 | + | widget.EXPECT().SetVexpand(true).Once() | |
| 28 | + | ||
| 29 | + | got, err := prepareNativePopupContentWidget(widget) | |
| 30 | + | require.NoError(t, err) | |
| 31 | + | assert.Same(t, gtkWidget, got) | |
| 32 | + | } | |
| 33 | + | ||
| 34 | + | func TestPrepareNativePopupContentWidget_ErrorsWhenWrappedGTKWidgetMissing(t *testing.T) { | |
| 35 | + | widget := layoutmocks.NewMockWidget(t) | |
| 36 | + | widget.EXPECT().GtkWidget().Return(nil).Once() | |
| 37 | + | ||
| 38 | + | got, err := prepareNativePopupContentWidget(widget) | |
| 39 | + | require.Error(t, err) | |
| 40 | + | assert.Nil(t, got) | |
| 41 | + | } | |
| 42 | + | ||
| 19 | 43 | func TestDestroyFailedNativePopupSetup_DestroysWebViewAndShell(t *testing.T) { | |
| 20 | 44 | wv := portmocks.NewMockWebView(t) | |
| 21 | 45 | wv.EXPECT().IsDestroyed().Return(false).Once() | |
| @@ -5,7 +5,10 @@ import ( | |||||
| 5 | 5 | ||||
| 6 | "github.com/bnema/dumber/internal/application/port" | 6 | "github.com/bnema/dumber/internal/application/port" | ||
| 7 | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | 7 | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | ||
| 8 | + | layoutmocks "github.com/bnema/dumber/internal/ui/layout/mocks" | |||
| 9 | + | "github.com/bnema/puregotk/v4/gtk" | |||
| 8 | "github.com/stretchr/testify/assert" | 10 | "github.com/stretchr/testify/assert" | ||
| 11 | + | "github.com/stretchr/testify/require" | |||
| 9 | ) | 12 | ) | ||
| 10 | 13 | ||||
| 11 | type popupDestroySpy struct { | 14 | type popupDestroySpy struct { | ||
| @@ -16,6 +19,27 @@ func (p *popupDestroySpy) Destroy() { | |||||
| 16 | p.destroyed = true | 19 | p.destroyed = true | ||
| 17 | } | 20 | } | ||
| 18 | 21 | ||||
| 22 | + | func TestPrepareNativePopupContentWidget_ExpandsWrappedWidget(t *testing.T) { | |||
| 23 | + | widget := layoutmocks.NewMockWidget(t) | |||
| 24 | + | gtkWidget := >k.Widget{} | |||
| 25 | + | widget.EXPECT().GtkWidget().Return(gtkWidget).Once() | |||
| 26 | + | widget.EXPECT().SetHexpand(true).Once() | |||
| 27 | + | widget.EXPECT().SetVexpand(true).Once() | |||
| 28 | + | ||||
| 29 | + | got, err := prepareNativePopupContentWidget(widget) | |||
| 30 | + | require.NoError(t, err) | |||
| 31 | + | assert.Same(t, gtkWidget, got) | |||
| 32 | + | } | |||
| 33 | + | ||||
| 34 | + | func TestPrepareNativePopupContentWidget_ErrorsWhenWrappedGTKWidgetMissing(t *testing.T) { | |||
| 35 | + | widget := layoutmocks.NewMockWidget(t) | |||
| 36 | + | widget.EXPECT().GtkWidget().Return(nil).Once() | |||
| 37 | + | ||||
| 38 | + | got, err := prepareNativePopupContentWidget(widget) | |||
| 39 | + | require.Error(t, err) | |||
| 40 | + | assert.Nil(t, got) | |||
| 41 | + | } | |||
| 42 | + | ||||
| 19 | func TestDestroyFailedNativePopupSetup_DestroysWebViewAndShell(t *testing.T) { | 43 | func TestDestroyFailedNativePopupSetup_DestroysWebViewAndShell(t *testing.T) { | ||
| 20 | wv := portmocks.NewMockWebView(t) | 44 | wv := portmocks.NewMockWebView(t) | ||
| 21 | wv.EXPECT().IsDestroyed().Return(false).Once() | 45 | wv.EXPECT().IsDestroyed().Return(false).Once() | ||