fix(ui): preserve revealed webviews across rebuilds
- commit
- 3495a300d03f4924f799a0e0c980f682f370d3f0
- author
- brice · · 2026-07-15 04:13 UTC
- parent
- ec6db2e
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
7 files · +157 -44
M internal/ui/component/pane_view.go +51 -39
| @@ -135,58 +135,70 @@ func (pv *PaneView) WebViewWidget() layout.Widget { | |||
| 135 | 135 | return pv.webViewWidget | |
| 136 | 136 | } | |
| 137 | 137 | ||
| 138 | - | // SetWebViewWidget replaces the WebView widget. | |
| 138 | + | // SetWebViewWidget replaces the WebView widget as not-yet-revealed content. | |
| 139 | + | // Call AttachWebViewWidget when the content coordinator knows the WebView has | |
| 140 | + | // already painted. | |
| 139 | 141 | func (pv *PaneView) SetWebViewWidget(widget layout.Widget) { | |
| 142 | + | pv.AttachWebViewWidget(widget, false) | |
| 143 | + | } | |
| 144 | + | ||
| 145 | + | // AttachWebViewWidget replaces the WebView widget and applies its explicit | |
| 146 | + | // content reveal state. GTK parentage is used only to satisfy GTK's one-parent | |
| 147 | + | // ownership rule; it must not be used to infer whether CEF has painted. | |
| 148 | + | func (pv *PaneView) AttachWebViewWidget(widget layout.Widget, revealed bool) { | |
| 140 | 149 | pv.mu.Lock() | |
| 141 | 150 | defer pv.mu.Unlock() | |
| 142 | 151 | ||
| 143 | - | // Remove old widget from this overlay | |
| 152 | + | // Remove old widget from this overlay. | |
| 144 | 153 | if pv.webViewWidget != nil { | |
| 145 | 154 | pv.overlay.SetChild(nil) | |
| 146 | 155 | } | |
| 147 | 156 | ||
| 148 | 157 | pv.webViewWidget = widget | |
| 158 | + | if widget == nil { | |
| 159 | + | return | |
| 160 | + | } | |
| 149 | 161 | ||
| 150 | - | if widget != nil { | |
| 151 | - | overlayAllocWidthBefore := pv.overlay.GetAllocatedWidth() | |
| 152 | - | overlayAllocHeightBefore := pv.overlay.GetAllocatedHeight() | |
| 153 | - | widgetAllocWidthBefore := widget.GetAllocatedWidth() | |
| 154 | - | widgetAllocHeightBefore := widget.GetAllocatedHeight() | |
| 155 | - | hadParent := widget.GetParent() != nil | |
| 156 | - | wasVisible := widget.IsVisible() | |
| 157 | - | ||
| 158 | - | // If this widget already had a parent, we're reparenting an existing WebView | |
| 159 | - | // (e.g. after rebuilding/moving panes). In that case the WebView has likely | |
| 160 | - | // already painted, so the loading skeleton should be hidden immediately. | |
| 161 | - | ||
| 162 | - | // Unparent widget from any previous parent (critical for rebuild scenarios) | |
| 163 | - | // In GTK4, a widget can only have one parent at a time | |
| 164 | - | if hadParent { | |
| 165 | - | widget.Unparent() | |
| 166 | - | } | |
| 167 | - | pv.overlay.SetChild(widget) | |
| 162 | + | overlayAllocWidthBefore := pv.overlay.GetAllocatedWidth() | |
| 163 | + | overlayAllocHeightBefore := pv.overlay.GetAllocatedHeight() | |
| 164 | + | widgetAllocWidthBefore := widget.GetAllocatedWidth() | |
| 165 | + | widgetAllocHeightBefore := widget.GetAllocatedHeight() | |
| 166 | + | hadParent := widget.GetParent() != nil | |
| 167 | + | wasVisible := widget.IsVisible() | |
| 168 | + | ||
| 169 | + | // GTK4 widgets may have only one parent. Cleanup during a WorkspaceView | |
| 170 | + | // rebuild leaves a reusable native widget unparented, which says nothing | |
| 171 | + | // about whether its content was previously revealed. | |
| 172 | + | if hadParent { | |
| 173 | + | widget.Unparent() | |
| 174 | + | } | |
| 175 | + | pv.overlay.SetChild(widget) | |
| 168 | 176 | ||
| 169 | - | if hadParent && wasVisible { | |
| 170 | - | widget.SetVisible(true) | |
| 171 | - | } | |
| 172 | - | if hadParent && pv.loading != nil { | |
| 173 | - | pv.loading.SetVisible(false) | |
| 174 | - | } | |
| 177 | + | if hadParent && wasVisible { | |
| 178 | + | widget.SetVisible(true) | |
| 179 | + | } | |
| 180 | + | if revealed && pv.loading != nil { | |
| 181 | + | pv.loading.SetVisible(false) | |
| 182 | + | } | |
| 175 | 183 | ||
| 176 | - | logging.FromContext(pv.ctx).Debug(). | |
| 177 | - | Str("pane_id", string(pv.paneID)). | |
| 178 | - | Bool("had_parent", hadParent). | |
| 179 | - | Bool("was_visible", wasVisible). | |
| 180 | - | Int("overlay_alloc_width_before", overlayAllocWidthBefore). | |
| 181 | - | Int("overlay_alloc_height_before", overlayAllocHeightBefore). | |
| 182 | - | Int("widget_alloc_width_before", widgetAllocWidthBefore). | |
| 183 | - | Int("widget_alloc_height_before", widgetAllocHeightBefore). | |
| 184 | - | Int("overlay_alloc_width_after", pv.overlay.GetAllocatedWidth()). | |
| 185 | - | Int("overlay_alloc_height_after", pv.overlay.GetAllocatedHeight()). | |
| 186 | - | Int("widget_alloc_width_after", widget.GetAllocatedWidth()). | |
| 187 | - | Int("widget_alloc_height_after", widget.GetAllocatedHeight()). | |
| 188 | - | Msg("pane view webview widget attached") | |
| 184 | + | ctx := pv.ctx | |
| 185 | + | if ctx == nil { | |
| 186 | + | ctx = context.Background() | |
| 189 | 187 | } | |
| 188 | + | logging.FromContext(ctx).Debug(). | |
| 189 | + | Str("pane_id", string(pv.paneID)). | |
| 190 | + | Bool("had_parent", hadParent). | |
| 191 | + | Bool("was_visible", wasVisible). | |
| 192 | + | Bool("revealed", revealed). | |
| 193 | + | Int("overlay_alloc_width_before", overlayAllocWidthBefore). | |
| 194 | + | Int("overlay_alloc_height_before", overlayAllocHeightBefore). | |
| 195 | + | Int("widget_alloc_width_before", widgetAllocWidthBefore). | |
| 196 | + | Int("widget_alloc_height_before", widgetAllocHeightBefore). | |
| 197 | + | Int("overlay_alloc_width_after", pv.overlay.GetAllocatedWidth()). | |
| 198 | + | Int("overlay_alloc_height_after", pv.overlay.GetAllocatedHeight()). | |
| 199 | + | Int("widget_alloc_width_after", widget.GetAllocatedWidth()). | |
| 200 | + | Int("widget_alloc_height_after", widget.GetAllocatedHeight()). | |
| 201 | + | Msg("pane view webview widget attached") | |
| 190 | 202 | } | |
| 191 | 203 | ||
| 192 | 204 | // GrabFocus attempts to focus the WebView. | |
| @@ -135,58 +135,70 @@ func (pv *PaneView) WebViewWidget() layout.Widget { | |||||
| 135 | return pv.webViewWidget | 135 | return pv.webViewWidget | ||
| 136 | } | 136 | } | ||
| 137 | 137 | ||||
| 138 | - | // SetWebViewWidget replaces the WebView widget. | 138 | + | // SetWebViewWidget replaces the WebView widget as not-yet-revealed content. |
| 139 | + | // Call AttachWebViewWidget when the content coordinator knows the WebView has | |||
| 140 | + | // already painted. | |||
| 139 | func (pv *PaneView) SetWebViewWidget(widget layout.Widget) { | 141 | func (pv *PaneView) SetWebViewWidget(widget layout.Widget) { | ||
| 142 | + | pv.AttachWebViewWidget(widget, false) | |||
| 143 | + | } | |||
| 144 | + | ||||
| 145 | + | // AttachWebViewWidget replaces the WebView widget and applies its explicit | |||
| 146 | + | // content reveal state. GTK parentage is used only to satisfy GTK's one-parent | |||
| 147 | + | // ownership rule; it must not be used to infer whether CEF has painted. | |||
| 148 | + | func (pv *PaneView) AttachWebViewWidget(widget layout.Widget, revealed bool) { | |||
| 140 | pv.mu.Lock() | 149 | pv.mu.Lock() | ||
| 141 | defer pv.mu.Unlock() | 150 | defer pv.mu.Unlock() | ||
| 142 | 151 | ||||
| 143 | - | // Remove old widget from this overlay | 152 | + | // Remove old widget from this overlay. |
| 144 | if pv.webViewWidget != nil { | 153 | if pv.webViewWidget != nil { | ||
| 145 | pv.overlay.SetChild(nil) | 154 | pv.overlay.SetChild(nil) | ||
| 146 | } | 155 | } | ||
| 147 | 156 | ||||
| 148 | pv.webViewWidget = widget | 157 | pv.webViewWidget = widget | ||
| 158 | + | if widget == nil { | |||
| 159 | + | return | |||
| 160 | + | } | |||
| 149 | 161 | ||||
| 150 | - | if widget != nil { | 162 | + | overlayAllocWidthBefore := pv.overlay.GetAllocatedWidth() |
| 151 | - | overlayAllocWidthBefore := pv.overlay.GetAllocatedWidth() | 163 | + | overlayAllocHeightBefore := pv.overlay.GetAllocatedHeight() |
| 152 | - | overlayAllocHeightBefore := pv.overlay.GetAllocatedHeight() | 164 | + | widgetAllocWidthBefore := widget.GetAllocatedWidth() |
| 153 | - | widgetAllocWidthBefore := widget.GetAllocatedWidth() | 165 | + | widgetAllocHeightBefore := widget.GetAllocatedHeight() |
| 154 | - | widgetAllocHeightBefore := widget.GetAllocatedHeight() | 166 | + | hadParent := widget.GetParent() != nil |
| 155 | - | hadParent := widget.GetParent() != nil | 167 | + | wasVisible := widget.IsVisible() |
| 156 | - | wasVisible := widget.IsVisible() | 168 | + | |
| 157 | - | 169 | + | // GTK4 widgets may have only one parent. Cleanup during a WorkspaceView | |
| 158 | - | // If this widget already had a parent, we're reparenting an existing WebView | 170 | + | // rebuild leaves a reusable native widget unparented, which says nothing |
| 159 | - | // (e.g. after rebuilding/moving panes). In that case the WebView has likely | 171 | + | // about whether its content was previously revealed. |
| 160 | - | // already painted, so the loading skeleton should be hidden immediately. | 172 | + | if hadParent { |
| 161 | - | 173 | + | widget.Unparent() | |
| 162 | - | // Unparent widget from any previous parent (critical for rebuild scenarios) | 174 | + | } |
| 163 | - | // In GTK4, a widget can only have one parent at a time | 175 | + | pv.overlay.SetChild(widget) |
| 164 | - | if hadParent { | |||
| 165 | - | widget.Unparent() | |||
| 166 | - | } | |||
| 167 | - | pv.overlay.SetChild(widget) | |||
| 168 | 176 | ||||
| 169 | - | if hadParent && wasVisible { | 177 | + | if hadParent && wasVisible { |
| 170 | - | widget.SetVisible(true) | 178 | + | widget.SetVisible(true) |
| 171 | - | } | 179 | + | } |
| 172 | - | if hadParent && pv.loading != nil { | 180 | + | if revealed && pv.loading != nil { |
| 173 | - | pv.loading.SetVisible(false) | 181 | + | pv.loading.SetVisible(false) |
| 174 | - | } | 182 | + | } |
| 175 | 183 | ||||
| 176 | - | logging.FromContext(pv.ctx).Debug(). | 184 | + | ctx := pv.ctx |
| 177 | - | Str("pane_id", string(pv.paneID)). | 185 | + | if ctx == nil { |
| 178 | - | Bool("had_parent", hadParent). | 186 | + | ctx = context.Background() |
| 179 | - | Bool("was_visible", wasVisible). | |||
| 180 | - | Int("overlay_alloc_width_before", overlayAllocWidthBefore). | |||
| 181 | - | Int("overlay_alloc_height_before", overlayAllocHeightBefore). | |||
| 182 | - | Int("widget_alloc_width_before", widgetAllocWidthBefore). | |||
| 183 | - | Int("widget_alloc_height_before", widgetAllocHeightBefore). | |||
| 184 | - | Int("overlay_alloc_width_after", pv.overlay.GetAllocatedWidth()). | |||
| 185 | - | Int("overlay_alloc_height_after", pv.overlay.GetAllocatedHeight()). | |||
| 186 | - | Int("widget_alloc_width_after", widget.GetAllocatedWidth()). | |||
| 187 | - | Int("widget_alloc_height_after", widget.GetAllocatedHeight()). | |||
| 188 | - | Msg("pane view webview widget attached") | |||
| 189 | } | 187 | } | ||
| 188 | + | logging.FromContext(ctx).Debug(). | |||
| 189 | + | Str("pane_id", string(pv.paneID)). | |||
| 190 | + | Bool("had_parent", hadParent). | |||
| 191 | + | Bool("was_visible", wasVisible). | |||
| 192 | + | Bool("revealed", revealed). | |||
| 193 | + | Int("overlay_alloc_width_before", overlayAllocWidthBefore). | |||
| 194 | + | Int("overlay_alloc_height_before", overlayAllocHeightBefore). | |||
| 195 | + | Int("widget_alloc_width_before", widgetAllocWidthBefore). | |||
| 196 | + | Int("widget_alloc_height_before", widgetAllocHeightBefore). | |||
| 197 | + | Int("overlay_alloc_width_after", pv.overlay.GetAllocatedWidth()). | |||
| 198 | + | Int("overlay_alloc_height_after", pv.overlay.GetAllocatedHeight()). | |||
| 199 | + | Int("widget_alloc_width_after", widget.GetAllocatedWidth()). | |||
| 200 | + | Int("widget_alloc_height_after", widget.GetAllocatedHeight()). | |||
| 201 | + | Msg("pane view webview widget attached") | |||
| 190 | } | 202 | } | ||
| 191 | 203 | ||||
| 192 | // GrabFocus attempts to focus the WebView. | 204 | // GrabFocus attempts to focus the WebView. | ||
M internal/ui/component/pane_view_internal_test.go +46 -0
| @@ -80,3 +80,49 @@ func TestPaneView_HideLoadingSkeleton_NoOpWhenNil(t *testing.T) { | |||
| 80 | 80 | pv := &PaneView{} | |
| 81 | 81 | pv.HideLoadingSkeleton() | |
| 82 | 82 | } | |
| 83 | + | ||
| 84 | + | func TestPaneView_AttachWebViewWidget_UsesExplicitRevealStateAfterCleanup(t *testing.T) { | |
| 85 | + | widget := mocks.NewMockWidget(t) | |
| 86 | + | oldOverlay := mocks.NewMockOverlayWidget(t) | |
| 87 | + | oldOverlay.EXPECT().SetChild(nil).Once() | |
| 88 | + | (&PaneView{overlay: oldOverlay, webViewWidget: widget}).Cleanup() | |
| 89 | + | ||
| 90 | + | t.Run("revealed widget detached by cleanup hides fresh skeleton", func(t *testing.T) { | |
| 91 | + | overlay := mocks.NewMockOverlayWidget(t) | |
| 92 | + | loadingContainer := mocks.NewMockBoxWidget(t) | |
| 93 | + | spinner := mocks.NewMockSpinnerWidget(t) | |
| 94 | + | overlay.EXPECT().GetAllocatedWidth().Return(0).Twice() | |
| 95 | + | overlay.EXPECT().GetAllocatedHeight().Return(0).Twice() | |
| 96 | + | widget.EXPECT().GetAllocatedWidth().Return(0).Twice() | |
| 97 | + | widget.EXPECT().GetAllocatedHeight().Return(0).Twice() | |
| 98 | + | widget.EXPECT().GetParent().Return(nil).Once() | |
| 99 | + | widget.EXPECT().IsVisible().Return(true).Once() | |
| 100 | + | overlay.EXPECT().SetChild(widget).Once() | |
| 101 | + | loadingContainer.EXPECT().SetVisible(false).Once() | |
| 102 | + | spinner.EXPECT().Stop().Once() | |
| 103 | + | ||
| 104 | + | pv := &PaneView{ | |
| 105 | + | overlay: overlay, | |
| 106 | + | loading: &LoadingSkeleton{container: loadingContainer, spinner: spinner}, | |
| 107 | + | } | |
| 108 | + | pv.AttachWebViewWidget(widget, true) | |
| 109 | + | }) | |
| 110 | + | ||
| 111 | + | t.Run("new unrevealed widget keeps fresh skeleton visible", func(t *testing.T) { | |
| 112 | + | overlay := mocks.NewMockOverlayWidget(t) | |
| 113 | + | loadingContainer := mocks.NewMockBoxWidget(t) | |
| 114 | + | overlay.EXPECT().GetAllocatedWidth().Return(0).Twice() | |
| 115 | + | overlay.EXPECT().GetAllocatedHeight().Return(0).Twice() | |
| 116 | + | widget.EXPECT().GetAllocatedWidth().Return(0).Twice() | |
| 117 | + | widget.EXPECT().GetAllocatedHeight().Return(0).Twice() | |
| 118 | + | widget.EXPECT().GetParent().Return(nil).Once() | |
| 119 | + | widget.EXPECT().IsVisible().Return(true).Once() | |
| 120 | + | overlay.EXPECT().SetChild(widget).Once() | |
| 121 | + | ||
| 122 | + | pv := &PaneView{ | |
| 123 | + | overlay: overlay, | |
| 124 | + | loading: &LoadingSkeleton{container: loadingContainer}, | |
| 125 | + | } | |
| 126 | + | pv.AttachWebViewWidget(widget, false) | |
| 127 | + | }) | |
| 128 | + | } | |
| @@ -80,3 +80,49 @@ func TestPaneView_HideLoadingSkeleton_NoOpWhenNil(t *testing.T) { | |||||
| 80 | pv := &PaneView{} | 80 | pv := &PaneView{} | ||
| 81 | pv.HideLoadingSkeleton() | 81 | pv.HideLoadingSkeleton() | ||
| 82 | } | 82 | } | ||
| 83 | + | ||||
| 84 | + | func TestPaneView_AttachWebViewWidget_UsesExplicitRevealStateAfterCleanup(t *testing.T) { | |||
| 85 | + | widget := mocks.NewMockWidget(t) | |||
| 86 | + | oldOverlay := mocks.NewMockOverlayWidget(t) | |||
| 87 | + | oldOverlay.EXPECT().SetChild(nil).Once() | |||
| 88 | + | (&PaneView{overlay: oldOverlay, webViewWidget: widget}).Cleanup() | |||
| 89 | + | ||||
| 90 | + | t.Run("revealed widget detached by cleanup hides fresh skeleton", func(t *testing.T) { | |||
| 91 | + | overlay := mocks.NewMockOverlayWidget(t) | |||
| 92 | + | loadingContainer := mocks.NewMockBoxWidget(t) | |||
| 93 | + | spinner := mocks.NewMockSpinnerWidget(t) | |||
| 94 | + | overlay.EXPECT().GetAllocatedWidth().Return(0).Twice() | |||
| 95 | + | overlay.EXPECT().GetAllocatedHeight().Return(0).Twice() | |||
| 96 | + | widget.EXPECT().GetAllocatedWidth().Return(0).Twice() | |||
| 97 | + | widget.EXPECT().GetAllocatedHeight().Return(0).Twice() | |||
| 98 | + | widget.EXPECT().GetParent().Return(nil).Once() | |||
| 99 | + | widget.EXPECT().IsVisible().Return(true).Once() | |||
| 100 | + | overlay.EXPECT().SetChild(widget).Once() | |||
| 101 | + | loadingContainer.EXPECT().SetVisible(false).Once() | |||
| 102 | + | spinner.EXPECT().Stop().Once() | |||
| 103 | + | ||||
| 104 | + | pv := &PaneView{ | |||
| 105 | + | overlay: overlay, | |||
| 106 | + | loading: &LoadingSkeleton{container: loadingContainer, spinner: spinner}, | |||
| 107 | + | } | |||
| 108 | + | pv.AttachWebViewWidget(widget, true) | |||
| 109 | + | }) | |||
| 110 | + | ||||
| 111 | + | t.Run("new unrevealed widget keeps fresh skeleton visible", func(t *testing.T) { | |||
| 112 | + | overlay := mocks.NewMockOverlayWidget(t) | |||
| 113 | + | loadingContainer := mocks.NewMockBoxWidget(t) | |||
| 114 | + | overlay.EXPECT().GetAllocatedWidth().Return(0).Twice() | |||
| 115 | + | overlay.EXPECT().GetAllocatedHeight().Return(0).Twice() | |||
| 116 | + | widget.EXPECT().GetAllocatedWidth().Return(0).Twice() | |||
| 117 | + | widget.EXPECT().GetAllocatedHeight().Return(0).Twice() | |||
| 118 | + | widget.EXPECT().GetParent().Return(nil).Once() | |||
| 119 | + | widget.EXPECT().IsVisible().Return(true).Once() | |||
| 120 | + | overlay.EXPECT().SetChild(widget).Once() | |||
| 121 | + | ||||
| 122 | + | pv := &PaneView{ | |||
| 123 | + | overlay: overlay, | |||
| 124 | + | loading: &LoadingSkeleton{container: loadingContainer}, | |||
| 125 | + | } | |||
| 126 | + | pv.AttachWebViewWidget(widget, false) | |||
| 127 | + | }) | |||
| 128 | + | } | |||
M internal/ui/component/workspace_view.go +10 -2
| @@ -482,8 +482,16 @@ func (wv *WorkspaceView) DeactivatePane(paneID entity.PaneID) { | |||
| 482 | 482 | } | |
| 483 | 483 | } | |
| 484 | 484 | ||
| 485 | - | // SetWebViewWidget attaches a WebView widget to a specific pane. | |
| 485 | + | // SetWebViewWidget attaches a not-yet-revealed WebView widget to a specific pane. | |
| 486 | + | // Call AttachWebViewWidget when the content coordinator has explicit reveal state. | |
| 486 | 487 | func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Widget) error { | |
| 488 | + | return wv.AttachWebViewWidget(paneID, widget, false) | |
| 489 | + | } | |
| 490 | + | ||
| 491 | + | // AttachWebViewWidget attaches a WebView widget with the content coordinator's | |
| 492 | + | // explicit reveal state. A WorkspaceView rebuild can unparent a live widget, so | |
| 493 | + | // widget parentage cannot determine whether its loading skeleton is still needed. | |
| 494 | + | func (wv *WorkspaceView) AttachWebViewWidget(paneID entity.PaneID, widget layout.Widget, revealed bool) error { | |
| 487 | 495 | wv.mu.RLock() | |
| 488 | 496 | pv, ok := wv.paneViews[paneID] | |
| 489 | 497 | callback := wv.onWebViewAttached | |
| @@ -493,7 +501,7 @@ func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Wi | |||
| 493 | 501 | return ErrPaneNotFound | |
| 494 | 502 | } | |
| 495 | 503 | ||
| 496 | - | pv.SetWebViewWidget(widget) | |
| 504 | + | pv.AttachWebViewWidget(widget, revealed) | |
| 497 | 505 | if widget != nil && callback != nil { | |
| 498 | 506 | callback(paneID) | |
| 499 | 507 | } | |
| @@ -482,8 +482,16 @@ func (wv *WorkspaceView) DeactivatePane(paneID entity.PaneID) { | |||||
| 482 | } | 482 | } | ||
| 483 | } | 483 | } | ||
| 484 | 484 | ||||
| 485 | - | // SetWebViewWidget attaches a WebView widget to a specific pane. | 485 | + | // SetWebViewWidget attaches a not-yet-revealed WebView widget to a specific pane. |
| 486 | + | // Call AttachWebViewWidget when the content coordinator has explicit reveal state. | |||
| 486 | func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Widget) error { | 487 | func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Widget) error { | ||
| 488 | + | return wv.AttachWebViewWidget(paneID, widget, false) | |||
| 489 | + | } | |||
| 490 | + | ||||
| 491 | + | // AttachWebViewWidget attaches a WebView widget with the content coordinator's | |||
| 492 | + | // explicit reveal state. A WorkspaceView rebuild can unparent a live widget, so | |||
| 493 | + | // widget parentage cannot determine whether its loading skeleton is still needed. | |||
| 494 | + | func (wv *WorkspaceView) AttachWebViewWidget(paneID entity.PaneID, widget layout.Widget, revealed bool) error { | |||
| 487 | wv.mu.RLock() | 495 | wv.mu.RLock() | ||
| 488 | pv, ok := wv.paneViews[paneID] | 496 | pv, ok := wv.paneViews[paneID] | ||
| 489 | callback := wv.onWebViewAttached | 497 | callback := wv.onWebViewAttached | ||
| @@ -493,7 +501,7 @@ func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Wi | |||||
| 493 | return ErrPaneNotFound | 501 | return ErrPaneNotFound | ||
| 494 | } | 502 | } | ||
| 495 | 503 | ||||
| 496 | - | pv.SetWebViewWidget(widget) | 504 | + | pv.AttachWebViewWidget(widget, revealed) |
| 497 | if widget != nil && callback != nil { | 505 | if widget != nil && callback != nil { | ||
| 498 | callback(paneID) | 506 | callback(paneID) | ||
| 499 | } | 507 | } | ||
M internal/ui/coordinator/content/content_reveal_test.go +13 -0
| @@ -17,6 +17,19 @@ import ( | |||
| 17 | 17 | // These are internal package tests that verify the state transitions | |
| 18 | 18 | // without needing to mock WebKit dependencies. | |
| 19 | 19 | ||
| 20 | + | func TestWebViewRevealed_PersistsUntilWebViewRelease(t *testing.T) { | |
| 21 | + | paneID := entity.PaneID("pane-1") | |
| 22 | + | c := &Coordinator{} | |
| 23 | + | ||
| 24 | + | assert.False(t, c.WebViewRevealed(paneID)) | |
| 25 | + | ||
| 26 | + | c.markWebViewRevealed(paneID) | |
| 27 | + | assert.True(t, c.WebViewRevealed(paneID)) | |
| 28 | + | ||
| 29 | + | c.clearWebViewRevealed(paneID) | |
| 30 | + | assert.False(t, c.WebViewRevealed(paneID)) | |
| 31 | + | } | |
| 32 | + | ||
| 20 | 33 | func TestMarkPendingReveal_SetsState(t *testing.T) { | |
| 21 | 34 | c := &Coordinator{ | |
| 22 | 35 | pendingReveal: make(map[entity.PaneID]bool), | |
| @@ -17,6 +17,19 @@ import ( | |||||
| 17 | // These are internal package tests that verify the state transitions | 17 | // These are internal package tests that verify the state transitions | ||
| 18 | // without needing to mock WebKit dependencies. | 18 | // without needing to mock WebKit dependencies. | ||
| 19 | 19 | ||||
| 20 | + | func TestWebViewRevealed_PersistsUntilWebViewRelease(t *testing.T) { | |||
| 21 | + | paneID := entity.PaneID("pane-1") | |||
| 22 | + | c := &Coordinator{} | |||
| 23 | + | ||||
| 24 | + | assert.False(t, c.WebViewRevealed(paneID)) | |||
| 25 | + | ||||
| 26 | + | c.markWebViewRevealed(paneID) | |||
| 27 | + | assert.True(t, c.WebViewRevealed(paneID)) | |||
| 28 | + | ||||
| 29 | + | c.clearWebViewRevealed(paneID) | |||
| 30 | + | assert.False(t, c.WebViewRevealed(paneID)) | |||
| 31 | + | } | |||
| 32 | + | ||||
| 20 | func TestMarkPendingReveal_SetsState(t *testing.T) { | 33 | func TestMarkPendingReveal_SetsState(t *testing.T) { | ||
| 21 | c := &Coordinator{ | 34 | c := &Coordinator{ | ||
| 22 | pendingReveal: make(map[entity.PaneID]bool), | 35 | pendingReveal: make(map[entity.PaneID]bool), | ||
M internal/ui/coordinator/content/coordinator.go +4 -2
| @@ -70,8 +70,9 @@ type Coordinator struct { | |||
| 70 | 70 | // Callback when the WebView becomes visible (first real commit) | |
| 71 | 71 | onWebViewShown func(paneID entity.PaneID) | |
| 72 | 72 | ||
| 73 | - | revealMu sync.Mutex | |
| 74 | - | pendingReveal map[entity.PaneID]bool | |
| 73 | + | revealMu sync.Mutex | |
| 74 | + | pendingReveal map[entity.PaneID]bool | |
| 75 | + | revealedWebViews map[entity.PaneID]bool | |
| 75 | 76 | ||
| 76 | 77 | appearanceMu sync.Mutex | |
| 77 | 78 | pendingScriptRefresh map[entity.PaneID]bool | |
| @@ -148,6 +149,7 @@ func NewCoordinator( | |||
| 148 | 149 | paneTitles: make(map[entity.PaneID]string), | |
| 149 | 150 | navOrigins: make(map[entity.PaneID]string), | |
| 150 | 151 | pendingReveal: make(map[entity.PaneID]bool), | |
| 152 | + | revealedWebViews: make(map[entity.PaneID]bool), | |
| 151 | 153 | pendingScriptRefresh: make(map[entity.PaneID]bool), | |
| 152 | 154 | pendingThemePanes: make(map[entity.PaneID]bool), | |
| 153 | 155 | getActiveWS: getActiveWS, | |
| @@ -70,8 +70,9 @@ type Coordinator struct { | |||||
| 70 | // Callback when the WebView becomes visible (first real commit) | 70 | // Callback when the WebView becomes visible (first real commit) | ||
| 71 | onWebViewShown func(paneID entity.PaneID) | 71 | onWebViewShown func(paneID entity.PaneID) | ||
| 72 | 72 | ||||
| 73 | - | revealMu sync.Mutex | 73 | + | revealMu sync.Mutex |
| 74 | - | pendingReveal map[entity.PaneID]bool | 74 | + | pendingReveal map[entity.PaneID]bool |
| 75 | + | revealedWebViews map[entity.PaneID]bool | |||
| 75 | 76 | ||||
| 76 | appearanceMu sync.Mutex | 77 | appearanceMu sync.Mutex | ||
| 77 | pendingScriptRefresh map[entity.PaneID]bool | 78 | pendingScriptRefresh map[entity.PaneID]bool | ||
| @@ -148,6 +149,7 @@ func NewCoordinator( | |||||
| 148 | paneTitles: make(map[entity.PaneID]string), | 149 | paneTitles: make(map[entity.PaneID]string), | ||
| 149 | navOrigins: make(map[entity.PaneID]string), | 150 | navOrigins: make(map[entity.PaneID]string), | ||
| 150 | pendingReveal: make(map[entity.PaneID]bool), | 151 | pendingReveal: make(map[entity.PaneID]bool), | ||
| 152 | + | revealedWebViews: make(map[entity.PaneID]bool), | |||
| 151 | pendingScriptRefresh: make(map[entity.PaneID]bool), | 153 | pendingScriptRefresh: make(map[entity.PaneID]bool), | ||
| 152 | pendingThemePanes: make(map[entity.PaneID]bool), | 154 | pendingThemePanes: make(map[entity.PaneID]bool), | ||
| 153 | getActiveWS: getActiveWS, | 155 | getActiveWS: getActiveWS, | ||
M internal/ui/coordinator/content/lifecycle.go +5 -1
| @@ -39,6 +39,9 @@ func (c *Coordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) ( | |||
| 39 | 39 | } | |
| 40 | 40 | logging.Trace().Mark("webview_acquired") | |
| 41 | 41 | ||
| 42 | + | // A pooled widget may have been previously revealed for another pane. Its | |
| 43 | + | // presentation lifecycle starts unrevealed for this pane until a new reveal. | |
| 44 | + | c.clearWebViewRevealed(paneID) | |
| 42 | 45 | c.setWebViewLocked(paneID, wv) | |
| 43 | 46 | c.setupWebViewCallbacks(ctx, paneID, wv) | |
| 44 | 47 | ||
| @@ -60,6 +63,7 @@ func (c *Coordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) | |||
| 60 | 63 | c.ensurePopupManager().clearReusableNamedPopupByPaneID(paneID) | |
| 61 | 64 | c.ensurePopupManager().clearReusableNamedPopupByWebViewID(wv.ID()) | |
| 62 | 65 | c.clearPendingAppearance(paneID) | |
| 66 | + | c.clearWebViewRevealed(paneID) | |
| 63 | 67 | ||
| 64 | 68 | // CRITICAL: If this webview was inhibiting idle (fullscreen or audio playing), | |
| 65 | 69 | // we must release the inhibition before destroying the webview. | |
| @@ -136,7 +140,7 @@ func (c *Coordinator) AttachToWorkspace(ctx context.Context, ws *entity.Workspac | |||
| 136 | 140 | Int("wrapped_widget_alloc_height", widget.GetAllocatedHeight()). | |
| 137 | 141 | Msg("wrapped webview widget prepared for pane") | |
| 138 | 142 | ||
| 139 | - | if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil { | |
| 143 | + | if err := wsView.AttachWebViewWidget(pane.ID, widget, c.WebViewRevealed(pane.ID)); err != nil { | |
| 140 | 144 | log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("failed to attach webview widget") | |
| 141 | 145 | continue | |
| 142 | 146 | } | |
| @@ -39,6 +39,9 @@ func (c *Coordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) ( | |||||
| 39 | } | 39 | } | ||
| 40 | logging.Trace().Mark("webview_acquired") | 40 | logging.Trace().Mark("webview_acquired") | ||
| 41 | 41 | ||||
| 42 | + | // A pooled widget may have been previously revealed for another pane. Its | |||
| 43 | + | // presentation lifecycle starts unrevealed for this pane until a new reveal. | |||
| 44 | + | c.clearWebViewRevealed(paneID) | |||
| 42 | c.setWebViewLocked(paneID, wv) | 45 | c.setWebViewLocked(paneID, wv) | ||
| 43 | c.setupWebViewCallbacks(ctx, paneID, wv) | 46 | c.setupWebViewCallbacks(ctx, paneID, wv) | ||
| 44 | 47 | ||||
| @@ -60,6 +63,7 @@ func (c *Coordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) | |||||
| 60 | c.ensurePopupManager().clearReusableNamedPopupByPaneID(paneID) | 63 | c.ensurePopupManager().clearReusableNamedPopupByPaneID(paneID) | ||
| 61 | c.ensurePopupManager().clearReusableNamedPopupByWebViewID(wv.ID()) | 64 | c.ensurePopupManager().clearReusableNamedPopupByWebViewID(wv.ID()) | ||
| 62 | c.clearPendingAppearance(paneID) | 65 | c.clearPendingAppearance(paneID) | ||
| 66 | + | c.clearWebViewRevealed(paneID) | |||
| 63 | 67 | ||||
| 64 | // CRITICAL: If this webview was inhibiting idle (fullscreen or audio playing), | 68 | // CRITICAL: If this webview was inhibiting idle (fullscreen or audio playing), | ||
| 65 | // we must release the inhibition before destroying the webview. | 69 | // we must release the inhibition before destroying the webview. | ||
| @@ -136,7 +140,7 @@ func (c *Coordinator) AttachToWorkspace(ctx context.Context, ws *entity.Workspac | |||||
| 136 | Int("wrapped_widget_alloc_height", widget.GetAllocatedHeight()). | 140 | Int("wrapped_widget_alloc_height", widget.GetAllocatedHeight()). | ||
| 137 | Msg("wrapped webview widget prepared for pane") | 141 | Msg("wrapped webview widget prepared for pane") | ||
| 138 | 142 | ||||
| 139 | - | if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil { | 143 | + | if err := wsView.AttachWebViewWidget(pane.ID, widget, c.WebViewRevealed(pane.ID)); err != nil { |
| 140 | log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("failed to attach webview widget") | 144 | log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("failed to attach webview widget") | ||
| 141 | continue | 145 | continue | ||
| 142 | } | 146 | } | ||
M internal/ui/coordinator/content/navigation.go +28 -0
| @@ -256,6 +256,30 @@ func (c *Coordinator) clearPendingReveal(paneID entity.PaneID) { | |||
| 256 | 256 | c.revealMu.Unlock() | |
| 257 | 257 | } | |
| 258 | 258 | ||
| 259 | + | // WebViewRevealed reports whether this pane's current WebView has completed a | |
| 260 | + | // reveal. This state belongs to content lifecycle rather than GTK widget | |
| 261 | + | // ownership, because WorkspaceView rebuilds unparent live native widgets. | |
| 262 | + | func (c *Coordinator) WebViewRevealed(paneID entity.PaneID) bool { | |
| 263 | + | c.revealMu.Lock() | |
| 264 | + | defer c.revealMu.Unlock() | |
| 265 | + | return c.revealedWebViews[paneID] | |
| 266 | + | } | |
| 267 | + | ||
| 268 | + | func (c *Coordinator) markWebViewRevealed(paneID entity.PaneID) { | |
| 269 | + | c.revealMu.Lock() | |
| 270 | + | if c.revealedWebViews == nil { | |
| 271 | + | c.revealedWebViews = make(map[entity.PaneID]bool) | |
| 272 | + | } | |
| 273 | + | c.revealedWebViews[paneID] = true | |
| 274 | + | c.revealMu.Unlock() | |
| 275 | + | } | |
| 276 | + | ||
| 277 | + | func (c *Coordinator) clearWebViewRevealed(paneID entity.PaneID) { | |
| 278 | + | c.revealMu.Lock() | |
| 279 | + | delete(c.revealedWebViews, paneID) | |
| 280 | + | c.revealMu.Unlock() | |
| 281 | + | } | |
| 282 | + | ||
| 259 | 283 | func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, uri, reason string) { | |
| 260 | 284 | c.revealMu.Lock() | |
| 261 | 285 | pending := c.pendingReveal[paneID] | |
| @@ -288,6 +312,10 @@ func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, | |||
| 288 | 312 | } | |
| 289 | 313 | } | |
| 290 | 314 | ||
| 315 | + | // Record presentation state before notifying the UI. The callback may run | |
| 316 | + | // while a WorkspaceView is being rebuilt, and the next PaneView must inherit | |
| 317 | + | // this state even though the native widget was unparented during cleanup. | |
| 318 | + | c.markWebViewRevealed(paneID) | |
| 291 | 319 | if c.onWebViewShown != nil { | |
| 292 | 320 | c.onWebViewShown(paneID) | |
| 293 | 321 | } | |
| @@ -256,6 +256,30 @@ func (c *Coordinator) clearPendingReveal(paneID entity.PaneID) { | |||||
| 256 | c.revealMu.Unlock() | 256 | c.revealMu.Unlock() | ||
| 257 | } | 257 | } | ||
| 258 | 258 | ||||
| 259 | + | // WebViewRevealed reports whether this pane's current WebView has completed a | |||
| 260 | + | // reveal. This state belongs to content lifecycle rather than GTK widget | |||
| 261 | + | // ownership, because WorkspaceView rebuilds unparent live native widgets. | |||
| 262 | + | func (c *Coordinator) WebViewRevealed(paneID entity.PaneID) bool { | |||
| 263 | + | c.revealMu.Lock() | |||
| 264 | + | defer c.revealMu.Unlock() | |||
| 265 | + | return c.revealedWebViews[paneID] | |||
| 266 | + | } | |||
| 267 | + | ||||
| 268 | + | func (c *Coordinator) markWebViewRevealed(paneID entity.PaneID) { | |||
| 269 | + | c.revealMu.Lock() | |||
| 270 | + | if c.revealedWebViews == nil { | |||
| 271 | + | c.revealedWebViews = make(map[entity.PaneID]bool) | |||
| 272 | + | } | |||
| 273 | + | c.revealedWebViews[paneID] = true | |||
| 274 | + | c.revealMu.Unlock() | |||
| 275 | + | } | |||
| 276 | + | ||||
| 277 | + | func (c *Coordinator) clearWebViewRevealed(paneID entity.PaneID) { | |||
| 278 | + | c.revealMu.Lock() | |||
| 279 | + | delete(c.revealedWebViews, paneID) | |||
| 280 | + | c.revealMu.Unlock() | |||
| 281 | + | } | |||
| 282 | + | ||||
| 259 | func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, uri, reason string) { | 283 | func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, uri, reason string) { | ||
| 260 | c.revealMu.Lock() | 284 | c.revealMu.Lock() | ||
| 261 | pending := c.pendingReveal[paneID] | 285 | pending := c.pendingReveal[paneID] | ||
| @@ -288,6 +312,10 @@ func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, | |||||
| 288 | } | 312 | } | ||
| 289 | } | 313 | } | ||
| 290 | 314 | ||||
| 315 | + | // Record presentation state before notifying the UI. The callback may run | |||
| 316 | + | // while a WorkspaceView is being rebuilt, and the next PaneView must inherit | |||
| 317 | + | // this state even though the native widget was unparented during cleanup. | |||
| 318 | + | c.markWebViewRevealed(paneID) | |||
| 291 | if c.onWebViewShown != nil { | 319 | if c.onWebViewShown != nil { | ||
| 292 | c.onWebViewShown(paneID) | 320 | c.onWebViewShown(paneID) | ||
| 293 | } | 321 | } | ||