fix: reveal webviews after paint starts
- commit
- e0c6274be7627a2dadd97939c62928cfaf26242e
- author
- bnema · · 2025-12-28 11:59 UTC
- committer
- bnema · · 2025-12-28 11:59 UTC
- parent
- 79b3471
- signature
- GPG Unverified
2B82207D370127A0The key is unknown
3 files · +64 -9
M internal/ui/component/pane_view.go +7 -1
| @@ -148,15 +148,21 @@ func (pv *PaneView) SetWebViewWidget(widget layout.Widget) { | |||
| 148 | 148 | // (e.g. after rebuilding/moving panes). In that case the WebView has likely | |
| 149 | 149 | // already painted, so the loading skeleton should be hidden immediately. | |
| 150 | 150 | hadParent := widget.GetParent() != nil | |
| 151 | + | wasVisible := false | |
| 152 | + | if hadParent { | |
| 153 | + | wasVisible = widget.IsVisible() | |
| 154 | + | } | |
| 151 | 155 | ||
| 152 | 156 | // Unparent widget from any previous parent (critical for rebuild scenarios) | |
| 153 | 157 | // In GTK4, a widget can only have one parent at a time | |
| 154 | 158 | if hadParent { | |
| 155 | 159 | widget.Unparent() | |
| 156 | 160 | } | |
| 157 | - | widget.SetVisible(true) | |
| 158 | 161 | pv.overlay.SetChild(widget) | |
| 159 | 162 | ||
| 163 | + | if hadParent && wasVisible { | |
| 164 | + | widget.SetVisible(true) | |
| 165 | + | } | |
| 160 | 166 | if hadParent && pv.loading != nil { | |
| 161 | 167 | pv.loading.SetVisible(false) | |
| 162 | 168 | } | |
| @@ -148,15 +148,21 @@ func (pv *PaneView) SetWebViewWidget(widget layout.Widget) { | |||||
| 148 | // (e.g. after rebuilding/moving panes). In that case the WebView has likely | 148 | // (e.g. after rebuilding/moving panes). In that case the WebView has likely | ||
| 149 | // already painted, so the loading skeleton should be hidden immediately. | 149 | // already painted, so the loading skeleton should be hidden immediately. | ||
| 150 | hadParent := widget.GetParent() != nil | 150 | hadParent := widget.GetParent() != nil | ||
| 151 | + | wasVisible := false | |||
| 152 | + | if hadParent { | |||
| 153 | + | wasVisible = widget.IsVisible() | |||
| 154 | + | } | |||
| 151 | 155 | ||||
| 152 | // Unparent widget from any previous parent (critical for rebuild scenarios) | 156 | // Unparent widget from any previous parent (critical for rebuild scenarios) | ||
| 153 | // In GTK4, a widget can only have one parent at a time | 157 | // In GTK4, a widget can only have one parent at a time | ||
| 154 | if hadParent { | 158 | if hadParent { | ||
| 155 | widget.Unparent() | 159 | widget.Unparent() | ||
| 156 | } | 160 | } | ||
| 157 | - | widget.SetVisible(true) | |||
| 158 | pv.overlay.SetChild(widget) | 161 | pv.overlay.SetChild(widget) | ||
| 159 | 162 | ||||
| 163 | + | if hadParent && wasVisible { | |||
| 164 | + | widget.SetVisible(true) | |||
| 165 | + | } | |||
| 160 | if hadParent && pv.loading != nil { | 166 | if hadParent && pv.loading != nil { | ||
| 161 | pv.loading.SetVisible(false) | 167 | pv.loading.SetVisible(false) | ||
| 162 | } | 168 | } | ||
M internal/ui/component/pane_view_test.go +0 -2
| @@ -211,7 +211,6 @@ func TestSetWebViewWidget_ReplacesWidget(t *testing.T) { | |||
| 211 | 211 | // Expect removal of old widget and addition of new | |
| 212 | 212 | mockOverlay.EXPECT().SetChild(nil).Once() | |
| 213 | 213 | mockNewWebView.EXPECT().GetParent().Return(nil).Once() | |
| 214 | - | mockNewWebView.EXPECT().SetVisible(true).Once() | |
| 215 | 214 | mockOverlay.EXPECT().SetChild(mockNewWebView).Once() | |
| 216 | 215 | ||
| 217 | 216 | // Act | |
| @@ -234,7 +233,6 @@ func TestSetWebViewWidget_FromNil(t *testing.T) { | |||
| 234 | 233 | ||
| 235 | 234 | // Expect only setting new child (no removal since old was nil) | |
| 236 | 235 | mockNewWebView.EXPECT().GetParent().Return(nil).Once() | |
| 237 | - | mockNewWebView.EXPECT().SetVisible(true).Once() | |
| 238 | 236 | mockOverlay.EXPECT().SetChild(mockNewWebView).Once() | |
| 239 | 237 | ||
| 240 | 238 | // Act | |
| @@ -211,7 +211,6 @@ func TestSetWebViewWidget_ReplacesWidget(t *testing.T) { | |||||
| 211 | // Expect removal of old widget and addition of new | 211 | // Expect removal of old widget and addition of new | ||
| 212 | mockOverlay.EXPECT().SetChild(nil).Once() | 212 | mockOverlay.EXPECT().SetChild(nil).Once() | ||
| 213 | mockNewWebView.EXPECT().GetParent().Return(nil).Once() | 213 | mockNewWebView.EXPECT().GetParent().Return(nil).Once() | ||
| 214 | - | mockNewWebView.EXPECT().SetVisible(true).Once() | |||
| 215 | mockOverlay.EXPECT().SetChild(mockNewWebView).Once() | 214 | mockOverlay.EXPECT().SetChild(mockNewWebView).Once() | ||
| 216 | 215 | ||||
| 217 | // Act | 216 | // Act | ||
| @@ -234,7 +233,6 @@ func TestSetWebViewWidget_FromNil(t *testing.T) { | |||||
| 234 | 233 | ||||
| 235 | // Expect only setting new child (no removal since old was nil) | 234 | // Expect only setting new child (no removal since old was nil) | ||
| 236 | mockNewWebView.EXPECT().GetParent().Return(nil).Once() | 235 | mockNewWebView.EXPECT().GetParent().Return(nil).Once() | ||
| 237 | - | mockNewWebView.EXPECT().SetVisible(true).Once() | |||
| 238 | mockOverlay.EXPECT().SetChild(mockNewWebView).Once() | 236 | mockOverlay.EXPECT().SetChild(mockNewWebView).Once() | ||
| 239 | 237 | ||||
| 240 | // Act | 238 | // Act | ||
M internal/ui/coordinator/content.go +57 -6
| @@ -55,6 +55,9 @@ type ContentCoordinator struct { | |||
| 55 | 55 | // Callback when the WebView becomes visible (first real commit) | |
| 56 | 56 | onWebViewShown func(paneID entity.PaneID) | |
| 57 | 57 | ||
| 58 | + | revealMu sync.Mutex | |
| 59 | + | pendingReveal map[entity.PaneID]bool | |
| 60 | + | ||
| 58 | 61 | // Gesture action handler for mouse button navigation | |
| 59 | 62 | gestureActionHandler input.ActionHandler | |
| 60 | 63 | ||
| @@ -100,6 +103,7 @@ func NewContentCoordinator( | |||
| 100 | 103 | webViews: make(map[entity.PaneID]*webkit.WebView), | |
| 101 | 104 | paneTitles: make(map[entity.PaneID]string), | |
| 102 | 105 | navOrigins: make(map[entity.PaneID]string), | |
| 106 | + | pendingReveal: make(map[entity.PaneID]bool), | |
| 103 | 107 | getActiveWS: getActiveWS, | |
| 104 | 108 | pendingPopups: make(map[port.WebViewID]*PendingPopup), | |
| 105 | 109 | } | |
| @@ -721,15 +725,13 @@ func (c *ContentCoordinator) onLoadCommitted(ctx context.Context, paneID entity. | |||
| 721 | 725 | // Avoid updating UI/domain state to about:blank when we know the pane is | |
| 722 | 726 | // navigating to a different URL. This prevents the omnibox/window title from | |
| 723 | 727 | // briefly showing about:blank on cold start. | |
| 728 | + | c.clearPendingReveal(paneID) | |
| 724 | 729 | return | |
| 725 | 730 | } | |
| 726 | 731 | ||
| 727 | - | if inner := wv.Widget(); inner != nil { | |
| 728 | - | inner.SetVisible(true) | |
| 729 | - | log.Debug().Str("pane_id", string(paneID)).Str("url", url).Msg("webview shown on load committed") | |
| 730 | - | } | |
| 731 | - | if c.onWebViewShown != nil { | |
| 732 | - | c.onWebViewShown(paneID) | |
| 732 | + | c.markPendingReveal(paneID) | |
| 733 | + | if wv.EstimatedProgress() > 0 { | |
| 734 | + | c.revealIfPending(ctx, paneID, url, "progress-after-commit") | |
| 733 | 735 | } | |
| 734 | 736 | ||
| 735 | 737 | // Update domain model with current URI for session snapshots | |
| @@ -800,10 +802,16 @@ func (c *ContentCoordinator) onLoadFinished(paneID entity.PaneID) { | |||
| 800 | 802 | if paneView != nil { | |
| 801 | 803 | paneView.SetLoading(false) | |
| 802 | 804 | } | |
| 805 | + | ||
| 806 | + | c.revealIfPending(context.Background(), paneID, "", "load-finished") | |
| 803 | 807 | } | |
| 804 | 808 | ||
| 805 | 809 | // onProgressChanged updates the progress bar with current load progress. | |
| 806 | 810 | func (c *ContentCoordinator) onProgressChanged(paneID entity.PaneID, progress float64) { | |
| 811 | + | if progress > 0 { | |
| 812 | + | c.revealIfPending(context.Background(), paneID, "", "progress") | |
| 813 | + | } | |
| 814 | + | ||
| 807 | 815 | _, wsView := c.getActiveWS() | |
| 808 | 816 | if wsView == nil { | |
| 809 | 817 | return | |
| @@ -815,6 +823,49 @@ func (c *ContentCoordinator) onProgressChanged(paneID entity.PaneID, progress fl | |||
| 815 | 823 | } | |
| 816 | 824 | } | |
| 817 | 825 | ||
| 826 | + | func (c *ContentCoordinator) markPendingReveal(paneID entity.PaneID) { | |
| 827 | + | c.revealMu.Lock() | |
| 828 | + | c.pendingReveal[paneID] = true | |
| 829 | + | c.revealMu.Unlock() | |
| 830 | + | } | |
| 831 | + | ||
| 832 | + | func (c *ContentCoordinator) clearPendingReveal(paneID entity.PaneID) { | |
| 833 | + | c.revealMu.Lock() | |
| 834 | + | delete(c.pendingReveal, paneID) | |
| 835 | + | c.revealMu.Unlock() | |
| 836 | + | } | |
| 837 | + | ||
| 838 | + | func (c *ContentCoordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, url, reason string) { | |
| 839 | + | c.revealMu.Lock() | |
| 840 | + | pending := c.pendingReveal[paneID] | |
| 841 | + | if pending { | |
| 842 | + | delete(c.pendingReveal, paneID) | |
| 843 | + | } | |
| 844 | + | c.revealMu.Unlock() | |
| 845 | + | ||
| 846 | + | if !pending { | |
| 847 | + | return | |
| 848 | + | } | |
| 849 | + | ||
| 850 | + | wv := c.webViews[paneID] | |
| 851 | + | if wv == nil || wv.IsDestroyed() { | |
| 852 | + | return | |
| 853 | + | } | |
| 854 | + | ||
| 855 | + | if inner := wv.Widget(); inner != nil { | |
| 856 | + | inner.SetVisible(true) | |
| 857 | + | logging.FromContext(ctx). | |
| 858 | + | Debug(). | |
| 859 | + | Str("pane_id", string(paneID)). | |
| 860 | + | Str("url", url). | |
| 861 | + | Str("reason", reason). | |
| 862 | + | Msg("webview shown after load commit") | |
| 863 | + | } | |
| 864 | + | if c.onWebViewShown != nil { | |
| 865 | + | c.onWebViewShown(paneID) | |
| 866 | + | } | |
| 867 | + | } | |
| 868 | + | ||
| 818 | 869 | // onLinkHover updates the link status overlay when hovering over links. | |
| 819 | 870 | func (c *ContentCoordinator) onLinkHover(paneID entity.PaneID, uri string) { | |
| 820 | 871 | _, wsView := c.getActiveWS() | |
| @@ -55,6 +55,9 @@ type ContentCoordinator struct { | |||||
| 55 | // Callback when the WebView becomes visible (first real commit) | 55 | // Callback when the WebView becomes visible (first real commit) | ||
| 56 | onWebViewShown func(paneID entity.PaneID) | 56 | onWebViewShown func(paneID entity.PaneID) | ||
| 57 | 57 | ||||
| 58 | + | revealMu sync.Mutex | |||
| 59 | + | pendingReveal map[entity.PaneID]bool | |||
| 60 | + | ||||
| 58 | // Gesture action handler for mouse button navigation | 61 | // Gesture action handler for mouse button navigation | ||
| 59 | gestureActionHandler input.ActionHandler | 62 | gestureActionHandler input.ActionHandler | ||
| 60 | 63 | ||||
| @@ -100,6 +103,7 @@ func NewContentCoordinator( | |||||
| 100 | webViews: make(map[entity.PaneID]*webkit.WebView), | 103 | webViews: make(map[entity.PaneID]*webkit.WebView), | ||
| 101 | paneTitles: make(map[entity.PaneID]string), | 104 | paneTitles: make(map[entity.PaneID]string), | ||
| 102 | navOrigins: make(map[entity.PaneID]string), | 105 | navOrigins: make(map[entity.PaneID]string), | ||
| 106 | + | pendingReveal: make(map[entity.PaneID]bool), | |||
| 103 | getActiveWS: getActiveWS, | 107 | getActiveWS: getActiveWS, | ||
| 104 | pendingPopups: make(map[port.WebViewID]*PendingPopup), | 108 | pendingPopups: make(map[port.WebViewID]*PendingPopup), | ||
| 105 | } | 109 | } | ||
| @@ -721,15 +725,13 @@ func (c *ContentCoordinator) onLoadCommitted(ctx context.Context, paneID entity. | |||||
| 721 | // Avoid updating UI/domain state to about:blank when we know the pane is | 725 | // Avoid updating UI/domain state to about:blank when we know the pane is | ||
| 722 | // navigating to a different URL. This prevents the omnibox/window title from | 726 | // navigating to a different URL. This prevents the omnibox/window title from | ||
| 723 | // briefly showing about:blank on cold start. | 727 | // briefly showing about:blank on cold start. | ||
| 728 | + | c.clearPendingReveal(paneID) | |||
| 724 | return | 729 | return | ||
| 725 | } | 730 | } | ||
| 726 | 731 | ||||
| 727 | - | if inner := wv.Widget(); inner != nil { | 732 | + | c.markPendingReveal(paneID) |
| 728 | - | inner.SetVisible(true) | 733 | + | if wv.EstimatedProgress() > 0 { |
| 729 | - | log.Debug().Str("pane_id", string(paneID)).Str("url", url).Msg("webview shown on load committed") | 734 | + | c.revealIfPending(ctx, paneID, url, "progress-after-commit") |
| 730 | - | } | |||
| 731 | - | if c.onWebViewShown != nil { | |||
| 732 | - | c.onWebViewShown(paneID) | |||
| 733 | } | 735 | } | ||
| 734 | 736 | ||||
| 735 | // Update domain model with current URI for session snapshots | 737 | // Update domain model with current URI for session snapshots | ||
| @@ -800,10 +802,16 @@ func (c *ContentCoordinator) onLoadFinished(paneID entity.PaneID) { | |||||
| 800 | if paneView != nil { | 802 | if paneView != nil { | ||
| 801 | paneView.SetLoading(false) | 803 | paneView.SetLoading(false) | ||
| 802 | } | 804 | } | ||
| 805 | + | ||||
| 806 | + | c.revealIfPending(context.Background(), paneID, "", "load-finished") | |||
| 803 | } | 807 | } | ||
| 804 | 808 | ||||
| 805 | // onProgressChanged updates the progress bar with current load progress. | 809 | // onProgressChanged updates the progress bar with current load progress. | ||
| 806 | func (c *ContentCoordinator) onProgressChanged(paneID entity.PaneID, progress float64) { | 810 | func (c *ContentCoordinator) onProgressChanged(paneID entity.PaneID, progress float64) { | ||
| 811 | + | if progress > 0 { | |||
| 812 | + | c.revealIfPending(context.Background(), paneID, "", "progress") | |||
| 813 | + | } | |||
| 814 | + | ||||
| 807 | _, wsView := c.getActiveWS() | 815 | _, wsView := c.getActiveWS() | ||
| 808 | if wsView == nil { | 816 | if wsView == nil { | ||
| 809 | return | 817 | return | ||
| @@ -815,6 +823,49 @@ func (c *ContentCoordinator) onProgressChanged(paneID entity.PaneID, progress fl | |||||
| 815 | } | 823 | } | ||
| 816 | } | 824 | } | ||
| 817 | 825 | ||||
| 826 | + | func (c *ContentCoordinator) markPendingReveal(paneID entity.PaneID) { | |||
| 827 | + | c.revealMu.Lock() | |||
| 828 | + | c.pendingReveal[paneID] = true | |||
| 829 | + | c.revealMu.Unlock() | |||
| 830 | + | } | |||
| 831 | + | ||||
| 832 | + | func (c *ContentCoordinator) clearPendingReveal(paneID entity.PaneID) { | |||
| 833 | + | c.revealMu.Lock() | |||
| 834 | + | delete(c.pendingReveal, paneID) | |||
| 835 | + | c.revealMu.Unlock() | |||
| 836 | + | } | |||
| 837 | + | ||||
| 838 | + | func (c *ContentCoordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, url, reason string) { | |||
| 839 | + | c.revealMu.Lock() | |||
| 840 | + | pending := c.pendingReveal[paneID] | |||
| 841 | + | if pending { | |||
| 842 | + | delete(c.pendingReveal, paneID) | |||
| 843 | + | } | |||
| 844 | + | c.revealMu.Unlock() | |||
| 845 | + | ||||
| 846 | + | if !pending { | |||
| 847 | + | return | |||
| 848 | + | } | |||
| 849 | + | ||||
| 850 | + | wv := c.webViews[paneID] | |||
| 851 | + | if wv == nil || wv.IsDestroyed() { | |||
| 852 | + | return | |||
| 853 | + | } | |||
| 854 | + | ||||
| 855 | + | if inner := wv.Widget(); inner != nil { | |||
| 856 | + | inner.SetVisible(true) | |||
| 857 | + | logging.FromContext(ctx). | |||
| 858 | + | Debug(). | |||
| 859 | + | Str("pane_id", string(paneID)). | |||
| 860 | + | Str("url", url). | |||
| 861 | + | Str("reason", reason). | |||
| 862 | + | Msg("webview shown after load commit") | |||
| 863 | + | } | |||
| 864 | + | if c.onWebViewShown != nil { | |||
| 865 | + | c.onWebViewShown(paneID) | |||
| 866 | + | } | |||
| 867 | + | } | |||
| 868 | + | ||||
| 818 | // onLinkHover updates the link status overlay when hovering over links. | 869 | // onLinkHover updates the link status overlay when hovering over links. | ||
| 819 | func (c *ContentCoordinator) onLinkHover(paneID entity.PaneID, uri string) { | 870 | func (c *ContentCoordinator) onLinkHover(paneID entity.PaneID, uri string) { | ||
| 820 | _, wsView := c.getActiveWS() | 871 | _, wsView := c.getActiveWS() | ||