fix(popup): preserve failure diagnostics and cleanup
- commit
- e37d7f07947126b33040229f7373e36d86a626c1
- author
- brice · · 2026-07-27 11:52 UTC
- parent
- 47627e6
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +135 -14
M cmd/dumber/runtime_safety.go +1 -0
| @@ -63,6 +63,7 @@ func ensureRuntimeSafety() error { | |||
| 63 | 63 | if err != nil { | |
| 64 | 64 | return fmt.Errorf("resolve executable for runtime safety re-exec: %w", err) | |
| 65 | 65 | } | |
| 66 | + | //nolint:gosec // G702: os.Executable and the current process argv are required for self-re-exec. | |
| 66 | 67 | if err := syscall.Exec(executable, os.Args, environ); err != nil { | |
| 67 | 68 | return fmt.Errorf("runtime safety re-exec: %w", err) | |
| 68 | 69 | } | |
| @@ -63,6 +63,7 @@ func ensureRuntimeSafety() error { | |||||
| 63 | if err != nil { | 63 | if err != nil { | ||
| 64 | return fmt.Errorf("resolve executable for runtime safety re-exec: %w", err) | 64 | return fmt.Errorf("resolve executable for runtime safety re-exec: %w", err) | ||
| 65 | } | 65 | } | ||
| 66 | + | //nolint:gosec // G702: os.Executable and the current process argv are required for self-re-exec. | |||
| 66 | if err := syscall.Exec(executable, os.Args, environ); err != nil { | 67 | if err := syscall.Exec(executable, os.Args, environ); err != nil { | ||
| 67 | return fmt.Errorf("runtime safety re-exec: %w", err) | 68 | return fmt.Errorf("runtime safety re-exec: %w", err) | ||
| 68 | } | 69 | } | ||
M internal/application/usecase/browsing_context_policy.go +60 -10
| @@ -24,37 +24,82 @@ func (BrowsingContextPolicy) Decide(req dto.NewBrowsingContextRequest, namedCont | |||
| 24 | 24 | case strings.TrimSpace(req.TargetURI) == "": | |
| 25 | 25 | return completeHostDecision(decision, dto.HostDecisionDeny, dto.HostDecisionReasonEmptyTarget, "empty target URI") | |
| 26 | 26 | case req.AuthIntent: | |
| 27 | - | return completeHostDecision(decision, dto.HostDecisionCreateNativePopup, dto.HostDecisionReasonAuthNativePopup, "authentication intent requires a related native popup") | |
| 27 | + | return completeHostDecision( | |
| 28 | + | decision, | |
| 29 | + | dto.HostDecisionCreateNativePopup, | |
| 30 | + | dto.HostDecisionReasonAuthNativePopup, | |
| 31 | + | "authentication intent requires a related native popup", | |
| 32 | + | ) | |
| 28 | 33 | case req.RequiresNativeOpener: | |
| 29 | - | return completeHostDecision(decision, dto.HostDecisionCreateNativePopup, dto.HostDecisionReasonNativeOpenerRequired, "native opener relationship is required") | |
| 34 | + | return completeHostDecision( | |
| 35 | + | decision, | |
| 36 | + | dto.HostDecisionCreateNativePopup, | |
| 37 | + | dto.HostDecisionReasonNativeOpenerRequired, | |
| 38 | + | "native opener relationship is required", | |
| 39 | + | ) | |
| 30 | 40 | case isAmbiguousNativeBrowsingContext(req): | |
| 31 | - | return completeHostDecision(decision, dto.HostDecisionCreateNativePopup, dto.HostDecisionReasonAmbiguousOpenerNative, "ambiguous opener-coupled request prefers a related native popup") | |
| 41 | + | return completeHostDecision( | |
| 42 | + | decision, | |
| 43 | + | dto.HostDecisionCreateNativePopup, | |
| 44 | + | dto.HostDecisionReasonAmbiguousOpenerNative, | |
| 45 | + | "ambiguous opener-coupled request prefers a related native popup", | |
| 46 | + | ) | |
| 32 | 47 | } | |
| 33 | 48 | ||
| 34 | 49 | name := ReusableBrowsingContextName(req.TargetFrameName) | |
| 35 | 50 | if namedContextExists && name != "" && !req.NoJavaScriptAccess { | |
| 36 | 51 | decision.ReuseContextName = name | |
| 37 | 52 | decision.BrowsingContextName = name | |
| 38 | - | return completeHostDecision(decision, dto.HostDecisionReuseNamedPane, dto.HostDecisionReasonNamedContextReuse, "reuse live named browsing context") | |
| 53 | + | return completeHostDecision( | |
| 54 | + | decision, | |
| 55 | + | dto.HostDecisionReuseNamedPane, | |
| 56 | + | dto.HostDecisionReasonNamedContextReuse, | |
| 57 | + | "reuse live named browsing context", | |
| 58 | + | ) | |
| 39 | 59 | } | |
| 40 | 60 | decision.BrowsingContextName = name | |
| 41 | 61 | ||
| 42 | 62 | if req.SourceHost == dto.SourceHostFloating { | |
| 43 | 63 | return decideFloatingHost(decision, req) | |
| 44 | 64 | } | |
| 45 | - | return completeHostDecision(decision, dto.HostDecisionCreatePane, dto.HostDecisionReasonWorkspacePane, "workspace request uses pane hosting") | |
| 65 | + | return completeHostDecision( | |
| 66 | + | decision, | |
| 67 | + | dto.HostDecisionCreatePane, | |
| 68 | + | dto.HostDecisionReasonWorkspacePane, | |
| 69 | + | "workspace request uses pane hosting", | |
| 70 | + | ) | |
| 46 | 71 | } | |
| 47 | 72 | ||
| 48 | 73 | func decideFloatingHost(decision dto.HostDecision, req dto.NewBrowsingContextRequest) dto.HostDecision { | |
| 49 | 74 | switch { | |
| 50 | 75 | case req.PopupFeatures.State == dto.PopupFeaturesUnknown && req.TriggerKind != dto.TriggerLinkNewPage: | |
| 51 | - | return completeHostDecision(decision, dto.HostDecisionAwaitPopupFeatures, dto.HostDecisionReasonFloatingFeaturesPending, "popup features unavailable until ready-to-show") | |
| 76 | + | return completeHostDecision( | |
| 77 | + | decision, | |
| 78 | + | dto.HostDecisionAwaitPopupFeatures, | |
| 79 | + | dto.HostDecisionReasonFloatingFeaturesPending, | |
| 80 | + | "popup features unavailable until ready-to-show", | |
| 81 | + | ) | |
| 52 | 82 | case popupFeaturesRequestHost(req.PopupFeatures): | |
| 53 | - | return completeHostDecision(decision, dto.HostDecisionCreateNativePopup, dto.HostDecisionReasonFloatingPopupFeatures, "explicit popup features requested") | |
| 83 | + | return completeHostDecision( | |
| 84 | + | decision, | |
| 85 | + | dto.HostDecisionCreateNativePopup, | |
| 86 | + | dto.HostDecisionReasonFloatingPopupFeatures, | |
| 87 | + | "explicit popup features requested", | |
| 88 | + | ) | |
| 54 | 89 | case ReusableBrowsingContextName(req.TargetFrameName) == "": | |
| 55 | - | return completeHostDecision(decision, dto.HostDecisionNavigateSource, dto.HostDecisionReasonFloatingFeaturelessBlank, "featureless blank request navigates the source floating pane") | |
| 90 | + | return completeHostDecision( | |
| 91 | + | decision, | |
| 92 | + | dto.HostDecisionNavigateSource, | |
| 93 | + | dto.HostDecisionReasonFloatingFeaturelessBlank, | |
| 94 | + | "featureless blank request navigates the source floating pane", | |
| 95 | + | ) | |
| 56 | 96 | default: | |
| 57 | - | return completeHostDecision(decision, dto.HostDecisionCreateBrowserWindow, dto.HostDecisionReasonFloatingBrowserWindow, "floating request opens a detached browser window") | |
| 97 | + | return completeHostDecision( | |
| 98 | + | decision, | |
| 99 | + | dto.HostDecisionCreateBrowserWindow, | |
| 100 | + | dto.HostDecisionReasonFloatingBrowserWindow, | |
| 101 | + | "floating request opens a detached browser window", | |
| 102 | + | ) | |
| 58 | 103 | } | |
| 59 | 104 | } | |
| 60 | 105 | ||
| @@ -68,7 +113,12 @@ func popupFeaturesRequestHost(features dto.PopupFeatures) bool { | |||
| 68 | 113 | (features.IsPopupSet && features.IsPopup)) | |
| 69 | 114 | } | |
| 70 | 115 | ||
| 71 | - | func completeHostDecision(decision dto.HostDecision, kind dto.HostDecisionKind, code dto.HostDecisionReasonCode, reason string) dto.HostDecision { | |
| 116 | + | func completeHostDecision( | |
| 117 | + | decision dto.HostDecision, | |
| 118 | + | kind dto.HostDecisionKind, | |
| 119 | + | code dto.HostDecisionReasonCode, | |
| 120 | + | reason string, | |
| 121 | + | ) dto.HostDecision { | |
| 72 | 122 | decision.Kind = kind | |
| 73 | 123 | decision.ReasonCode = code | |
| 74 | 124 | decision.Reason = reason | |
| @@ -24,37 +24,82 @@ func (BrowsingContextPolicy) Decide(req dto.NewBrowsingContextRequest, namedCont | |||||
| 24 | case strings.TrimSpace(req.TargetURI) == "": | 24 | case strings.TrimSpace(req.TargetURI) == "": | ||
| 25 | return completeHostDecision(decision, dto.HostDecisionDeny, dto.HostDecisionReasonEmptyTarget, "empty target URI") | 25 | return completeHostDecision(decision, dto.HostDecisionDeny, dto.HostDecisionReasonEmptyTarget, "empty target URI") | ||
| 26 | case req.AuthIntent: | 26 | case req.AuthIntent: | ||
| 27 | - | return completeHostDecision(decision, dto.HostDecisionCreateNativePopup, dto.HostDecisionReasonAuthNativePopup, "authentication intent requires a related native popup") | 27 | + | return completeHostDecision( |
| 28 | + | decision, | |||
| 29 | + | dto.HostDecisionCreateNativePopup, | |||
| 30 | + | dto.HostDecisionReasonAuthNativePopup, | |||
| 31 | + | "authentication intent requires a related native popup", | |||
| 32 | + | ) | |||
| 28 | case req.RequiresNativeOpener: | 33 | case req.RequiresNativeOpener: | ||
| 29 | - | return completeHostDecision(decision, dto.HostDecisionCreateNativePopup, dto.HostDecisionReasonNativeOpenerRequired, "native opener relationship is required") | 34 | + | return completeHostDecision( |
| 35 | + | decision, | |||
| 36 | + | dto.HostDecisionCreateNativePopup, | |||
| 37 | + | dto.HostDecisionReasonNativeOpenerRequired, | |||
| 38 | + | "native opener relationship is required", | |||
| 39 | + | ) | |||
| 30 | case isAmbiguousNativeBrowsingContext(req): | 40 | case isAmbiguousNativeBrowsingContext(req): | ||
| 31 | - | return completeHostDecision(decision, dto.HostDecisionCreateNativePopup, dto.HostDecisionReasonAmbiguousOpenerNative, "ambiguous opener-coupled request prefers a related native popup") | 41 | + | return completeHostDecision( |
| 42 | + | decision, | |||
| 43 | + | dto.HostDecisionCreateNativePopup, | |||
| 44 | + | dto.HostDecisionReasonAmbiguousOpenerNative, | |||
| 45 | + | "ambiguous opener-coupled request prefers a related native popup", | |||
| 46 | + | ) | |||
| 32 | } | 47 | } | ||
| 33 | 48 | ||||
| 34 | name := ReusableBrowsingContextName(req.TargetFrameName) | 49 | name := ReusableBrowsingContextName(req.TargetFrameName) | ||
| 35 | if namedContextExists && name != "" && !req.NoJavaScriptAccess { | 50 | if namedContextExists && name != "" && !req.NoJavaScriptAccess { | ||
| 36 | decision.ReuseContextName = name | 51 | decision.ReuseContextName = name | ||
| 37 | decision.BrowsingContextName = name | 52 | decision.BrowsingContextName = name | ||
| 38 | - | return completeHostDecision(decision, dto.HostDecisionReuseNamedPane, dto.HostDecisionReasonNamedContextReuse, "reuse live named browsing context") | 53 | + | return completeHostDecision( |
| 54 | + | decision, | |||
| 55 | + | dto.HostDecisionReuseNamedPane, | |||
| 56 | + | dto.HostDecisionReasonNamedContextReuse, | |||
| 57 | + | "reuse live named browsing context", | |||
| 58 | + | ) | |||
| 39 | } | 59 | } | ||
| 40 | decision.BrowsingContextName = name | 60 | decision.BrowsingContextName = name | ||
| 41 | 61 | ||||
| 42 | if req.SourceHost == dto.SourceHostFloating { | 62 | if req.SourceHost == dto.SourceHostFloating { | ||
| 43 | return decideFloatingHost(decision, req) | 63 | return decideFloatingHost(decision, req) | ||
| 44 | } | 64 | } | ||
| 45 | - | return completeHostDecision(decision, dto.HostDecisionCreatePane, dto.HostDecisionReasonWorkspacePane, "workspace request uses pane hosting") | 65 | + | return completeHostDecision( |
| 66 | + | decision, | |||
| 67 | + | dto.HostDecisionCreatePane, | |||
| 68 | + | dto.HostDecisionReasonWorkspacePane, | |||
| 69 | + | "workspace request uses pane hosting", | |||
| 70 | + | ) | |||
| 46 | } | 71 | } | ||
| 47 | 72 | ||||
| 48 | func decideFloatingHost(decision dto.HostDecision, req dto.NewBrowsingContextRequest) dto.HostDecision { | 73 | func decideFloatingHost(decision dto.HostDecision, req dto.NewBrowsingContextRequest) dto.HostDecision { | ||
| 49 | switch { | 74 | switch { | ||
| 50 | case req.PopupFeatures.State == dto.PopupFeaturesUnknown && req.TriggerKind != dto.TriggerLinkNewPage: | 75 | case req.PopupFeatures.State == dto.PopupFeaturesUnknown && req.TriggerKind != dto.TriggerLinkNewPage: | ||
| 51 | - | return completeHostDecision(decision, dto.HostDecisionAwaitPopupFeatures, dto.HostDecisionReasonFloatingFeaturesPending, "popup features unavailable until ready-to-show") | 76 | + | return completeHostDecision( |
| 77 | + | decision, | |||
| 78 | + | dto.HostDecisionAwaitPopupFeatures, | |||
| 79 | + | dto.HostDecisionReasonFloatingFeaturesPending, | |||
| 80 | + | "popup features unavailable until ready-to-show", | |||
| 81 | + | ) | |||
| 52 | case popupFeaturesRequestHost(req.PopupFeatures): | 82 | case popupFeaturesRequestHost(req.PopupFeatures): | ||
| 53 | - | return completeHostDecision(decision, dto.HostDecisionCreateNativePopup, dto.HostDecisionReasonFloatingPopupFeatures, "explicit popup features requested") | 83 | + | return completeHostDecision( |
| 84 | + | decision, | |||
| 85 | + | dto.HostDecisionCreateNativePopup, | |||
| 86 | + | dto.HostDecisionReasonFloatingPopupFeatures, | |||
| 87 | + | "explicit popup features requested", | |||
| 88 | + | ) | |||
| 54 | case ReusableBrowsingContextName(req.TargetFrameName) == "": | 89 | case ReusableBrowsingContextName(req.TargetFrameName) == "": | ||
| 55 | - | return completeHostDecision(decision, dto.HostDecisionNavigateSource, dto.HostDecisionReasonFloatingFeaturelessBlank, "featureless blank request navigates the source floating pane") | 90 | + | return completeHostDecision( |
| 91 | + | decision, | |||
| 92 | + | dto.HostDecisionNavigateSource, | |||
| 93 | + | dto.HostDecisionReasonFloatingFeaturelessBlank, | |||
| 94 | + | "featureless blank request navigates the source floating pane", | |||
| 95 | + | ) | |||
| 56 | default: | 96 | default: | ||
| 57 | - | return completeHostDecision(decision, dto.HostDecisionCreateBrowserWindow, dto.HostDecisionReasonFloatingBrowserWindow, "floating request opens a detached browser window") | 97 | + | return completeHostDecision( |
| 98 | + | decision, | |||
| 99 | + | dto.HostDecisionCreateBrowserWindow, | |||
| 100 | + | dto.HostDecisionReasonFloatingBrowserWindow, | |||
| 101 | + | "floating request opens a detached browser window", | |||
| 102 | + | ) | |||
| 58 | } | 103 | } | ||
| 59 | } | 104 | } | ||
| 60 | 105 | ||||
| @@ -68,7 +113,12 @@ func popupFeaturesRequestHost(features dto.PopupFeatures) bool { | |||||
| 68 | (features.IsPopupSet && features.IsPopup)) | 113 | (features.IsPopupSet && features.IsPopup)) | ||
| 69 | } | 114 | } | ||
| 70 | 115 | ||||
| 71 | - | func completeHostDecision(decision dto.HostDecision, kind dto.HostDecisionKind, code dto.HostDecisionReasonCode, reason string) dto.HostDecision { | 116 | + | func completeHostDecision( |
| 117 | + | decision dto.HostDecision, | |||
| 118 | + | kind dto.HostDecisionKind, | |||
| 119 | + | code dto.HostDecisionReasonCode, | |||
| 120 | + | reason string, | |||
| 121 | + | ) dto.HostDecision { | |||
| 72 | decision.Kind = kind | 122 | decision.Kind = kind | ||
| 73 | decision.ReasonCode = code | 123 | decision.ReasonCode = code | ||
| 74 | decision.Reason = reason | 124 | decision.Reason = reason | ||
M internal/infrastructure/cef/webview_handlers.go +24 -2
| @@ -638,8 +638,6 @@ func mapCEFWindowDisposition(disposition purecef.WindowOpenDisposition) dto.Wind | |||
| 638 | 638 | // client back to CEF and allow native popup creation so opener semantics are | |
| 639 | 639 | // preserved. Otherwise we keep blocking and let the coordinator's fallback | |
| 640 | 640 | // pane handle the navigation. | |
| 641 | - | // | |
| 642 | - | //nolint:funlen // Popup ownership transitions stay linear to keep CEF callback semantics auditable. | |
| 643 | 641 | func (h *handlerSet) OnBeforePopup( | |
| 644 | 642 | browser purecef.Browser, frame purecef.Frame, popupID int32, targetURL, targetFrameName string, | |
| 645 | 643 | targetDisposition purecef.WindowOpenDisposition, userGesture int32, popupFeatures *purecef.PopupFeatures, | |
| @@ -710,6 +708,30 @@ func (h *handlerSet) OnBeforePopup( | |||
| 710 | 708 | } | |
| 711 | 709 | } | |
| 712 | 710 | ||
| 711 | + | return h.handlePopupHostDecision( | |
| 712 | + | cefPopup, | |
| 713 | + | decision, | |
| 714 | + | requestNoJavaScriptAccess, | |
| 715 | + | popupID, | |
| 716 | + | targetURL, | |
| 717 | + | targetDisposition, | |
| 718 | + | windowInfo, | |
| 719 | + | clientSlot, | |
| 720 | + | settings, | |
| 721 | + | ) | |
| 722 | + | } | |
| 723 | + | ||
| 724 | + | func (h *handlerSet) handlePopupHostDecision( | |
| 725 | + | cefPopup *WebView, | |
| 726 | + | decision dto.HostDecision, | |
| 727 | + | requestNoJavaScriptAccess bool, | |
| 728 | + | popupID int32, | |
| 729 | + | targetURL string, | |
| 730 | + | targetDisposition purecef.WindowOpenDisposition, | |
| 731 | + | windowInfo *purecef.WindowInfo, | |
| 732 | + | clientSlot *purecef.RawClientWriteSlot, | |
| 733 | + | settings *purecef.BrowserSettings, | |
| 734 | + | ) bool { | |
| 713 | 735 | cefPopup.setPopupNoJavaScriptAccess(requestNoJavaScriptAccess) | |
| 714 | 736 | switch decision.Kind { | |
| 715 | 737 | case dto.HostDecisionCreateNativePopup: | |
| @@ -638,8 +638,6 @@ func mapCEFWindowDisposition(disposition purecef.WindowOpenDisposition) dto.Wind | |||||
| 638 | // client back to CEF and allow native popup creation so opener semantics are | 638 | // client back to CEF and allow native popup creation so opener semantics are | ||
| 639 | // preserved. Otherwise we keep blocking and let the coordinator's fallback | 639 | // preserved. Otherwise we keep blocking and let the coordinator's fallback | ||
| 640 | // pane handle the navigation. | 640 | // pane handle the navigation. | ||
| 641 | - | // | |||
| 642 | - | //nolint:funlen // Popup ownership transitions stay linear to keep CEF callback semantics auditable. | |||
| 643 | func (h *handlerSet) OnBeforePopup( | 641 | func (h *handlerSet) OnBeforePopup( | ||
| 644 | browser purecef.Browser, frame purecef.Frame, popupID int32, targetURL, targetFrameName string, | 642 | browser purecef.Browser, frame purecef.Frame, popupID int32, targetURL, targetFrameName string, | ||
| 645 | targetDisposition purecef.WindowOpenDisposition, userGesture int32, popupFeatures *purecef.PopupFeatures, | 643 | targetDisposition purecef.WindowOpenDisposition, userGesture int32, popupFeatures *purecef.PopupFeatures, | ||
| @@ -710,6 +708,30 @@ func (h *handlerSet) OnBeforePopup( | |||||
| 710 | } | 708 | } | ||
| 711 | } | 709 | } | ||
| 712 | 710 | ||||
| 711 | + | return h.handlePopupHostDecision( | |||
| 712 | + | cefPopup, | |||
| 713 | + | decision, | |||
| 714 | + | requestNoJavaScriptAccess, | |||
| 715 | + | popupID, | |||
| 716 | + | targetURL, | |||
| 717 | + | targetDisposition, | |||
| 718 | + | windowInfo, | |||
| 719 | + | clientSlot, | |||
| 720 | + | settings, | |||
| 721 | + | ) | |||
| 722 | + | } | |||
| 723 | + | ||||
| 724 | + | func (h *handlerSet) handlePopupHostDecision( | |||
| 725 | + | cefPopup *WebView, | |||
| 726 | + | decision dto.HostDecision, | |||
| 727 | + | requestNoJavaScriptAccess bool, | |||
| 728 | + | popupID int32, | |||
| 729 | + | targetURL string, | |||
| 730 | + | targetDisposition purecef.WindowOpenDisposition, | |||
| 731 | + | windowInfo *purecef.WindowInfo, | |||
| 732 | + | clientSlot *purecef.RawClientWriteSlot, | |||
| 733 | + | settings *purecef.BrowserSettings, | |||
| 734 | + | ) bool { | |||
| 713 | cefPopup.setPopupNoJavaScriptAccess(requestNoJavaScriptAccess) | 735 | cefPopup.setPopupNoJavaScriptAccess(requestNoJavaScriptAccess) | ||
| 714 | switch decision.Kind { | 736 | switch decision.Kind { | ||
| 715 | case dto.HostDecisionCreateNativePopup: | 737 | case dto.HostDecisionCreateNativePopup: | ||
M internal/ui/coordinator/content/popup_manager.go +12 -0
| @@ -933,6 +933,18 @@ func (pm *popupManager) handleLinkMiddleClick( | |||
| 933 | 933 | return false | |
| 934 | 934 | } | |
| 935 | 935 | ||
| 936 | + | return pm.openMiddleClickPane(ctx, hooks, parentPaneID, parentWV, uri, decision) | |
| 937 | + | } | |
| 938 | + | ||
| 939 | + | func (pm *popupManager) openMiddleClickPane( | |
| 940 | + | ctx context.Context, | |
| 941 | + | hooks popupCoordinatorHooks, | |
| 942 | + | parentPaneID entity.PaneID, | |
| 943 | + | parentWV port.WebView, | |
| 944 | + | uri string, | |
| 945 | + | decision dto.HostDecision, | |
| 946 | + | ) bool { | |
| 947 | + | log := logging.FromContext(ctx) | |
| 936 | 948 | newWV, err := pm.createPopupWebView(ctx, parentWV.ID(), uri, true) | |
| 937 | 949 | if err != nil { | |
| 938 | 950 | log.Error().Err(err).Msg("failed to create webview for middle-click") | |
| @@ -933,6 +933,18 @@ func (pm *popupManager) handleLinkMiddleClick( | |||||
| 933 | return false | 933 | return false | ||
| 934 | } | 934 | } | ||
| 935 | 935 | ||||
| 936 | + | return pm.openMiddleClickPane(ctx, hooks, parentPaneID, parentWV, uri, decision) | |||
| 937 | + | } | |||
| 938 | + | ||||
| 939 | + | func (pm *popupManager) openMiddleClickPane( | |||
| 940 | + | ctx context.Context, | |||
| 941 | + | hooks popupCoordinatorHooks, | |||
| 942 | + | parentPaneID entity.PaneID, | |||
| 943 | + | parentWV port.WebView, | |||
| 944 | + | uri string, | |||
| 945 | + | decision dto.HostDecision, | |||
| 946 | + | ) bool { | |||
| 947 | + | log := logging.FromContext(ctx) | |||
| 936 | newWV, err := pm.createPopupWebView(ctx, parentWV.ID(), uri, true) | 948 | newWV, err := pm.createPopupWebView(ctx, parentWV.ID(), uri, true) | ||
| 937 | if err != nil { | 949 | if err != nil { | ||
| 938 | log.Error().Err(err).Msg("failed to create webview for middle-click") | 950 | log.Error().Err(err).Msg("failed to create webview for middle-click") | ||
M internal/ui/popup_staging_host.go +4 -2
| @@ -27,6 +27,8 @@ var newPopupStagingWindow = func(ctx context.Context, app *gtk.Application) (pop | |||
| 27 | 27 | return window.NewPopup(ctx, app) | |
| 28 | 28 | } | |
| 29 | 29 | ||
| 30 | + | var preparePopupStagingContentWidget = prepareNativePopupContentWidget | |
| 31 | + | ||
| 30 | 32 | func (h *popupStagingHost) cleanup() { | |
| 31 | 33 | if h == nil { | |
| 32 | 34 | return | |
| @@ -68,10 +70,10 @@ func (a *App) stagePopup(ctx context.Context, input content.StagePopupInput) (co | |||
| 68 | 70 | return nil, err | |
| 69 | 71 | } | |
| 70 | 72 | widget := a.contentCoord.WrapWidget(ctx, input.PopupWebView) | |
| 71 | - | gtkWidget, err := prepareNativePopupContentWidget(widget) | |
| 73 | + | gtkWidget, err := preparePopupStagingContentWidget(widget) | |
| 72 | 74 | if err != nil { | |
| 73 | 75 | shell.Destroy() | |
| 74 | - | return nil, fmt.Errorf("failed to wrap popup staging webview widget") | |
| 76 | + | return nil, fmt.Errorf("prepare popup staging webview widget: %w", err) | |
| 75 | 77 | } | |
| 76 | 78 | shell.SetContent(gtkWidget) | |
| 77 | 79 | return &popupStagingHost{shell: shell, attached: gtkWidget}, nil | |
| @@ -27,6 +27,8 @@ var newPopupStagingWindow = func(ctx context.Context, app *gtk.Application) (pop | |||||
| 27 | return window.NewPopup(ctx, app) | 27 | return window.NewPopup(ctx, app) | ||
| 28 | } | 28 | } | ||
| 29 | 29 | ||||
| 30 | + | var preparePopupStagingContentWidget = prepareNativePopupContentWidget | |||
| 31 | + | ||||
| 30 | func (h *popupStagingHost) cleanup() { | 32 | func (h *popupStagingHost) cleanup() { | ||
| 31 | if h == nil { | 33 | if h == nil { | ||
| 32 | return | 34 | return | ||
| @@ -68,10 +70,10 @@ func (a *App) stagePopup(ctx context.Context, input content.StagePopupInput) (co | |||||
| 68 | return nil, err | 70 | return nil, err | ||
| 69 | } | 71 | } | ||
| 70 | widget := a.contentCoord.WrapWidget(ctx, input.PopupWebView) | 72 | widget := a.contentCoord.WrapWidget(ctx, input.PopupWebView) | ||
| 71 | - | gtkWidget, err := prepareNativePopupContentWidget(widget) | 73 | + | gtkWidget, err := preparePopupStagingContentWidget(widget) |
| 72 | if err != nil { | 74 | if err != nil { | ||
| 73 | shell.Destroy() | 75 | shell.Destroy() | ||
| 74 | - | return nil, fmt.Errorf("failed to wrap popup staging webview widget") | 76 | + | return nil, fmt.Errorf("prepare popup staging webview widget: %w", err) |
| 75 | } | 77 | } | ||
| 76 | shell.SetContent(gtkWidget) | 78 | shell.SetContent(gtkWidget) | ||
| 77 | return &popupStagingHost{shell: shell, attached: gtkWidget}, nil | 79 | return &popupStagingHost{shell: shell, attached: gtkWidget}, nil | ||
M internal/ui/popup_staging_host_test.go +34 -0
| @@ -1,8 +1,13 @@ | |||
| 1 | 1 | package ui | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 5 | + | "errors" | |
| 4 | 6 | "testing" | |
| 5 | 7 | ||
| 8 | + | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | |
| 9 | + | "github.com/bnema/dumber/internal/ui/coordinator/content" | |
| 10 | + | "github.com/bnema/dumber/internal/ui/layout" | |
| 6 | 11 | "github.com/bnema/puregotk/v4/gtk" | |
| 7 | 12 | "github.com/stretchr/testify/assert" | |
| 8 | 13 | "github.com/stretchr/testify/require" | |
| @@ -24,6 +29,35 @@ func (s *popupStagingShellSpy) DetachContent() *gtk.Widget { | |||
| 24 | 29 | } | |
| 25 | 30 | func (s *popupStagingShellSpy) Destroy() { s.destroyCalls++ } | |
| 26 | 31 | ||
| 32 | + | func TestStagePopupWrapsContentPreparationErrorAndDestroysShell(t *testing.T) { | |
| 33 | + | prepareErr := errors.New("prepare failed") | |
| 34 | + | shell := &popupStagingShellSpy{} | |
| 35 | + | originalNewShell := newPopupStagingWindow | |
| 36 | + | originalPrepare := preparePopupStagingContentWidget | |
| 37 | + | t.Cleanup(func() { | |
| 38 | + | newPopupStagingWindow = originalNewShell | |
| 39 | + | preparePopupStagingContentWidget = originalPrepare | |
| 40 | + | }) | |
| 41 | + | newPopupStagingWindow = func(context.Context, *gtk.Application) (popupStagingShell, error) { | |
| 42 | + | return shell, nil | |
| 43 | + | } | |
| 44 | + | preparePopupStagingContentWidget = func(layout.Widget) (*gtk.Widget, error) { | |
| 45 | + | return nil, prepareErr | |
| 46 | + | } | |
| 47 | + | ||
| 48 | + | app := &App{ | |
| 49 | + | gtkApp: >k.Application{}, | |
| 50 | + | contentCoord: content.NewCoordinator(context.Background(), nil, nil, nil, nil, nil, nil, nil), | |
| 51 | + | } | |
| 52 | + | host, err := app.stagePopup(context.Background(), content.StagePopupInput{ | |
| 53 | + | PopupWebView: portmocks.NewMockWebView(t), | |
| 54 | + | }) | |
| 55 | + | ||
| 56 | + | require.Nil(t, host) | |
| 57 | + | require.ErrorIs(t, err, prepareErr) | |
| 58 | + | assert.Equal(t, 1, shell.destroyCalls) | |
| 59 | + | } | |
| 60 | + | ||
| 27 | 61 | func TestPopupStagingHostDetachRemovesContentThenDestroysShell(t *testing.T) { | |
| 28 | 62 | t.Parallel() | |
| 29 | 63 | ||
| @@ -1,8 +1,13 @@ | |||||
| 1 | package ui | 1 | package ui | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "context" | |||
| 5 | + | "errors" | |||
| 4 | "testing" | 6 | "testing" | ||
| 5 | 7 | ||||
| 8 | + | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | |||
| 9 | + | "github.com/bnema/dumber/internal/ui/coordinator/content" | |||
| 10 | + | "github.com/bnema/dumber/internal/ui/layout" | |||
| 6 | "github.com/bnema/puregotk/v4/gtk" | 11 | "github.com/bnema/puregotk/v4/gtk" | ||
| 7 | "github.com/stretchr/testify/assert" | 12 | "github.com/stretchr/testify/assert" | ||
| 8 | "github.com/stretchr/testify/require" | 13 | "github.com/stretchr/testify/require" | ||
| @@ -24,6 +29,35 @@ func (s *popupStagingShellSpy) DetachContent() *gtk.Widget { | |||||
| 24 | } | 29 | } | ||
| 25 | func (s *popupStagingShellSpy) Destroy() { s.destroyCalls++ } | 30 | func (s *popupStagingShellSpy) Destroy() { s.destroyCalls++ } | ||
| 26 | 31 | ||||
| 32 | + | func TestStagePopupWrapsContentPreparationErrorAndDestroysShell(t *testing.T) { | |||
| 33 | + | prepareErr := errors.New("prepare failed") | |||
| 34 | + | shell := &popupStagingShellSpy{} | |||
| 35 | + | originalNewShell := newPopupStagingWindow | |||
| 36 | + | originalPrepare := preparePopupStagingContentWidget | |||
| 37 | + | t.Cleanup(func() { | |||
| 38 | + | newPopupStagingWindow = originalNewShell | |||
| 39 | + | preparePopupStagingContentWidget = originalPrepare | |||
| 40 | + | }) | |||
| 41 | + | newPopupStagingWindow = func(context.Context, *gtk.Application) (popupStagingShell, error) { | |||
| 42 | + | return shell, nil | |||
| 43 | + | } | |||
| 44 | + | preparePopupStagingContentWidget = func(layout.Widget) (*gtk.Widget, error) { | |||
| 45 | + | return nil, prepareErr | |||
| 46 | + | } | |||
| 47 | + | ||||
| 48 | + | app := &App{ | |||
| 49 | + | gtkApp: >k.Application{}, | |||
| 50 | + | contentCoord: content.NewCoordinator(context.Background(), nil, nil, nil, nil, nil, nil, nil), | |||
| 51 | + | } | |||
| 52 | + | host, err := app.stagePopup(context.Background(), content.StagePopupInput{ | |||
| 53 | + | PopupWebView: portmocks.NewMockWebView(t), | |||
| 54 | + | }) | |||
| 55 | + | ||||
| 56 | + | require.Nil(t, host) | |||
| 57 | + | require.ErrorIs(t, err, prepareErr) | |||
| 58 | + | assert.Equal(t, 1, shell.destroyCalls) | |||
| 59 | + | } | |||
| 60 | + | ||||
| 27 | func TestPopupStagingHostDetachRemovesContentThenDestroysShell(t *testing.T) { | 61 | func TestPopupStagingHostDetachRemovesContentThenDestroysShell(t *testing.T) { | ||
| 28 | t.Parallel() | 62 | t.Parallel() | ||
| 29 | 63 | ||||