fix(ui): resolve loading skeleton on failed navigation
- commit
- c9df2f8d4a5cbc612aaa36e434a92e14042be923
- author
- brice · · 2026-06-24 12:21 UTC
- committer
- brice · · 2026-06-24 18:44 UTC
- parent
- eba3182
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
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 | |||
| 542 | 542 | // CEF's OnLoadingProgressChange also provides progress=1.0 at completion. | |
| 543 | 543 | } | |
| 544 | 544 | ||
| 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 | + | } | |
| 547 | 588 | } | |
| 548 | 589 | ||
| 549 | 590 | 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) { | |||
| 423 | 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 | 511 | func TestOnTextSelectionChanged_ForwardsSelectionToClipboardOrchestrator(t *testing.T) { | |
| 427 | 512 | recorder := &clipboardOrchestratorRecorder{} | |
| 428 | 513 | 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) | ||