detached 961fbde

Merge pull request #320 from bnema/fix/issue-311-load-error-skeleton

fix(ui): resolve loading skeleton on failed navigation
commit
961fbde2e37050e1af9768d7fb3de5f93c9354d6
author
bnema · · 2026-06-25 04:27 UTC
committer
GitHub · · 2026-06-25 04:27 UTC
merge of
245fafa + a048e29 · diff against the first parent
signature
GPG UnverifiedB5690EEEBB952194 The key is unknown
2 files · +128 -2
M internal/infrastructure/cef/webview_handlers.go +43 -2
@@ -542,8 +542,49 @@ func (h *handlerSet) OnLoadEnd(_ purecef.Browser, frame purecef.Frame, httpStatu
542542 // CEF's OnLoadingProgressChange also provides progress=1.0 at completion.
543543 }
544544
545-// OnLoadError is a no-op.
546-func (h *handlerSet) OnLoadError(_ purecef.Browser, _ purecef.Frame, _ purecef.Errorcode, _, _ string) {
545+const cefErrAborted purecef.Errorcode = -3
546+
547+// OnLoadError handles failed main-frame navigations that never reach
548+// OnLoadStart. CEF still sends OnLoadingStateChange(false) after load errors,
549+// so this callback only synthesizes the missing committed event needed to reveal
550+// the pane; the normal browser-level loading-state callback remains responsible
551+// for the terminal LoadFinished state.
552+func (h *handlerSet) OnLoadError(_ purecef.Browser, frame purecef.Frame, errorCode purecef.Errorcode, errorText, failedURL string) {
553+ if h == nil || h.wv == nil || frame == nil || !frame.IsMain() {
554+ return
555+ }
556+ if errorCode == cefErrAborted {
557+ return
558+ }
559+ if failedURL == "" {
560+ failedURL = frame.GetURL()
561+ }
562+
563+ h.wv.mu.RLock()
564+ currentURI := h.wv.uri
565+ cb := h.wv.callbacks
566+ h.wv.mu.RUnlock()
567+
568+ if h.wv.ctx != nil {
569+ logging.FromContext(h.wv.ctx).Debug().
570+ Int32("error_code", errorCode).
571+ Str("error_text", errorText).
572+ Str("failed_url", logging.TruncateURL(failedURL, logging.PermissionLogURLMaxLen)).
573+ Msg("cef: OnLoadError")
574+ }
575+
576+ if failedURL != "" {
577+ h.wv.updateURI(failedURL)
578+ }
579+
580+ if cb == nil || cb.OnLoadChanged == nil {
581+ return
582+ }
583+ if failedURL != "" && !pendingURIEquivalent(currentURI, failedURL) {
584+ h.wv.runOnGTK(func() {
585+ cb.OnLoadChanged(port.LoadCommitted)
586+ })
587+ }
547588 }
548589
549590 func mapCEFWindowDisposition(disposition purecef.WindowOpenDisposition) dto.WindowDisposition {
@@ -542,8 +542,49 @@ func (h *handlerSet) OnLoadEnd(_ purecef.Browser, frame purecef.Frame, httpStatu
542 // CEF's OnLoadingProgressChange also provides progress=1.0 at completion.542 // CEF's OnLoadingProgressChange also provides progress=1.0 at completion.
543 }543 }
544 544
545-// OnLoadError is a no-op.545+const cefErrAborted purecef.Errorcode = -3
546-func (h *handlerSet) OnLoadError(_ purecef.Browser, _ purecef.Frame, _ purecef.Errorcode, _, _ string) {546+
547+// OnLoadError handles failed main-frame navigations that never reach
548+// OnLoadStart. CEF still sends OnLoadingStateChange(false) after load errors,
549+// so this callback only synthesizes the missing committed event needed to reveal
550+// the pane; the normal browser-level loading-state callback remains responsible
551+// for the terminal LoadFinished state.
552+func (h *handlerSet) OnLoadError(_ purecef.Browser, frame purecef.Frame, errorCode purecef.Errorcode, errorText, failedURL string) {
553+ if h == nil || h.wv == nil || frame == nil || !frame.IsMain() {
554+ return
555+ }
556+ if errorCode == cefErrAborted {
557+ return
558+ }
559+ if failedURL == "" {
560+ failedURL = frame.GetURL()
561+ }
562+
563+ h.wv.mu.RLock()
564+ currentURI := h.wv.uri
565+ cb := h.wv.callbacks
566+ h.wv.mu.RUnlock()
567+
568+ if h.wv.ctx != nil {
569+ logging.FromContext(h.wv.ctx).Debug().
570+ Int32("error_code", errorCode).
571+ Str("error_text", errorText).
572+ Str("failed_url", logging.TruncateURL(failedURL, logging.PermissionLogURLMaxLen)).
573+ Msg("cef: OnLoadError")
574+ }
575+
576+ if failedURL != "" {
577+ h.wv.updateURI(failedURL)
578+ }
579+
580+ if cb == nil || cb.OnLoadChanged == nil {
581+ return
582+ }
583+ if failedURL != "" && !pendingURIEquivalent(currentURI, failedURL) {
584+ h.wv.runOnGTK(func() {
585+ cb.OnLoadChanged(port.LoadCommitted)
586+ })
587+ }
547 }588 }
548 589
549 func mapCEFWindowDisposition(disposition purecef.WindowOpenDisposition) dto.WindowDisposition {590 func mapCEFWindowDisposition(disposition purecef.WindowOpenDisposition) dto.WindowDisposition {
M internal/infrastructure/cef/webview_handlers_test.go +85 -0
@@ -423,6 +423,91 @@ func TestOnLoadEndDoesNotDispatchBrowserLevelCompletion(t *testing.T) {
423423 assert.Empty(t, gotProgress)
424424 }
425425
426+func TestOnLoadErrorMainFrameCommitsFailedNavigation(t *testing.T) {
427+ wv := &WebView{ctx: context.Background()}
428+ wv.updateLoadState(true, true, false)
429+ var gotEvents []port.LoadEvent
430+ wv.SetCallbacks(&port.WebViewCallbacks{
431+ OnLoadChanged: func(event port.LoadEvent) {
432+ gotEvents = append(gotEvents, event)
433+ },
434+ })
435+
436+ h := &handlerSet{wv: wv}
437+ h.OnLoadError(nil, stubFrame{main: true, url: "http://localhost:9"}, -102, "CONNECTION_REFUSED", "http://localhost:9")
438+
439+ require.Equal(t, []port.LoadEvent{port.LoadCommitted}, gotEvents)
440+ require.Equal(t, "http://localhost:9", wv.URI())
441+ require.True(t, wv.IsLoading())
442+ require.True(t, wv.CanGoBack())
443+}
444+
445+func TestOnLoadErrorFollowedByLoadingStateFinishTerminatesNavigation(t *testing.T) {
446+ wv := &WebView{ctx: context.Background()}
447+ wv.updateLoadState(true, false, false)
448+ var gotEvents []port.LoadEvent
449+ wv.SetCallbacks(&port.WebViewCallbacks{
450+ OnLoadChanged: func(event port.LoadEvent) {
451+ gotEvents = append(gotEvents, event)
452+ },
453+ })
454+
455+ h := &handlerSet{wv: wv}
456+ h.OnLoadError(nil, stubFrame{main: true, url: "http://localhost:9"}, -102, "CONNECTION_REFUSED", "http://localhost:9")
457+ h.OnLoadingStateChange(nil, 0, 0, 0)
458+
459+ require.Equal(t, []port.LoadEvent{port.LoadCommitted, port.LoadFinished}, gotEvents)
460+ require.False(t, wv.IsLoading())
461+}
462+
463+func TestOnLoadErrorAfterLoadStartDoesNotDispatchDuplicateEvents(t *testing.T) {
464+ wv := &WebView{ctx: context.Background()}
465+ var gotEvents []port.LoadEvent
466+ wv.SetCallbacks(&port.WebViewCallbacks{
467+ OnLoadChanged: func(event port.LoadEvent) {
468+ gotEvents = append(gotEvents, event)
469+ },
470+ })
471+
472+ h := &handlerSet{wv: wv}
473+ h.OnLoadStart(nil, stubFrame{main: true, url: "http://localhost:9"}, 0)
474+ gotEvents = nil
475+ h.OnLoadError(nil, stubFrame{main: true, url: "http://localhost:9"}, -102, "CONNECTION_REFUSED", "http://localhost:9")
476+
477+ require.Empty(t, gotEvents)
478+}
479+
480+func TestOnLoadErrorAbortedNavigationDoesNotCommit(t *testing.T) {
481+ wv := &WebView{ctx: context.Background()}
482+ var gotEvents []port.LoadEvent
483+ wv.SetCallbacks(&port.WebViewCallbacks{
484+ OnLoadChanged: func(event port.LoadEvent) {
485+ gotEvents = append(gotEvents, event)
486+ },
487+ })
488+
489+ h := &handlerSet{wv: wv}
490+ h.OnLoadError(nil, stubFrame{main: true, url: "https://example.com/file.zip"}, cefErrAborted, "ERR_ABORTED", "https://example.com/file.zip")
491+
492+ require.Empty(t, gotEvents)
493+ require.Empty(t, wv.URI())
494+}
495+
496+func TestOnLoadErrorSubFrameDoesNotTerminateNavigation(t *testing.T) {
497+ wv := &WebView{ctx: context.Background()}
498+ var gotEvents []port.LoadEvent
499+ wv.SetCallbacks(&port.WebViewCallbacks{
500+ OnLoadChanged: func(event port.LoadEvent) {
501+ gotEvents = append(gotEvents, event)
502+ },
503+ })
504+
505+ h := &handlerSet{wv: wv}
506+ h.OnLoadError(nil, stubFrame{main: false, url: "http://localhost:9/frame"}, -102, "CONNECTION_REFUSED", "http://localhost:9/frame")
507+
508+ assert.Empty(t, gotEvents)
509+}
510+
426511 func TestOnTextSelectionChanged_ForwardsSelectionToClipboardOrchestrator(t *testing.T) {
427512 recorder := &clipboardOrchestratorRecorder{}
428513 orchestrator := portmocks.NewMockClipboardTextOrchestrator(t)
@@ -423,6 +423,91 @@ func TestOnLoadEndDoesNotDispatchBrowserLevelCompletion(t *testing.T) {
423 assert.Empty(t, gotProgress)423 assert.Empty(t, gotProgress)
424 }424 }
425 425
426+func TestOnLoadErrorMainFrameCommitsFailedNavigation(t *testing.T) {
427+ wv := &WebView{ctx: context.Background()}
428+ wv.updateLoadState(true, true, false)
429+ var gotEvents []port.LoadEvent
430+ wv.SetCallbacks(&port.WebViewCallbacks{
431+ OnLoadChanged: func(event port.LoadEvent) {
432+ gotEvents = append(gotEvents, event)
433+ },
434+ })
435+
436+ h := &handlerSet{wv: wv}
437+ h.OnLoadError(nil, stubFrame{main: true, url: "http://localhost:9"}, -102, "CONNECTION_REFUSED", "http://localhost:9")
438+
439+ require.Equal(t, []port.LoadEvent{port.LoadCommitted}, gotEvents)
440+ require.Equal(t, "http://localhost:9", wv.URI())
441+ require.True(t, wv.IsLoading())
442+ require.True(t, wv.CanGoBack())
443+}
444+
445+func TestOnLoadErrorFollowedByLoadingStateFinishTerminatesNavigation(t *testing.T) {
446+ wv := &WebView{ctx: context.Background()}
447+ wv.updateLoadState(true, false, false)
448+ var gotEvents []port.LoadEvent
449+ wv.SetCallbacks(&port.WebViewCallbacks{
450+ OnLoadChanged: func(event port.LoadEvent) {
451+ gotEvents = append(gotEvents, event)
452+ },
453+ })
454+
455+ h := &handlerSet{wv: wv}
456+ h.OnLoadError(nil, stubFrame{main: true, url: "http://localhost:9"}, -102, "CONNECTION_REFUSED", "http://localhost:9")
457+ h.OnLoadingStateChange(nil, 0, 0, 0)
458+
459+ require.Equal(t, []port.LoadEvent{port.LoadCommitted, port.LoadFinished}, gotEvents)
460+ require.False(t, wv.IsLoading())
461+}
462+
463+func TestOnLoadErrorAfterLoadStartDoesNotDispatchDuplicateEvents(t *testing.T) {
464+ wv := &WebView{ctx: context.Background()}
465+ var gotEvents []port.LoadEvent
466+ wv.SetCallbacks(&port.WebViewCallbacks{
467+ OnLoadChanged: func(event port.LoadEvent) {
468+ gotEvents = append(gotEvents, event)
469+ },
470+ })
471+
472+ h := &handlerSet{wv: wv}
473+ h.OnLoadStart(nil, stubFrame{main: true, url: "http://localhost:9"}, 0)
474+ gotEvents = nil
475+ h.OnLoadError(nil, stubFrame{main: true, url: "http://localhost:9"}, -102, "CONNECTION_REFUSED", "http://localhost:9")
476+
477+ require.Empty(t, gotEvents)
478+}
479+
480+func TestOnLoadErrorAbortedNavigationDoesNotCommit(t *testing.T) {
481+ wv := &WebView{ctx: context.Background()}
482+ var gotEvents []port.LoadEvent
483+ wv.SetCallbacks(&port.WebViewCallbacks{
484+ OnLoadChanged: func(event port.LoadEvent) {
485+ gotEvents = append(gotEvents, event)
486+ },
487+ })
488+
489+ h := &handlerSet{wv: wv}
490+ h.OnLoadError(nil, stubFrame{main: true, url: "https://example.com/file.zip"}, cefErrAborted, "ERR_ABORTED", "https://example.com/file.zip")
491+
492+ require.Empty(t, gotEvents)
493+ require.Empty(t, wv.URI())
494+}
495+
496+func TestOnLoadErrorSubFrameDoesNotTerminateNavigation(t *testing.T) {
497+ wv := &WebView{ctx: context.Background()}
498+ var gotEvents []port.LoadEvent
499+ wv.SetCallbacks(&port.WebViewCallbacks{
500+ OnLoadChanged: func(event port.LoadEvent) {
501+ gotEvents = append(gotEvents, event)
502+ },
503+ })
504+
505+ h := &handlerSet{wv: wv}
506+ h.OnLoadError(nil, stubFrame{main: false, url: "http://localhost:9/frame"}, -102, "CONNECTION_REFUSED", "http://localhost:9/frame")
507+
508+ assert.Empty(t, gotEvents)
509+}
510+
426 func TestOnTextSelectionChanged_ForwardsSelectionToClipboardOrchestrator(t *testing.T) {511 func TestOnTextSelectionChanged_ForwardsSelectionToClipboardOrchestrator(t *testing.T) {
427 recorder := &clipboardOrchestratorRecorder{}512 recorder := &clipboardOrchestratorRecorder{}
428 orchestrator := portmocks.NewMockClipboardTextOrchestrator(t)513 orchestrator := portmocks.NewMockClipboardTextOrchestrator(t)

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 8

Tags 42

↑↓ moveenter openesc close