Split browsing contexts between panes and native windows
Reason: OAuth, FedCM, and opener-coupled flows need real native popup semantics, while ordinary new-page requests should stay inside the workspace without popup emulation. Scope: - add browsing-context policy, config migration, and named-context reuse around workspace.browsing_contexts - route ordinary _blank / ctrl-click / window.open requests to related workspace panes - route auth and native-required flows to dedicated native popup windows that share the parent request context - remove synthetic opener / popup-proxy behavior and stabilize global shortcut repeat handling found during manual popup testing Description: This refactor treats CEF popup signals as browsing-context requests instead of pane popups. It adds a host-decision seam across the ports, coordinator, and CEF/WebKit implementations; introduces a native popup window host for auth flows; preserves inherited request contexts in both pane and native branches; updates config/docs/tests to the new browsing-context vocabulary; and hardens global shortcut suppression so manual popup-matrix testing no longer spuriously replays one-shot actions.
- commit
- 07857df841b230892e9d43755d0ba97cbec46a95
- author
- brice · · 2026-05-13 04:45 UTC
- parent
- 80f6f7b
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
41 files · +2709 -742
M docs/config/index.md +8 -8
| @@ -377,17 +377,17 @@ url = "https://github.com" | |||
| 377 | 377 | desc = "Open floating pane on GitHub" | |
| 378 | 378 | ``` | |
| 379 | 379 | ||
| 380 | - | ### Popup Behavior | |
| 380 | + | ### Browsing Context Behavior | |
| 381 | 381 | ||
| 382 | 382 | | Key | Type | Default | Valid Values | Description | | |
| 383 | 383 | |-----|------|---------|--------------|-------------| | |
| 384 | - | | `workspace.popups.behavior` | string | `"split"` | `split`, `stacked`, `tabbed`, `windowed` | Popup opening mode | | |
| 385 | - | | `workspace.popups.placement` | string | `"right"` | `right`, `left`, `top`, `bottom` | Split direction | | |
| 386 | - | | `workspace.popups.open_in_new_pane` | bool | `true` | - | Open popups in workspace | | |
| 387 | - | | `workspace.popups.follow_pane_context` | bool | `true` | - | Follow parent pane context | | |
| 388 | - | | `workspace.popups.blank_target_behavior` | string | `"pane"` | `pane`, `tab` | `_blank` target handling | | |
| 389 | - | | `workspace.popups.enable_smart_detection` | bool | `true` | - | Use WindowProperties for detection | | |
| 390 | - | | `workspace.popups.oauth_auto_close` | bool | `true` | - | Auto-close OAuth popups | | |
| 384 | + | | `workspace.browsing_contexts.behavior` | string | `"split"` | `split`, `stacked`, `tabbed`, `windowed` | Placement mode for script-opened browsing contexts | | |
| 385 | + | | `workspace.browsing_contexts.placement` | string | `"right"` | `right`, `left`, `top`, `bottom` | Split direction when behavior is `split` | | |
| 386 | + | | `workspace.browsing_contexts.open_in_new_pane` | bool | `true` | - | Allow new browsing contexts to open in the workspace | | |
| 387 | + | | `workspace.browsing_contexts.follow_pane_context` | bool | `true` | - | Keep new browsing contexts aligned with the parent pane context | | |
| 388 | + | | `workspace.browsing_contexts.blank_target_behavior` | string | `"stacked"` | `split`, `stacked`, `tabbed` | Placement mode for `_blank` / new-page link contexts | | |
| 389 | + | | `workspace.browsing_contexts.enable_smart_detection` | bool | `true` | - | Use window properties to refine browsing-context classification | | |
| 390 | + | | `workspace.browsing_contexts.oauth_auto_close` | bool | `true` | - | Auto-close OAuth browsing contexts after success | | |
| 391 | 391 | ||
| 392 | 392 | ### Workspace Styling | |
| 393 | 393 | ||
M docs/reference/configuration.md +7 -7
| @@ -79,13 +79,13 @@ | |||
| 79 | 79 | | `workspace.floating_pane.profiles.<name>.keys` | []string | | at least one key | | |
| 80 | 80 | | `workspace.floating_pane.profiles.<name>.url` | string | | required URL | | |
| 81 | 81 | | `workspace.floating_pane.profiles.<name>.desc` | string | | | | |
| 82 | - | | `workspace.popups.behavior` | string | `split` | `split`, `stacked`, `tabbed`, `windowed` | | |
| 83 | - | | `workspace.popups.placement` | string | `right` | `right`, `left`, `top`, `bottom` | | |
| 84 | - | | `workspace.popups.open_in_new_pane` | bool | `true` | | | |
| 85 | - | | `workspace.popups.follow_pane_context` | bool | `true` | | | |
| 86 | - | | `workspace.popups.blank_target_behavior` | string | `pane` | `pane`, `tab` | | |
| 87 | - | | `workspace.popups.enable_smart_detection` | bool | `true` | | | |
| 88 | - | | `workspace.popups.oauth_auto_close` | bool | `true` | | | |
| 82 | + | | `workspace.browsing_contexts.behavior` | string | `split` | `split`, `stacked`, `tabbed`, `windowed` | | |
| 83 | + | | `workspace.browsing_contexts.placement` | string | `right` | `right`, `left`, `top`, `bottom` | | |
| 84 | + | | `workspace.browsing_contexts.open_in_new_pane` | bool | `true` | | | |
| 85 | + | | `workspace.browsing_contexts.follow_pane_context` | bool | `true` | | | |
| 86 | + | | `workspace.browsing_contexts.blank_target_behavior` | string | `stacked` | `split`, `stacked`, `tabbed` | | |
| 87 | + | | `workspace.browsing_contexts.enable_smart_detection` | bool | `true` | | | |
| 88 | + | | `workspace.browsing_contexts.oauth_auto_close` | bool | `true` | | | |
| 89 | 89 | | `workspace.styling.border_width` | int | `1` | | | |
| 90 | 90 | | `workspace.styling.border_color` | string | `@theme_selected_bg_color` | | | |
| 91 | 91 | | `workspace.styling.mode_border_width` | int | `4` | | | |
A internal/application/port/browsing_context.go +110 -0
| @@ -0,0 +1,110 @@ | |||
| 1 | + | // Package port defines interfaces (ports) for use cases to depend on. | |
| 2 | + | // These types are the seam for the approved browsing-context architecture. | |
| 3 | + | // They are standalone for now; no callbacks are wired yet. | |
| 4 | + | ||
| 5 | + | package port | |
| 6 | + | ||
| 7 | + | // WindowDisposition describes how the browsing context should be presented | |
| 8 | + | // relative to the source frame. Values mirror Chromium/CEF/WebKit navigation | |
| 9 | + | // dispositions at a normalized product seam. | |
| 10 | + | type WindowDisposition string | |
| 11 | + | ||
| 12 | + | const ( | |
| 13 | + | // WindowDispositionCurrentTab replaces the current document in the same browsing context. | |
| 14 | + | WindowDispositionCurrentTab WindowDisposition = "current-tab" | |
| 15 | + | // WindowDispositionNewTab opens in a new tab-like context. | |
| 16 | + | WindowDispositionNewTab WindowDisposition = "new-tab" | |
| 17 | + | // WindowDispositionNewPopup opens in a popup-style context. | |
| 18 | + | WindowDispositionNewPopup WindowDisposition = "new-popup" | |
| 19 | + | // WindowDispositionNewWindow opens in a new top-level window. | |
| 20 | + | WindowDispositionNewWindow WindowDisposition = "new-window" | |
| 21 | + | ) | |
| 22 | + | ||
| 23 | + | // TriggerKind categorizes what triggered a browsing-context request. | |
| 24 | + | type TriggerKind string | |
| 25 | + | ||
| 26 | + | const ( | |
| 27 | + | TriggerUnknown TriggerKind = "unknown" | |
| 28 | + | TriggerLinkNewPage TriggerKind = "link-new-page" | |
| 29 | + | TriggerScriptWindowOpen TriggerKind = "script-window-open" | |
| 30 | + | TriggerNamedTargetNavigation TriggerKind = "named-target-navigation" | |
| 31 | + | TriggerAuthPopupRequest TriggerKind = "auth-popup-request" | |
| 32 | + | ) | |
| 33 | + | ||
| 34 | + | // RequestContextDisposition makes request-context inheritance explicit at the | |
| 35 | + | // policy seam without leaking infrastructure request-context types upward. | |
| 36 | + | type RequestContextDisposition string | |
| 37 | + | ||
| 38 | + | const ( | |
| 39 | + | // RequestContextInheritParent preserves the parent request context/session. | |
| 40 | + | RequestContextInheritParent RequestContextDisposition = "inherit-parent" | |
| 41 | + | // RequestContextIsolate starts from a fresh unrelated request context. | |
| 42 | + | RequestContextIsolate RequestContextDisposition = "isolate" | |
| 43 | + | ) | |
| 44 | + | ||
| 45 | + | // NewBrowsingContextRequest is the normalized request captured from engine/UI | |
| 46 | + | // signals before policy decides how it should be hosted. | |
| 47 | + | type NewBrowsingContextRequest struct { | |
| 48 | + | ParentWebViewID WebViewID | |
| 49 | + | ||
| 50 | + | SourceBrowserID int32 | |
| 51 | + | SourceFrameID string | |
| 52 | + | SourceFrameURL string | |
| 53 | + | ||
| 54 | + | TargetURI string | |
| 55 | + | TargetFrameName string | |
| 56 | + | TargetDisposition WindowDisposition | |
| 57 | + | ||
| 58 | + | IsUserGesture bool | |
| 59 | + | NoJavaScriptAccess bool | |
| 60 | + | WindowFeatures string | |
| 61 | + | ||
| 62 | + | TriggerKind TriggerKind | |
| 63 | + | AuthIntent bool | |
| 64 | + | RequiresNativeOpener bool | |
| 65 | + | RequestContextDisposition RequestContextDisposition | |
| 66 | + | } | |
| 67 | + | ||
| 68 | + | // HostDecisionKind categorizes the approved host choices for a new browsing context. | |
| 69 | + | type HostDecisionKind string | |
| 70 | + | ||
| 71 | + | const ( | |
| 72 | + | HostDecisionReuseNamedPane HostDecisionKind = "reuse-named-pane" | |
| 73 | + | HostDecisionCreatePane HostDecisionKind = "create-pane" | |
| 74 | + | HostDecisionCreateNativeWin HostDecisionKind = "create-native-window" | |
| 75 | + | HostDecisionDeny HostDecisionKind = "deny" | |
| 76 | + | ) | |
| 77 | + | ||
| 78 | + | // HostDecision represents the output of browsing-context policy. | |
| 79 | + | type HostDecision struct { | |
| 80 | + | Kind HostDecisionKind | |
| 81 | + | ||
| 82 | + | ReuseContextName string | |
| 83 | + | BrowsingContextName string | |
| 84 | + | ||
| 85 | + | RequestContextDisposition RequestContextDisposition | |
| 86 | + | RequiresNativeOpener bool | |
| 87 | + | Reason string | |
| 88 | + | } | |
| 89 | + | ||
| 90 | + | // PaneHostedBrowsingContextCapable is implemented by engines that need an | |
| 91 | + | // explicit transition from a related/native-popup shell into a pane-hosted | |
| 92 | + | // child browsing context path. | |
| 93 | + | type PaneHostedBrowsingContextCapable interface { | |
| 94 | + | PreparePaneHostedBrowsingContext() | |
| 95 | + | } | |
| 96 | + | ||
| 97 | + | // BrowsingContextHostDecisionCapable lets UI coordinators attach the chosen | |
| 98 | + | // host decision to an engine webview so lower-level popup handlers can honor | |
| 99 | + | // pane-vs-native behavior synchronously. | |
| 100 | + | type BrowsingContextHostDecisionCapable interface { | |
| 101 | + | SetBrowsingContextHostDecision(decision HostDecision) | |
| 102 | + | BrowsingContextHostDecision() (HostDecision, bool) | |
| 103 | + | } | |
| 104 | + | ||
| 105 | + | // NativePopupHostAbortCapable lets the engine-level popup seam abort an | |
| 106 | + | // already-created native popup shell/window when native arming fails. | |
| 107 | + | type NativePopupHostAbortCapable interface { | |
| 108 | + | SetNativePopupHostAbort(fn func()) | |
| 109 | + | AbortNativePopupHost() | |
| 110 | + | } | |
M internal/application/port/webview.go +7 -0
| @@ -67,6 +67,13 @@ type PopupRequest struct { | |||
| 67 | 67 | IsUserGesture bool | |
| 68 | 68 | NoJavaScriptAccess bool // true for noopener/noreferrer-style popups with no opener access | |
| 69 | 69 | ParentViewID WebViewID | |
| 70 | + | ||
| 71 | + | SourceBrowserID int32 | |
| 72 | + | SourceFrameID string | |
| 73 | + | SourceFrameURL string | |
| 74 | + | ||
| 75 | + | TargetDisposition WindowDisposition | |
| 76 | + | WindowFeatures string | |
| 70 | 77 | } | |
| 71 | 78 | ||
| 72 | 79 | // Texture represents a graphics texture returned by the engine. | |
M internal/domain/entity/config_types.go +14 -3
| @@ -166,8 +166,9 @@ const ( | |||
| 166 | 166 | OmniboxInitialBehaviorNone OmniboxInitialBehavior = "none" | |
| 167 | 167 | ) | |
| 168 | 168 | ||
| 169 | - | // PopupBehaviorConfig controls how popup windows are handled. | |
| 170 | - | type PopupBehaviorConfig struct { | |
| 169 | + | // BrowsingContextConfig controls how browsing contexts (popups, tabs, new windows) are handled. | |
| 170 | + | // This is the canonical config type; PopupBehaviorConfig is a compatibility alias. | |
| 171 | + | type BrowsingContextConfig struct { | |
| 171 | 172 | Behavior PopupBehavior `mapstructure:"behavior" yaml:"behavior" toml:"behavior" json:"behavior"` | |
| 172 | 173 | ||
| 173 | 174 | Placement string `mapstructure:"placement" yaml:"placement" toml:"placement" json:"placement"` | |
| @@ -183,6 +184,9 @@ type PopupBehaviorConfig struct { | |||
| 183 | 184 | OAuthAutoClose bool `mapstructure:"oauth_auto_close" yaml:"oauth_auto_close" toml:"oauth_auto_close" json:"oauth_auto_close"` | |
| 184 | 185 | } | |
| 185 | 186 | ||
| 187 | + | // Deprecated: PopupBehaviorConfig is a compatibility alias for BrowsingContextConfig. | |
| 188 | + | type PopupBehaviorConfig = BrowsingContextConfig | |
| 189 | + | ||
| 186 | 190 | // WorkspaceConfig holds all workspace layout and behavior settings. | |
| 187 | 191 | type WorkspaceConfig struct { | |
| 188 | 192 | NewPaneURL string `mapstructure:"new_pane_url" yaml:"new_pane_url" toml:"new_pane_url" json:"new_pane_url"` | |
| @@ -196,7 +200,14 @@ type WorkspaceConfig struct { | |||
| 196 | 200 | HideTabBarWhenSingleTab bool `mapstructure:"hide_tab_bar_when_single_tab" yaml:"hide_tab_bar_when_single_tab" toml:"hide_tab_bar_when_single_tab" json:"hide_tab_bar_when_single_tab"` //nolint:lll // struct tags must stay on one line | |
| 197 | 201 | SwitchToTabOnMove bool `mapstructure:"switch_to_tab_on_move" yaml:"switch_to_tab_on_move" toml:"switch_to_tab_on_move" json:"switch_to_tab_on_move"` //nolint:lll // struct tags must stay on one line | |
| 198 | 202 | ||
| 199 | - | Popups PopupBehaviorConfig `mapstructure:"popups" yaml:"popups" toml:"popups" json:"popups"` | |
| 203 | + | // BrowsingContexts is the canonical field for browsing context behavior. | |
| 204 | + | // It replaces the legacy popups configuration. | |
| 205 | + | BrowsingContexts BrowsingContextConfig `mapstructure:"browsing_contexts" yaml:"browsing_contexts" toml:"browsing_contexts" json:"browsing_contexts"` //nolint:lll // struct tags must stay on one line | |
| 206 | + | ||
| 207 | + | // Popups is a runtime compatibility alias synced from BrowsingContexts during normalization. | |
| 208 | + | // It is not serialized or deserialized from config files. | |
| 209 | + | Popups BrowsingContextConfig `mapstructure:"-" yaml:"-" toml:"-" json:"-"` | |
| 210 | + | ||
| 200 | 211 | Styling WorkspaceStylingConfig `mapstructure:"styling" yaml:"styling" toml:"styling" json:"styling"` | |
| 201 | 212 | } | |
| 202 | 213 | ||
M internal/infrastructure/cef/bridge_script_templates.go +3 -231
| @@ -1,7 +1,6 @@ | |||
| 1 | 1 | package cef | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | - | "fmt" | |
| 5 | 4 | "strings" | |
| 6 | 5 | ||
| 7 | 6 | "github.com/bnema/dumber/internal/infrastructure/webutil" | |
| @@ -12,234 +11,11 @@ import ( | |||
| 12 | 11 | // bridge nonce placeholder is replaced per navigation. | |
| 13 | 12 | var trustedPageFetchBridgeBaseJS = buildTrustedPageFetchBridgeBaseJS() | |
| 14 | 13 | ||
| 15 | - | // rendererBridgeExtensionJS is assembled once from the shared popup proxy | |
| 16 | - | // builder so the renderer-extension and fetch-based popup bridges stay aligned. | |
| 14 | + | // rendererBridgeExtensionJS is assembled once for renderer-process bridge helpers. | |
| 17 | 15 | var rendererBridgeExtensionJS = buildRendererBridgeExtensionJS() | |
| 18 | 16 | ||
| 19 | - | type popupProxyBridgeDispatch struct { | |
| 20 | - | patchFlag string | |
| 21 | - | helpers string | |
| 22 | - | } | |
| 23 | - | ||
| 24 | - | const popupProxyBridgeBodyTemplateJS = ` | |
| 25 | - | if (typeof window === 'undefined') return; | |
| 26 | - | if (window.__DUMBER_POPUP_PATCH_FLAG__) return; | |
| 27 | - | ||
| 28 | - | __DUMBER_POPUP_DISPATCH_HELPERS__ | |
| 29 | - | ||
| 30 | - | function resolvePopupURL(rawURL) { | |
| 31 | - | if (rawURL == null || rawURL === '') return ''; | |
| 32 | - | try { | |
| 33 | - | return new URL(String(rawURL), (document && document.baseURI) || location.href).href; | |
| 34 | - | } catch (_) { | |
| 35 | - | return String(rawURL); | |
| 36 | - | } | |
| 37 | - | } | |
| 38 | - | ||
| 39 | - | function hasUserGesture() { | |
| 40 | - | try { | |
| 41 | - | return !!(navigator && navigator.userActivation && navigator.userActivation.isActive); | |
| 42 | - | } catch (_) { | |
| 43 | - | return false; | |
| 44 | - | } | |
| 45 | - | } | |
| 46 | - | ||
| 47 | - | function popupHasNoOpener(features) { | |
| 48 | - | if (features == null || features === '') return false; | |
| 49 | - | var normalized = String(features).toLowerCase(); | |
| 50 | - | return normalized.indexOf('noopener') !== -1 || normalized.indexOf('noreferrer') !== -1; | |
| 51 | - | } | |
| 52 | - | ||
| 53 | - | function shouldDelegateWindowOpen(target) { | |
| 54 | - | return target === '_self' || target === '_top' || target === '_parent'; | |
| 55 | - | } | |
| 56 | - | ||
| 57 | - | function createPopupProxyID() { | |
| 58 | - | var timestamp = Date.now(); | |
| 59 | - | try { | |
| 60 | - | if (typeof crypto !== 'undefined' && crypto && typeof crypto.getRandomValues === 'function') { | |
| 61 | - | var bytes = new Uint8Array(4); | |
| 62 | - | crypto.getRandomValues(bytes); | |
| 63 | - | var nonce = ''; | |
| 64 | - | for (var i = 0; i < bytes.length; i++) { | |
| 65 | - | nonce += ('0' + bytes[i].toString(16)).slice(-2); | |
| 66 | - | } | |
| 67 | - | return 'popup-' + timestamp + '-' + nonce; | |
| 68 | - | } | |
| 69 | - | } catch (_) {} | |
| 70 | - | return 'popup-' + timestamp + '-' + Math.random().toString(36).slice(2); | |
| 71 | - | } | |
| 72 | - | ||
| 73 | - | function createSyntheticPopupProxy(proxyID, initialURL, features) { | |
| 74 | - | var href = initialURL || 'about:blank'; | |
| 75 | - | var closed = false; | |
| 76 | - | var noJavaScriptAccess = popupHasNoOpener(features); | |
| 77 | - | ||
| 78 | - | function navigate(nextURL) { | |
| 79 | - | href = resolvePopupURL(nextURL); | |
| 80 | - | dispatchPopupNavigate(proxyID, href); | |
| 81 | - | return href; | |
| 82 | - | } | |
| 83 | - | ||
| 84 | - | var locationProxy = { | |
| 85 | - | assign: function(nextURL) { return navigate(nextURL); }, | |
| 86 | - | replace: function(nextURL) { return navigate(nextURL); }, | |
| 87 | - | toString: function() { return href; } | |
| 88 | - | }; | |
| 89 | - | try { | |
| 90 | - | Object.defineProperty(locationProxy, 'href', { | |
| 91 | - | configurable: true, | |
| 92 | - | enumerable: true, | |
| 93 | - | get: function() { return href; }, | |
| 94 | - | set: function(nextURL) { navigate(nextURL); } | |
| 95 | - | }); | |
| 96 | - | } catch (_) { | |
| 97 | - | locationProxy.href = href; | |
| 98 | - | } | |
| 99 | - | ||
| 100 | - | var proxy = { | |
| 101 | - | blur: function() { return undefined; }, | |
| 102 | - | close: function() { | |
| 103 | - | if (closed) return; | |
| 104 | - | closed = true; | |
| 105 | - | dispatchPopupClose(proxyID); | |
| 106 | - | }, | |
| 107 | - | focus: function() { return undefined; }, | |
| 108 | - | postMessage: function() { return undefined; } | |
| 109 | - | }; | |
| 110 | - | proxy.opener = noJavaScriptAccess ? null : window; | |
| 111 | - | proxy.self = proxy; | |
| 112 | - | proxy.window = proxy; | |
| 113 | - | ||
| 114 | - | try { | |
| 115 | - | Object.defineProperty(proxy, 'closed', { | |
| 116 | - | configurable: true, | |
| 117 | - | enumerable: true, | |
| 118 | - | get: function() { return closed; }, | |
| 119 | - | set: function(next) { closed = !!next; } | |
| 120 | - | }); | |
| 121 | - | } catch (_) { | |
| 122 | - | proxy.closed = false; | |
| 123 | - | } | |
| 124 | - | ||
| 125 | - | try { | |
| 126 | - | Object.defineProperty(proxy, 'location', { | |
| 127 | - | configurable: true, | |
| 128 | - | enumerable: true, | |
| 129 | - | get: function() { return locationProxy; }, | |
| 130 | - | set: function(nextURL) { navigate(nextURL); } | |
| 131 | - | }); | |
| 132 | - | } catch (_) { | |
| 133 | - | proxy.location = locationProxy; | |
| 134 | - | } | |
| 135 | - | ||
| 136 | - | try { | |
| 137 | - | Object.defineProperty(proxy, Symbol.toStringTag, { | |
| 138 | - | configurable: true, | |
| 139 | - | value: 'Window' | |
| 140 | - | }); | |
| 141 | - | } catch (_) {} | |
| 142 | - | ||
| 143 | - | return proxy; | |
| 144 | - | } | |
| 145 | - | ||
| 146 | - | try { | |
| 147 | - | var originalWindowOpen = typeof window.open === 'function' ? window.open : null; | |
| 148 | - | if (!originalWindowOpen) return; | |
| 149 | - | ||
| 150 | - | window.__DUMBER_POPUP_PATCH_FLAG__ = true; | |
| 151 | - | window.open = function(url, target, features) { | |
| 152 | - | var normalizedTarget = target == null ? '' : String(target); | |
| 153 | - | if (shouldDelegateWindowOpen(normalizedTarget)) { | |
| 154 | - | return originalWindowOpen.apply(this, arguments); | |
| 155 | - | } | |
| 156 | - | ||
| 157 | - | var proxyID = createPopupProxyID(); | |
| 158 | - | var resolvedURL = resolvePopupURL(url); | |
| 159 | - | var noJavaScriptAccess = popupHasNoOpener(features); | |
| 160 | - | var popupProxy = createSyntheticPopupProxy(proxyID, resolvedURL, features); | |
| 161 | - | dispatchPopupOpen({ | |
| 162 | - | proxy_id: proxyID, | |
| 163 | - | url: resolvedURL, | |
| 164 | - | frame_name: normalizedTarget, | |
| 165 | - | user_gesture: hasUserGesture(), | |
| 166 | - | no_javascript_access: noJavaScriptAccess | |
| 167 | - | }); | |
| 168 | - | return popupProxy; | |
| 169 | - | }; | |
| 170 | - | } catch (_) { | |
| 171 | - | window.__DUMBER_POPUP_PATCH_FLAG__ = false; | |
| 172 | - | } | |
| 173 | - | ` | |
| 174 | - | ||
| 175 | - | const popupFetchBridgeHelpersJSTemplate = ` | |
| 176 | - | var bridgeNonce = '__DUMBER_BRIDGE_NONCE__'; | |
| 177 | - | ||
| 178 | - | function encodeBody(body) { | |
| 179 | - | try { | |
| 180 | - | return btoa(unescape(encodeURIComponent(body))); | |
| 181 | - | } catch (_) { | |
| 182 | - | return ''; | |
| 183 | - | } | |
| 184 | - | } | |
| 185 | - | ||
| 186 | - | function postBridge(url, payload) { | |
| 187 | - | var encoded = encodeBody(JSON.stringify(payload)); | |
| 188 | - | if (!encoded) return; | |
| 189 | - | fetch(url, { | |
| 190 | - | method: 'POST', | |
| 191 | - | headers: { | |
| 192 | - | 'X-Dumber-Body': encoded, | |
| 193 | - | 'X-Dumber-Bridge-Nonce': bridgeNonce | |
| 194 | - | } | |
| 195 | - | }).catch(function() {}); | |
| 196 | - | } | |
| 197 | - | ||
| 198 | - | function dispatchPopupOpen(payload) { | |
| 199 | - | postBridge('dumb:///api/popup-open', payload); | |
| 200 | - | } | |
| 201 | - | ||
| 202 | - | function dispatchPopupNavigate(proxyID, url) { | |
| 203 | - | postBridge('dumb:///api/popup-navigate', { proxy_id: proxyID, url: url }); | |
| 204 | - | } | |
| 205 | - | ||
| 206 | - | function dispatchPopupClose(proxyID) { | |
| 207 | - | postBridge('dumb:///api/popup-close', { proxy_id: proxyID }); | |
| 208 | - | } | |
| 209 | - | ` | |
| 210 | - | ||
| 211 | - | const rendererPopupDispatchHelpersJS = ` | |
| 212 | - | function dispatchPopupOpen(payload) { | |
| 213 | - | send('popup_open', JSON.stringify(payload)); | |
| 214 | - | } | |
| 215 | - | ||
| 216 | - | function dispatchPopupNavigate(proxyID, url) { | |
| 217 | - | send('popup_navigate', JSON.stringify({ proxy_id: proxyID, url: url })); | |
| 218 | - | } | |
| 219 | - | ||
| 220 | - | function dispatchPopupClose(proxyID) { | |
| 221 | - | send('popup_close', JSON.stringify({ proxy_id: proxyID })); | |
| 222 | - | } | |
| 223 | - | ` | |
| 224 | - | ||
| 225 | - | func buildPopupProxyBridgeBodyJS(dispatch popupProxyBridgeDispatch) string { | |
| 226 | - | replacer := strings.NewReplacer( | |
| 227 | - | "__DUMBER_POPUP_PATCH_FLAG__", dispatch.patchFlag, | |
| 228 | - | "__DUMBER_POPUP_DISPATCH_HELPERS__", dispatch.helpers, | |
| 229 | - | ) | |
| 230 | - | return replacer.Replace(popupProxyBridgeBodyTemplateJS) | |
| 231 | - | } | |
| 232 | - | ||
| 233 | - | func wrapSelfExecutingScript(body string) string { | |
| 234 | - | return fmt.Sprintf("(function() {\n%s\n})();", body) | |
| 235 | - | } | |
| 236 | - | ||
| 237 | 17 | func buildTrustedPageFetchBridgeBaseJS() string { | |
| 238 | - | popupBridge := wrapSelfExecutingScript(buildPopupProxyBridgeBodyJS(popupProxyBridgeDispatch{ | |
| 239 | - | patchFlag: "__dumberPopupBridgePatched", | |
| 240 | - | helpers: popupFetchBridgeHelpersJSTemplate, | |
| 241 | - | })) | |
| 242 | - | return clipboardSelectionFetchBridgeJSTemplate + "\n" + popupBridge | |
| 18 | + | return clipboardSelectionFetchBridgeJSTemplate | |
| 243 | 19 | } | |
| 244 | 20 | ||
| 245 | 21 | const bridgeNoncePlaceholder = "__DUMBER_BRIDGE_NONCE__" | |
| @@ -252,11 +28,7 @@ func buildTrustedPageFetchBridgeJS(bridgeNonce string) string { | |||
| 252 | 28 | } | |
| 253 | 29 | ||
| 254 | 30 | func buildRendererBridgeExtensionJS() string { | |
| 255 | - | popupBridgeBody := buildPopupProxyBridgeBodyJS(popupProxyBridgeDispatch{ | |
| 256 | - | patchFlag: "__dumberPopupOpenPatched", | |
| 257 | - | helpers: rendererPopupDispatchHelpersJS, | |
| 258 | - | }) | |
| 259 | - | return strings.ReplaceAll(rendererBridgeExtensionTemplateJS, "__DUMBER_POPUP_PROXY_BRIDGE_BODY__", popupBridgeBody) | |
| 31 | + | return strings.ReplaceAll(rendererBridgeExtensionTemplateJS, "__DUMBER_POPUP_PROXY_BRIDGE_BODY__", "") | |
| 260 | 32 | } | |
| 261 | 33 | ||
| 262 | 34 | func buildPopupOpenerBridgeJS(bridgeNonce, parentURI string) string { | |
M internal/infrastructure/cef/content_injector_test.go +7 -12
| @@ -63,20 +63,15 @@ func TestClipboardSelectionFetchBridgeJS_PatchesAsyncClipboardAPIs(t *testing.T) | |||
| 63 | 63 | require.Contains(t, script, "test-bridge-nonce") | |
| 64 | 64 | } | |
| 65 | 65 | ||
| 66 | - | func TestTrustedPageFetchBridgeJS_ShimsWindowOpenToBridgePopupRequests(t *testing.T) { | |
| 66 | + | func TestTrustedPageFetchBridgeJS_DoesNotShimWindowOpen(t *testing.T) { | |
| 67 | 67 | script := testTrustedPageFetchBridgeJS() | |
| 68 | 68 | ||
| 69 | - | require.Contains(t, script, "window.__dumberPopupBridgePatched") | |
| 70 | - | require.Contains(t, script, "window.open = function(url, target, features)") | |
| 71 | - | require.Contains(t, script, "dumb:///api/popup-open") | |
| 72 | - | require.Contains(t, script, "dumb:///api/popup-navigate") | |
| 73 | - | require.Contains(t, script, "dumb:///api/popup-close") | |
| 74 | - | require.Contains(t, script, "proxy_id") | |
| 75 | - | require.Contains(t, script, "createSyntheticPopupProxy") | |
| 76 | - | require.Contains(t, script, "createPopupProxyID") | |
| 77 | - | require.Contains(t, script, "crypto.getRandomValues") | |
| 78 | - | require.Contains(t, script, "no_javascript_access") | |
| 79 | - | require.Contains(t, script, "Object.defineProperty(proxy, 'closed'") | |
| 69 | + | require.NotContains(t, script, "window.__dumberPopupBridgePatched") | |
| 70 | + | require.NotContains(t, script, "window.open = function(url, target, features)") | |
| 71 | + | require.NotContains(t, script, "dumb:///api/popup-open") | |
| 72 | + | require.NotContains(t, script, "dumb:///api/popup-navigate") | |
| 73 | + | require.NotContains(t, script, "dumb:///api/popup-close") | |
| 74 | + | require.NotContains(t, script, "createSyntheticPopupProxy") | |
| 80 | 75 | require.Contains(t, script, "test-bridge-nonce") | |
| 81 | 76 | require.NotContains(t, script, "__DUMBER_BRIDGE_NONCE__") | |
| 82 | 77 | } | |
M internal/infrastructure/cef/renderer_bridge_test.go +7 -9
| @@ -97,15 +97,13 @@ func TestRendererBridgeExtensionJS_UsesNativeDispatchInExtensionScope(t *testing | |||
| 97 | 97 | require.Contains(t, rendererBridgeExtensionJS, "send('bridge_ready',") | |
| 98 | 98 | } | |
| 99 | 99 | ||
| 100 | - | func TestRendererBridgeExtensionJS_ShimsWindowOpenForSyntheticPopups(t *testing.T) { | |
| 101 | - | require.Contains(t, rendererBridgeExtensionJS, "window.__dumberPopupOpenPatched") | |
| 102 | - | require.Contains(t, rendererBridgeExtensionJS, "function dispatchPopupOpen(payload)") | |
| 103 | - | require.Contains(t, rendererBridgeExtensionJS, "send('popup_open', JSON.stringify(payload));") | |
| 104 | - | require.Contains(t, rendererBridgeExtensionJS, "dispatchPopupNavigate(proxyID, href)") | |
| 105 | - | require.Contains(t, rendererBridgeExtensionJS, "dispatchPopupClose(proxyID)") | |
| 106 | - | require.Contains(t, rendererBridgeExtensionJS, "no_javascript_access") | |
| 107 | - | require.Contains(t, rendererBridgeExtensionJS, "Object.defineProperty(proxy, 'closed'") | |
| 108 | - | require.Contains(t, rendererBridgeExtensionJS, "return popupProxy;") | |
| 100 | + | func TestRendererBridgeExtensionJS_DoesNotShimWindowOpenForSyntheticPopups(t *testing.T) { | |
| 101 | + | require.NotContains(t, rendererBridgeExtensionJS, "window.__dumberPopupOpenPatched") | |
| 102 | + | require.NotContains(t, rendererBridgeExtensionJS, "function dispatchPopupOpen(payload)") | |
| 103 | + | require.NotContains(t, rendererBridgeExtensionJS, "send('popup_open', JSON.stringify(payload));") | |
| 104 | + | require.NotContains(t, rendererBridgeExtensionJS, "dispatchPopupNavigate(proxyID, href)") | |
| 105 | + | require.NotContains(t, rendererBridgeExtensionJS, "dispatchPopupClose(proxyID)") | |
| 106 | + | require.NotContains(t, rendererBridgeExtensionJS, "return popupProxy;") | |
| 109 | 107 | } | |
| 110 | 108 | ||
| 111 | 109 | func TestRendererBridgeExtensionJS_EncodesTrustedSuccessSemantics(t *testing.T) { | |
M internal/infrastructure/cef/webview.go +54 -25
| @@ -117,9 +117,12 @@ type WebView struct { | |||
| 117 | 117 | // crash → redirect → crash loops. | |
| 118 | 118 | crashCount atomic.Int32 | |
| 119 | 119 | ||
| 120 | - | // Callbacks set by use case layer. | |
| 121 | - | mu sync.RWMutex | |
| 122 | - | callbacks *port.WebViewCallbacks | |
| 120 | + | // Callbacks and browsing-context state set by the UI layer. | |
| 121 | + | mu sync.RWMutex | |
| 122 | + | callbacks *port.WebViewCallbacks | |
| 123 | + | browsingContextDecision port.HostDecision | |
| 124 | + | hasBrowsingContextDecision bool | |
| 125 | + | nativePopupHostAbort func() | |
| 123 | 126 | ||
| 124 | 127 | // Synthetic popup proxies created by the renderer bridge's window.open shim. | |
| 125 | 128 | syntheticPopupMu sync.Mutex | |
| @@ -656,33 +659,64 @@ func (wv *WebView) isNativePopupCandidate() bool { | |||
| 656 | 659 | return wv.nativePopupCandidate | |
| 657 | 660 | } | |
| 658 | 661 | ||
| 659 | - | func popupParentWindowHandle(parent *WebView) uintptr { | |
| 660 | - | if parent == nil { | |
| 661 | - | return 0 | |
| 662 | + | func (wv *WebView) discardNativePopupCandidate() { | |
| 663 | + | if wv == nil { | |
| 664 | + | return | |
| 662 | 665 | } | |
| 663 | - | parent.mu.RLock() | |
| 664 | - | host := parent.host | |
| 665 | - | parent.mu.RUnlock() | |
| 666 | - | if host == nil { | |
| 667 | - | return 0 | |
| 666 | + | wv.mu.Lock() | |
| 667 | + | wv.nativePopupCandidate = false | |
| 668 | + | wv.nativePopupParent = nil | |
| 669 | + | wv.nativePopupID = 0 | |
| 670 | + | wv.mu.Unlock() | |
| 671 | + | } | |
| 672 | + | ||
| 673 | + | func (wv *WebView) PreparePaneHostedBrowsingContext() { | |
| 674 | + | if wv == nil { | |
| 675 | + | return | |
| 668 | 676 | } | |
| 669 | - | if handle := host.GetWindowHandle(); handle != 0 { | |
| 670 | - | return handle | |
| 677 | + | wv.discardNativePopupCandidate() | |
| 678 | + | } | |
| 679 | + | ||
| 680 | + | func (wv *WebView) SetBrowsingContextHostDecision(decision port.HostDecision) { | |
| 681 | + | if wv == nil { | |
| 682 | + | return | |
| 671 | 683 | } | |
| 672 | - | return host.GetOpenerWindowHandle() | |
| 684 | + | wv.mu.Lock() | |
| 685 | + | wv.browsingContextDecision = decision | |
| 686 | + | wv.hasBrowsingContextDecision = true | |
| 687 | + | wv.mu.Unlock() | |
| 673 | 688 | } | |
| 674 | 689 | ||
| 675 | - | func (wv *WebView) discardNativePopupCandidate() { | |
| 690 | + | func (wv *WebView) BrowsingContextHostDecision() (port.HostDecision, bool) { | |
| 691 | + | if wv == nil { | |
| 692 | + | return port.HostDecision{}, false | |
| 693 | + | } | |
| 694 | + | wv.mu.RLock() | |
| 695 | + | defer wv.mu.RUnlock() | |
| 696 | + | return wv.browsingContextDecision, wv.hasBrowsingContextDecision | |
| 697 | + | } | |
| 698 | + | ||
| 699 | + | func (wv *WebView) SetNativePopupHostAbort(fn func()) { | |
| 676 | 700 | if wv == nil { | |
| 677 | 701 | return | |
| 678 | 702 | } | |
| 679 | 703 | wv.mu.Lock() | |
| 680 | - | wv.nativePopupCandidate = false | |
| 681 | - | wv.nativePopupParent = nil | |
| 682 | - | wv.nativePopupID = 0 | |
| 704 | + | wv.nativePopupHostAbort = fn | |
| 683 | 705 | wv.mu.Unlock() | |
| 684 | 706 | } | |
| 685 | 707 | ||
| 708 | + | func (wv *WebView) AbortNativePopupHost() { | |
| 709 | + | if wv == nil { | |
| 710 | + | return | |
| 711 | + | } | |
| 712 | + | wv.mu.RLock() | |
| 713 | + | fn := wv.nativePopupHostAbort | |
| 714 | + | wv.mu.RUnlock() | |
| 715 | + | if fn != nil { | |
| 716 | + | fn() | |
| 717 | + | } | |
| 718 | + | } | |
| 719 | + | ||
| 686 | 720 | func (wv *WebView) awaitsNativePopupAttachment() bool { | |
| 687 | 721 | if wv == nil { | |
| 688 | 722 | return false | |
| @@ -836,10 +870,6 @@ func (wv *WebView) prepareNativePopup( | |||
| 836 | 870 | if !ok { | |
| 837 | 871 | return false | |
| 838 | 872 | } | |
| 839 | - | parentWindowHandle := popupParentWindowHandle(prep.parent) | |
| 840 | - | if parentWindowHandle == 0 { | |
| 841 | - | return false | |
| 842 | - | } | |
| 843 | 873 | client, ok := wv.activateNativePopup(popupID, targetURL) | |
| 844 | 874 | if !ok { | |
| 845 | 875 | return false | |
| @@ -848,7 +878,7 @@ func (wv *WebView) prepareNativePopup( | |||
| 848 | 878 | prep.parent.trackPendingNativePopup(popupID, wv) | |
| 849 | 879 | } | |
| 850 | 880 | ||
| 851 | - | configureNativePopupWindow(windowInfo, settings, parentWindowHandle, prep.frameRate, prep.backgroundColor) | |
| 881 | + | configureNativePopupWindow(windowInfo, settings, prep.frameRate, prep.backgroundColor) | |
| 852 | 882 | clientSlot.Set(client) | |
| 853 | 883 | return true | |
| 854 | 884 | } | |
| @@ -900,11 +930,10 @@ func (wv *WebView) canPrepareNativePopupLocked() bool { | |||
| 900 | 930 | func configureNativePopupWindow( | |
| 901 | 931 | windowInfo *purecef.WindowInfo, | |
| 902 | 932 | settings *purecef.BrowserSettings, | |
| 903 | - | parentWindowHandle uintptr, | |
| 904 | 933 | frameRate int32, | |
| 905 | 934 | backgroundColor uint32, | |
| 906 | 935 | ) { | |
| 907 | - | purecef.SetAsWindowless(windowInfo, purecef.WindowHandle(parentWindowHandle), false) | |
| 936 | + | cef2gtk.ConfigureWindowInfo(windowInfo, cef2gtk.WindowInfoOptions{}) | |
| 908 | 937 | if externalBeginFrameEnabled() { | |
| 909 | 938 | windowInfo.ExternalBeginFrameEnabled = 1 | |
| 910 | 939 | } | |
M internal/infrastructure/cef/webview_handlers.go +79 -11
| @@ -419,6 +419,24 @@ func (h *handlerSet) OnLoadEnd(_ purecef.Browser, frame purecef.Frame, httpStatu | |||
| 419 | 419 | func (h *handlerSet) OnLoadError(_ purecef.Browser, _ purecef.Frame, _ purecef.Errorcode, _, _ string) { | |
| 420 | 420 | } | |
| 421 | 421 | ||
| 422 | + | func mapCEFWindowDisposition(disposition purecef.WindowOpenDisposition) port.WindowDisposition { | |
| 423 | + | switch disposition { | |
| 424 | + | case purecef.WindowOpenDispositionWodCurrentTab: | |
| 425 | + | return port.WindowDispositionCurrentTab | |
| 426 | + | case purecef.WindowOpenDispositionWodNewPopup: | |
| 427 | + | return port.WindowDispositionNewPopup | |
| 428 | + | case purecef.WindowOpenDispositionWodNewWindow: | |
| 429 | + | return port.WindowDispositionNewWindow | |
| 430 | + | case purecef.WindowOpenDispositionWodNewForegroundTab, | |
| 431 | + | purecef.WindowOpenDispositionWodNewBackgroundTab, | |
| 432 | + | purecef.WindowOpenDispositionWodSingletonTab, | |
| 433 | + | purecef.WindowOpenDispositionWodSwitchToTab: | |
| 434 | + | return port.WindowDispositionNewTab | |
| 435 | + | default: | |
| 436 | + | return "" | |
| 437 | + | } | |
| 438 | + | } | |
| 439 | + | ||
| 422 | 440 | // =========================================================================== | |
| 423 | 441 | // LifeSpanHandler (6 methods) | |
| 424 | 442 | // =========================================================================== | |
| @@ -429,8 +447,8 @@ func (h *handlerSet) OnLoadError(_ purecef.Browser, _ purecef.Frame, _ purecef.E | |||
| 429 | 447 | // preserved. Otherwise we keep blocking and let the coordinator's fallback | |
| 430 | 448 | // pane handle the navigation. | |
| 431 | 449 | func (h *handlerSet) OnBeforePopup( | |
| 432 | - | _ purecef.Browser, _ purecef.Frame, popupID int32, targetURL, targetFrameName string, | |
| 433 | - | _ purecef.WindowOpenDisposition, userGesture int32, _ *purecef.PopupFeatures, | |
| 450 | + | browser purecef.Browser, frame purecef.Frame, popupID int32, targetURL, targetFrameName string, | |
| 451 | + | targetDisposition purecef.WindowOpenDisposition, userGesture int32, _ *purecef.PopupFeatures, | |
| 434 | 452 | windowInfo *purecef.WindowInfo, clientSlot *purecef.RawClientWriteSlot, settings *purecef.BrowserSettings, | |
| 435 | 453 | _ *purecef.DictionaryValue, noJavaScriptAccess *bool, | |
| 436 | 454 | ) bool { | |
| @@ -450,6 +468,17 @@ func (h *handlerSet) OnBeforePopup( | |||
| 450 | 468 | requestNoJavaScriptAccess = *noJavaScriptAccess | |
| 451 | 469 | } | |
| 452 | 470 | var popup port.WebView | |
| 471 | + | sourceBrowserID := int32(0) | |
| 472 | + | if browser != nil { | |
| 473 | + | sourceBrowserID = browser.GetIdentifier() | |
| 474 | + | } | |
| 475 | + | sourceFrameID := "" | |
| 476 | + | sourceFrameURL := "" | |
| 477 | + | if frame != nil { | |
| 478 | + | sourceFrameID = frame.GetIdentifier() | |
| 479 | + | sourceFrameURL = frame.GetURL() | |
| 480 | + | } | |
| 481 | + | ||
| 453 | 482 | h.wv.runOnGTKSync(func() { | |
| 454 | 483 | popup = cb.OnCreate(port.PopupRequest{ | |
| 455 | 484 | TargetURI: targetURL, | |
| @@ -457,6 +486,10 @@ func (h *handlerSet) OnBeforePopup( | |||
| 457 | 486 | IsUserGesture: userGesture != 0, | |
| 458 | 487 | NoJavaScriptAccess: requestNoJavaScriptAccess, | |
| 459 | 488 | ParentViewID: h.wv.id, | |
| 489 | + | SourceBrowserID: sourceBrowserID, | |
| 490 | + | SourceFrameID: sourceFrameID, | |
| 491 | + | SourceFrameURL: sourceFrameURL, | |
| 492 | + | TargetDisposition: mapCEFWindowDisposition(targetDisposition), | |
| 460 | 493 | }) | |
| 461 | 494 | }) | |
| 462 | 495 | ||
| @@ -464,17 +497,52 @@ func (h *handlerSet) OnBeforePopup( | |||
| 464 | 497 | if !ok || cefPopup == nil { | |
| 465 | 498 | return true | |
| 466 | 499 | } | |
| 467 | - | cefPopup.setPopupNoJavaScriptAccess(requestNoJavaScriptAccess) | |
| 468 | - | if cefPopup.prepareNativePopup(popupID, targetURL, windowInfo, clientSlot, settings) { | |
| 469 | - | return false | |
| 500 | + | ||
| 501 | + | decision := port.HostDecision{} | |
| 502 | + | if carrier, ok := popup.(port.BrowsingContextHostDecisionCapable); ok { | |
| 503 | + | if got, hasDecision := carrier.BrowsingContextHostDecision(); hasDecision { | |
| 504 | + | decision = got | |
| 505 | + | } | |
| 470 | 506 | } | |
| 471 | 507 | ||
| 472 | - | cefPopup.discardNativePopupCandidate() | |
| 473 | - | logging.FromContext(h.currentContext()).Debug(). | |
| 474 | - | Int32("popup_id", popupID). | |
| 475 | - | Str("target_url", logging.TruncateURL(targetURL, logging.PermissionLogURLMaxLen)). | |
| 476 | - | Msg("cef: native popup bridge unavailable, blocking CEF popup and using popup shell browser creation") | |
| 477 | - | return true | |
| 508 | + | cefPopup.setPopupNoJavaScriptAccess(requestNoJavaScriptAccess) | |
| 509 | + | switch decision.Kind { | |
| 510 | + | case port.HostDecisionCreateNativeWin: | |
| 511 | + | if cefPopup.prepareNativePopup(popupID, targetURL, windowInfo, clientSlot, settings) { | |
| 512 | + | logging.FromContext(h.currentContext()).Debug(). | |
| 513 | + | Int32("popup_id", popupID). | |
| 514 | + | Str("target_url", logging.TruncateURL(targetURL, logging.PermissionLogURLMaxLen)). | |
| 515 | + | Msg("cef: native popup armed") | |
| 516 | + | return false | |
| 517 | + | } | |
| 518 | + | if aborter, ok := popup.(port.NativePopupHostAbortCapable); ok { | |
| 519 | + | aborter.AbortNativePopupHost() | |
| 520 | + | } | |
| 521 | + | cefPopup.discardNativePopupCandidate() | |
| 522 | + | logging.FromContext(h.currentContext()).Warn(). | |
| 523 | + | Int32("popup_id", popupID). | |
| 524 | + | Str("target_url", logging.TruncateURL(targetURL, logging.PermissionLogURLMaxLen)). | |
| 525 | + | Msg("cef: native popup arming failed, denying without fallback") | |
| 526 | + | return true | |
| 527 | + | case port.HostDecisionCreatePane, port.HostDecisionReuseNamedPane: | |
| 528 | + | logging.FromContext(h.currentContext()).Debug(). | |
| 529 | + | Int32("popup_id", popupID). | |
| 530 | + | Str("target_url", logging.TruncateURL(targetURL, logging.PermissionLogURLMaxLen)). | |
| 531 | + | Str("decision", string(decision.Kind)). | |
| 532 | + | Msg("cef: pane-hosted browsing context, blocking native popup and using related shell browser creation") | |
| 533 | + | return true | |
| 534 | + | default: | |
| 535 | + | if cefPopup.prepareNativePopup(popupID, targetURL, windowInfo, clientSlot, settings) { | |
| 536 | + | return false | |
| 537 | + | } | |
| 538 | + | ||
| 539 | + | cefPopup.discardNativePopupCandidate() | |
| 540 | + | logging.FromContext(h.currentContext()).Debug(). | |
| 541 | + | Int32("popup_id", popupID). | |
| 542 | + | Str("target_url", logging.TruncateURL(targetURL, logging.PermissionLogURLMaxLen)). | |
| 543 | + | Msg("cef: native popup bridge unavailable, blocking CEF popup and using popup shell browser creation") | |
| 544 | + | return true | |
| 545 | + | } | |
| 478 | 546 | } | |
| 479 | 547 | ||
| 480 | 548 | func (h *handlerSet) OnBeforePopupAborted(_ purecef.Browser, popupID int32) { | |
M internal/infrastructure/cef/webview_handlers_test.go +54 -0
| @@ -247,6 +247,60 @@ func TestOnBeforePopup_PrimesPopupShellWhenNativePopupCannotBeArmed(t *testing.T | |||
| 247 | 247 | require.Equal(t, "https://example.com/login", popupWV.pendingNavigationURI()) | |
| 248 | 248 | } | |
| 249 | 249 | ||
| 250 | + | func TestOnBeforePopup_PaneDecisionBlocksNativePopupAndKeepsPanePath(t *testing.T) { | |
| 251 | + | parentWV := &WebView{ctx: context.Background(), id: 41} | |
| 252 | + | popupWV := &WebView{ctx: context.Background(), id: 42} | |
| 253 | + | popupWV.SetBrowsingContextHostDecision(port.HostDecision{Kind: port.HostDecisionCreatePane}) | |
| 254 | + | parentWV.SetCallbacks(&port.WebViewCallbacks{ | |
| 255 | + | OnCreate: func(req port.PopupRequest) port.WebView { | |
| 256 | + | popupWV.PreparePaneHostedBrowsingContext() | |
| 257 | + | popupWV.PrimePopupNavigation(req.TargetURI) | |
| 258 | + | return popupWV | |
| 259 | + | }, | |
| 260 | + | }) | |
| 261 | + | ||
| 262 | + | h := &handlerSet{wv: parentWV} | |
| 263 | + | blocked := h.OnBeforePopup(nil, nil, 81, "https://example.com/pane", "_blank", 0, 1, nil, nil, nil, nil, nil, nil) | |
| 264 | + | ||
| 265 | + | require.True(t, blocked) | |
| 266 | + | require.Equal(t, "https://example.com/pane", popupWV.pendingNavigationURI()) | |
| 267 | + | } | |
| 268 | + | ||
| 269 | + | func TestOnBeforePopup_NativeDecisionAbortsHostWhenArmingFails(t *testing.T) { | |
| 270 | + | parentWV := &WebView{ctx: context.Background(), id: 51} | |
| 271 | + | popupWV := &WebView{ctx: context.Background(), id: 52} | |
| 272 | + | popupWV.markNativePopupCandidate(parentWV) | |
| 273 | + | popupWV.SetBrowsingContextHostDecision(port.HostDecision{Kind: port.HostDecisionCreateNativeWin}) | |
| 274 | + | aborted := false | |
| 275 | + | popupWV.SetNativePopupHostAbort(func() { aborted = true }) | |
| 276 | + | parentWV.SetCallbacks(&port.WebViewCallbacks{ | |
| 277 | + | OnCreate: func(req port.PopupRequest) port.WebView { | |
| 278 | + | popupWV.PrimePopupNavigation(req.TargetURI) | |
| 279 | + | return popupWV | |
| 280 | + | }, | |
| 281 | + | }) | |
| 282 | + | ||
| 283 | + | h := &handlerSet{wv: parentWV} | |
| 284 | + | blocked := h.OnBeforePopup(nil, nil, 82, "https://accounts.google.com/o/oauth2/v2/auth", "oauth", 0, 1, nil, nil, nil, nil, nil, nil) | |
| 285 | + | ||
| 286 | + | require.True(t, blocked) | |
| 287 | + | require.True(t, aborted) | |
| 288 | + | } | |
| 289 | + | ||
| 290 | + | func TestConfigureNativePopupWindow_UsesSharedTextureWindowlessDefaults(t *testing.T) { | |
| 291 | + | windowInfo := purecef.NewWindowInfo() | |
| 292 | + | settings := purecef.NewBrowserSettings() | |
| 293 | + | ||
| 294 | + | configureNativePopupWindow(&windowInfo, &settings, 72, 0xFF112233) | |
| 295 | + | ||
| 296 | + | require.Equal(t, purecef.WindowHandle(0), windowInfo.ParentWindow) | |
| 297 | + | require.Equal(t, int32(1), windowInfo.WindowlessRenderingEnabled) | |
| 298 | + | require.Equal(t, int32(1), windowInfo.SharedTextureEnabled) | |
| 299 | + | require.Equal(t, int32(72), settings.WindowlessFrameRate) | |
| 300 | + | require.Equal(t, int32(1), settings.LocalStorage) | |
| 301 | + | require.Equal(t, uint32(0xFF112233), settings.BackgroundColor) | |
| 302 | + | } | |
| 303 | + | ||
| 250 | 304 | func TestOnLoadStartFiresCommittedAndUpdatesURI(t *testing.T) { | |
| 251 | 305 | wv := &WebView{ctx: context.Background()} | |
| 252 | 306 | var gotEvents []port.LoadEvent | |
A internal/infrastructure/config/browsing_contexts_test.go +442 -0
| @@ -0,0 +1,442 @@ | |||
| 1 | + | package config | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "os" | |
| 5 | + | "path/filepath" | |
| 6 | + | "strings" | |
| 7 | + | "testing" | |
| 8 | + | ||
| 9 | + | "github.com/spf13/viper" | |
| 10 | + | "github.com/stretchr/testify/assert" | |
| 11 | + | "github.com/stretchr/testify/require" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | // --- Legacy popups → browsing_contexts transform tests --- | |
| 15 | + | ||
| 16 | + | func TestTransformLegacyPopupsToBrowsingContexts_BasicMapping(t *testing.T) { | |
| 17 | + | transformer := NewLegacyConfigTransformer() | |
| 18 | + | ||
| 19 | + | rawConfig := map[string]any{ | |
| 20 | + | "workspace": map[string]any{ | |
| 21 | + | "popups": map[string]any{ | |
| 22 | + | "behavior": "tabbed", | |
| 23 | + | "oauth_auto_close": false, | |
| 24 | + | }, | |
| 25 | + | }, | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | transformer.TransformLegacyPopupsToBrowsingContexts(rawConfig) | |
| 29 | + | ||
| 30 | + | ws := rawConfig["workspace"].(map[string]any) | |
| 31 | + | // Old key should be deleted. | |
| 32 | + | _, hasOld := ws["popups"] | |
| 33 | + | assert.False(t, hasOld, "popups key should be removed after transform") | |
| 34 | + | ||
| 35 | + | // New key should contain the old values. | |
| 36 | + | newCfg, hasNew := ws["browsing_contexts"].(map[string]any) | |
| 37 | + | require.True(t, hasNew, "browsing_contexts key should exist") | |
| 38 | + | assert.Equal(t, "tabbed", newCfg["behavior"]) | |
| 39 | + | assert.Equal(t, false, newCfg["oauth_auto_close"]) | |
| 40 | + | } | |
| 41 | + | ||
| 42 | + | func TestTransformLegacyPopupsToBrowsingContexts_NewKeyAlreadyPresent(t *testing.T) { | |
| 43 | + | transformer := NewLegacyConfigTransformer() | |
| 44 | + | ||
| 45 | + | rawConfig := map[string]any{ | |
| 46 | + | "workspace": map[string]any{ | |
| 47 | + | "popups": map[string]any{ | |
| 48 | + | "behavior": "tabbed", | |
| 49 | + | }, | |
| 50 | + | "browsing_contexts": map[string]any{ | |
| 51 | + | "behavior": "split", | |
| 52 | + | }, | |
| 53 | + | }, | |
| 54 | + | } | |
| 55 | + | ||
| 56 | + | transformer.TransformLegacyPopupsToBrowsingContexts(rawConfig) | |
| 57 | + | ||
| 58 | + | ws := rawConfig["workspace"].(map[string]any) | |
| 59 | + | // Old key should still exist (new key was already present, skip transform). | |
| 60 | + | oldCfg, hasOld := ws["popups"].(map[string]any) | |
| 61 | + | assert.True(t, hasOld, "popups key should remain when browsing_contexts already exists") | |
| 62 | + | assert.Equal(t, "tabbed", oldCfg["behavior"]) | |
| 63 | + | ||
| 64 | + | // New key should NOT be overwritten. | |
| 65 | + | newCfg, hasNew := ws["browsing_contexts"].(map[string]any) | |
| 66 | + | require.True(t, hasNew, "browsing_contexts key should still exist") | |
| 67 | + | assert.Equal(t, "split", newCfg["behavior"], "browsing_contexts should not be overwritten by legacy transform") | |
| 68 | + | } | |
| 69 | + | ||
| 70 | + | func TestTransformLegacyPopupsToBrowsingContexts_NoWorkspace(t *testing.T) { | |
| 71 | + | transformer := NewLegacyConfigTransformer() | |
| 72 | + | rawConfig := map[string]any{"other": "value"} | |
| 73 | + | // Should not panic | |
| 74 | + | transformer.TransformLegacyPopupsToBrowsingContexts(rawConfig) | |
| 75 | + | assert.NotNil(t, rawConfig) | |
| 76 | + | } | |
| 77 | + | ||
| 78 | + | func TestTransformLegacyPopupsToBrowsingContexts_NoPopupsKey(t *testing.T) { | |
| 79 | + | transformer := NewLegacyConfigTransformer() | |
| 80 | + | rawConfig := map[string]any{ | |
| 81 | + | "workspace": map[string]any{ | |
| 82 | + | "new_pane_url": "about:blank", | |
| 83 | + | }, | |
| 84 | + | } | |
| 85 | + | transformer.TransformLegacyPopupsToBrowsingContexts(rawConfig) | |
| 86 | + | ws := rawConfig["workspace"].(map[string]any) | |
| 87 | + | assert.Equal(t, "about:blank", ws["new_pane_url"]) | |
| 88 | + | _, hasBC := ws["browsing_contexts"] | |
| 89 | + | assert.False(t, hasBC, "browsing_contexts should not be created if popups absent") | |
| 90 | + | } | |
| 91 | + | ||
| 92 | + | // --- Default config writing emits browsing_contexts instead of popups --- | |
| 93 | + | ||
| 94 | + | func TestDefaultConfig_TOMLHasBrowsingContextsNotPopups(t *testing.T) { | |
| 95 | + | cfg := DefaultConfig() | |
| 96 | + | ||
| 97 | + | content, err := encodeConfigToTOML(cfg) | |
| 98 | + | require.NoError(t, err) | |
| 99 | + | ||
| 100 | + | // Should contain [workspace.browsing_contexts] section. | |
| 101 | + | assert.Contains(t, content, "[workspace.browsing_contexts]", | |
| 102 | + | "TOML output should contain browsing_contexts section") | |
| 103 | + | ||
| 104 | + | // Should NOT contain [workspace.popups] section. | |
| 105 | + | assert.NotContains(t, content, "[workspace.popups]", | |
| 106 | + | "TOML output should NOT contain deprecated popups section") | |
| 107 | + | ||
| 108 | + | // The section should have the expected keys. | |
| 109 | + | assert.Contains(t, content, "behavior = ") | |
| 110 | + | assert.Contains(t, content, "oauth_auto_close = ") | |
| 111 | + | } | |
| 112 | + | ||
| 113 | + | // --- Loader defaults expose browsing_contexts keys, NOT popups --- | |
| 114 | + | ||
| 115 | + | func TestSetWorkspaceDefaults_ExposesBrowsingContextsNotPopups(t *testing.T) { | |
| 116 | + | mgr := &Manager{viper: viper.New()} | |
| 117 | + | mgr.setDefaults() | |
| 118 | + | ||
| 119 | + | // browsing_contexts defaults should be registered. | |
| 120 | + | assert.Equal(t, "split", mgr.viper.GetString("workspace.browsing_contexts.behavior")) | |
| 121 | + | assert.Equal(t, "right", mgr.viper.GetString("workspace.browsing_contexts.placement")) | |
| 122 | + | assert.Equal(t, "stacked", mgr.viper.GetString("workspace.browsing_contexts.blank_target_behavior")) | |
| 123 | + | assert.True(t, mgr.viper.GetBool("workspace.browsing_contexts.open_in_new_pane")) | |
| 124 | + | assert.True(t, mgr.viper.GetBool("workspace.browsing_contexts.follow_pane_context")) | |
| 125 | + | assert.True(t, mgr.viper.GetBool("workspace.browsing_contexts.enable_smart_detection")) | |
| 126 | + | assert.True(t, mgr.viper.GetBool("workspace.browsing_contexts.oauth_auto_close")) | |
| 127 | + | ||
| 128 | + | // popups defaults should NOT be registered. | |
| 129 | + | assert.Empty(t, mgr.viper.GetString("workspace.popups.behavior")) | |
| 130 | + | assert.Empty(t, mgr.viper.GetString("workspace.popups.placement")) | |
| 131 | + | assert.Empty(t, mgr.viper.GetString("workspace.popups.blank_target_behavior")) | |
| 132 | + | assert.False(t, mgr.viper.GetBool("workspace.popups.open_in_new_pane")) | |
| 133 | + | assert.False(t, mgr.viper.GetBool("workspace.popups.follow_pane_context")) | |
| 134 | + | assert.False(t, mgr.viper.GetBool("workspace.popups.enable_smart_detection")) | |
| 135 | + | assert.False(t, mgr.viper.GetBool("workspace.popups.oauth_auto_close")) | |
| 136 | + | } | |
| 137 | + | ||
| 138 | + | func TestBindBrowsingContextEnvAliases_AcceptsLegacyPopupEnvNames(t *testing.T) { | |
| 139 | + | t.Setenv("DUMBER_WORKSPACE_POPUPS_BEHAVIOR", "tabbed") | |
| 140 | + | ||
| 141 | + | v := viper.New() | |
| 142 | + | v.SetEnvPrefix("DUMBER") | |
| 143 | + | v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) | |
| 144 | + | v.AutomaticEnv() | |
| 145 | + | v.SetDefault("workspace.browsing_contexts.behavior", "split") | |
| 146 | + | require.NoError(t, bindBrowsingContextEnvAliases(v)) | |
| 147 | + | ||
| 148 | + | assert.Equal(t, "tabbed", v.GetString("workspace.browsing_contexts.behavior")) | |
| 149 | + | } | |
| 150 | + | ||
| 151 | + | // --- Validation error paths mention workspace.browsing_contexts, not workspace.popups --- | |
| 152 | + | ||
| 153 | + | func TestValidatePopups_ErrorMessagesUseBrowsingContexts(t *testing.T) { | |
| 154 | + | tests := []struct { | |
| 155 | + | name string | |
| 156 | + | mutate func(*Config) | |
| 157 | + | wantText string | |
| 158 | + | }{ | |
| 159 | + | { | |
| 160 | + | name: "invalid behavior", | |
| 161 | + | mutate: func(cfg *Config) { | |
| 162 | + | cfg.Workspace.BrowsingContexts.Behavior = PopupBehavior("bad") | |
| 163 | + | }, | |
| 164 | + | wantText: "workspace.browsing_contexts.behavior", | |
| 165 | + | }, | |
| 166 | + | { | |
| 167 | + | name: "invalid placement", | |
| 168 | + | mutate: func(cfg *Config) { | |
| 169 | + | cfg.Workspace.BrowsingContexts.Behavior = PopupBehaviorSplit | |
| 170 | + | cfg.Workspace.BrowsingContexts.Placement = "center" | |
| 171 | + | }, | |
| 172 | + | wantText: "workspace.browsing_contexts.placement", | |
| 173 | + | }, | |
| 174 | + | { | |
| 175 | + | name: "invalid blank_target_behavior", | |
| 176 | + | mutate: func(cfg *Config) { | |
| 177 | + | cfg.Workspace.BrowsingContexts.BlankTargetBehavior = "popup" | |
| 178 | + | }, | |
| 179 | + | wantText: "workspace.browsing_contexts.blank_target_behavior", | |
| 180 | + | }, | |
| 181 | + | } | |
| 182 | + | ||
| 183 | + | for _, tt := range tests { | |
| 184 | + | t.Run(tt.name, func(t *testing.T) { | |
| 185 | + | cfg := DefaultConfig() | |
| 186 | + | // Sync Popups alias so both fields match (validation reads BrowsingContexts). | |
| 187 | + | cfg.Workspace.Popups = cfg.Workspace.BrowsingContexts | |
| 188 | + | tt.mutate(cfg) | |
| 189 | + | cfg.Workspace.Popups = cfg.Workspace.BrowsingContexts | |
| 190 | + | ||
| 191 | + | err := validateConfig(cfg) | |
| 192 | + | require.Error(t, err) | |
| 193 | + | assert.Contains(t, err.Error(), tt.wantText, | |
| 194 | + | "validation error should reference browsing_contexts, not popups") | |
| 195 | + | ||
| 196 | + | // Ensure it does NOT reference the old popups nomenclature. | |
| 197 | + | assert.NotContains(t, err.Error(), "workspace.popups.", | |
| 198 | + | "validation error should NOT reference deprecated workspace.popups") | |
| 199 | + | }) | |
| 200 | + | } | |
| 201 | + | } | |
| 202 | + | ||
| 203 | + | func TestValidatePopups_CompatibilityAliasMirrorsBrowsingContexts(t *testing.T) { | |
| 204 | + | // When BrowsingContexts is valid but Popups alias is stale, | |
| 205 | + | // validation should still pass because it reads BrowsingContexts. | |
| 206 | + | cfg := DefaultConfig() | |
| 207 | + | // Deliberately make Popups stale. | |
| 208 | + | cfg.Workspace.Popups.Behavior = PopupBehavior("invalid") | |
| 209 | + | cfg.Workspace.Popups.OAuthAutoClose = !cfg.Workspace.BrowsingContexts.OAuthAutoClose | |
| 210 | + | ||
| 211 | + | err := validateConfig(cfg) | |
| 212 | + | require.NoError(t, err, "validation should read BrowsingContexts, not the stale Popups alias") | |
| 213 | + | } | |
| 214 | + | ||
| 215 | + | // --- Runtime alias sync (BrowsingContexts → Popups) --- | |
| 216 | + | ||
| 217 | + | func TestNormalizeBrowsingContexts_SyncsPopupsAlias(t *testing.T) { | |
| 218 | + | cfg := DefaultConfig() | |
| 219 | + | ||
| 220 | + | // Make BrowsingContexts and Popups diverge. | |
| 221 | + | cfg.Workspace.BrowsingContexts.Behavior = PopupBehaviorTabbed | |
| 222 | + | cfg.Workspace.Popups.Behavior = PopupBehaviorSplit | |
| 223 | + | ||
| 224 | + | normalizeBrowsingContexts(cfg) | |
| 225 | + | ||
| 226 | + | assert.Equal(t, PopupBehaviorTabbed, cfg.Workspace.Popups.Behavior, | |
| 227 | + | "normalizeConfig should sync Popups ← BrowsingContexts") | |
| 228 | + | assert.Equal(t, cfg.Workspace.BrowsingContexts.Placement, cfg.Workspace.Popups.Placement) | |
| 229 | + | assert.Equal(t, cfg.Workspace.BrowsingContexts.OAuthAutoClose, cfg.Workspace.Popups.OAuthAutoClose) | |
| 230 | + | assert.Equal(t, cfg.Workspace.BrowsingContexts.OpenInNewPane, cfg.Workspace.Popups.OpenInNewPane) | |
| 231 | + | } | |
| 232 | + | ||
| 233 | + | func TestDefaultConfig_PopupsMirrorsBrowsingContexts(t *testing.T) { | |
| 234 | + | cfg := DefaultConfig() | |
| 235 | + | ||
| 236 | + | // After DefaultConfig(), both fields should be identical. | |
| 237 | + | assert.Equal(t, cfg.Workspace.BrowsingContexts.Behavior, cfg.Workspace.Popups.Behavior) | |
| 238 | + | assert.Equal(t, cfg.Workspace.BrowsingContexts.Placement, cfg.Workspace.Popups.Placement) | |
| 239 | + | assert.Equal(t, cfg.Workspace.BrowsingContexts.BlankTargetBehavior, cfg.Workspace.Popups.BlankTargetBehavior) | |
| 240 | + | assert.Equal(t, cfg.Workspace.BrowsingContexts.OpenInNewPane, cfg.Workspace.Popups.OpenInNewPane) | |
| 241 | + | assert.Equal(t, cfg.Workspace.BrowsingContexts.FollowPaneContext, cfg.Workspace.Popups.FollowPaneContext) | |
| 242 | + | assert.Equal(t, cfg.Workspace.BrowsingContexts.EnableSmartDetection, cfg.Workspace.Popups.EnableSmartDetection) | |
| 243 | + | assert.Equal(t, cfg.Workspace.BrowsingContexts.OAuthAutoClose, cfg.Workspace.Popups.OAuthAutoClose) | |
| 244 | + | } | |
| 245 | + | ||
| 246 | + | func TestSchemaProvider_UsesBrowsingContextsKeys(t *testing.T) { | |
| 247 | + | provider := NewSchemaProvider() | |
| 248 | + | schema := provider.GetSchema() | |
| 249 | + | ||
| 250 | + | foundBrowsingContexts := false | |
| 251 | + | foundPopups := false | |
| 252 | + | ||
| 253 | + | for _, key := range schema { | |
| 254 | + | if key.Key == "workspace.browsing_contexts.behavior" { | |
| 255 | + | foundBrowsingContexts = true | |
| 256 | + | } | |
| 257 | + | if key.Key == "workspace.popups.behavior" { | |
| 258 | + | foundPopups = true | |
| 259 | + | } | |
| 260 | + | } | |
| 261 | + | ||
| 262 | + | assert.True(t, foundBrowsingContexts, "schema should include workspace.browsing_contexts.* keys") | |
| 263 | + | assert.False(t, foundPopups, "schema should NOT include workspace.popups.* keys") | |
| 264 | + | } | |
| 265 | + | ||
| 266 | + | // --- End-to-end: legacy config round-trips through viper --- | |
| 267 | + | ||
| 268 | + | func TestLegacyPopupsFullRoundTrip(t *testing.T) { | |
| 269 | + | // Simulate reading an old config file with [workspace.popups]. | |
| 270 | + | m := &Manager{viper: viper.New()} | |
| 271 | + | m.viper.Set("workspace.popups.behavior", "stacked") | |
| 272 | + | m.viper.Set("workspace.popups.placement", "left") | |
| 273 | + | m.viper.Set("workspace.popups.oauth_auto_close", false) | |
| 274 | + | ||
| 275 | + | // Transform should map popups → browsing_contexts. | |
| 276 | + | m.transformLegacyConfig() | |
| 277 | + | ||
| 278 | + | // popups key should be gone from viper after transform. | |
| 279 | + | assert.False(t, m.viper.IsSet("workspace.popups.behavior"), | |
| 280 | + | "workspace.popups.behavior should not exist after transform") | |
| 281 | + | ||
| 282 | + | // browsing_contexts should now hold the values. | |
| 283 | + | assert.Equal(t, "stacked", m.viper.GetString("workspace.browsing_contexts.behavior")) | |
| 284 | + | assert.Equal(t, "left", m.viper.GetString("workspace.browsing_contexts.placement")) | |
| 285 | + | assert.False(t, m.viper.GetBool("workspace.browsing_contexts.oauth_auto_close")) | |
| 286 | + | ||
| 287 | + | // Unmarshal should succeed. | |
| 288 | + | cfg := &Config{} | |
| 289 | + | err := m.viper.Unmarshal(cfg) | |
| 290 | + | require.NoError(t, err) | |
| 291 | + | ||
| 292 | + | // BrowsingContexts should have the legacy values. | |
| 293 | + | assert.Equal(t, PopupBehaviorStacked, cfg.Workspace.BrowsingContexts.Behavior) | |
| 294 | + | assert.Equal(t, "left", cfg.Workspace.BrowsingContexts.Placement) | |
| 295 | + | assert.False(t, cfg.Workspace.BrowsingContexts.OAuthAutoClose) | |
| 296 | + | ||
| 297 | + | // Popups should be empty (not unmarshaled, will be synced in normalizeConfig). | |
| 298 | + | assert.Equal(t, PopupBehavior(""), cfg.Workspace.Popups.Behavior) | |
| 299 | + | ||
| 300 | + | // After normalization, Popups should mirror BrowsingContexts. | |
| 301 | + | normalizeBrowsingContexts(cfg) | |
| 302 | + | assert.Equal(t, PopupBehaviorStacked, cfg.Workspace.Popups.Behavior) | |
| 303 | + | assert.Equal(t, "left", cfg.Workspace.Popups.Placement) | |
| 304 | + | assert.False(t, cfg.Workspace.Popups.OAuthAutoClose) | |
| 305 | + | } | |
| 306 | + | ||
| 307 | + | func TestLegacyPopupsFileOverridesBrowsingContextDefaults(t *testing.T) { | |
| 308 | + | configFile := filepath.Join(t.TempDir(), "config.toml") | |
| 309 | + | content := ` | |
| 310 | + | [workspace.popups] | |
| 311 | + | behavior = "stacked" | |
| 312 | + | placement = "left" | |
| 313 | + | oauth_auto_close = false | |
| 314 | + | ` | |
| 315 | + | require.NoError(t, os.WriteFile(configFile, []byte(content), 0o644)) | |
| 316 | + | ||
| 317 | + | m := &Manager{viper: viper.New()} | |
| 318 | + | m.viper.SetConfigFile(configFile) | |
| 319 | + | m.setDefaults() | |
| 320 | + | require.NoError(t, m.viper.ReadInConfig()) | |
| 321 | + | ||
| 322 | + | m.transformLegacyConfig() | |
| 323 | + | ||
| 324 | + | assert.Equal(t, "stacked", m.viper.GetString("workspace.browsing_contexts.behavior")) | |
| 325 | + | assert.Equal(t, "left", m.viper.GetString("workspace.browsing_contexts.placement")) | |
| 326 | + | assert.False(t, m.viper.GetBool("workspace.browsing_contexts.oauth_auto_close")) | |
| 327 | + | ||
| 328 | + | cfg := &Config{} | |
| 329 | + | require.NoError(t, m.viper.Unmarshal(cfg)) | |
| 330 | + | normalizeConfig(cfg) | |
| 331 | + | ||
| 332 | + | assert.Equal(t, PopupBehaviorStacked, cfg.Workspace.BrowsingContexts.Behavior) | |
| 333 | + | assert.Equal(t, "left", cfg.Workspace.BrowsingContexts.Placement) | |
| 334 | + | assert.False(t, cfg.Workspace.BrowsingContexts.OAuthAutoClose) | |
| 335 | + | assert.Equal(t, cfg.Workspace.BrowsingContexts, cfg.Workspace.Popups) | |
| 336 | + | } | |
| 337 | + | ||
| 338 | + | func TestLegacyPopupsFilePreservesCanonicalBrowsingContextsEnvOverride(t *testing.T) { | |
| 339 | + | configFile := filepath.Join(t.TempDir(), "config.toml") | |
| 340 | + | content := ` | |
| 341 | + | [workspace.popups] | |
| 342 | + | behavior = "stacked" | |
| 343 | + | placement = "left" | |
| 344 | + | ` | |
| 345 | + | require.NoError(t, os.WriteFile(configFile, []byte(content), 0o644)) | |
| 346 | + | t.Setenv("DUMBER_WORKSPACE_BROWSING_CONTEXTS_BEHAVIOR", "tabbed") | |
| 347 | + | ||
| 348 | + | m := &Manager{viper: viper.New()} | |
| 349 | + | m.viper.SetConfigType("toml") | |
| 350 | + | m.viper.SetConfigFile(configFile) | |
| 351 | + | m.viper.SetEnvPrefix("DUMBER") | |
| 352 | + | m.viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) | |
| 353 | + | m.viper.AutomaticEnv() | |
| 354 | + | m.setDefaults() | |
| 355 | + | require.NoError(t, m.viper.ReadInConfig()) | |
| 356 | + | ||
| 357 | + | m.transformLegacyConfig() | |
| 358 | + | ||
| 359 | + | assert.Equal(t, "tabbed", m.viper.GetString("workspace.browsing_contexts.behavior")) | |
| 360 | + | assert.Equal(t, "left", m.viper.GetString("workspace.browsing_contexts.placement")) | |
| 361 | + | } | |
| 362 | + | ||
| 363 | + | func TestReload_ReappliesLegacyPopupsCompatibility(t *testing.T) { | |
| 364 | + | configFile := filepath.Join(t.TempDir(), "config.toml") | |
| 365 | + | writeConfig := func(content string) { | |
| 366 | + | require.NoError(t, os.WriteFile(configFile, []byte(content), 0o644)) | |
| 367 | + | } | |
| 368 | + | ||
| 369 | + | m := &Manager{viper: viper.New()} | |
| 370 | + | m.viper.SetConfigType("toml") | |
| 371 | + | m.viper.SetConfigFile(configFile) | |
| 372 | + | m.viper.SetEnvPrefix("DUMBER") | |
| 373 | + | m.viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) | |
| 374 | + | m.viper.AutomaticEnv() | |
| 375 | + | m.setDefaults() | |
| 376 | + | ||
| 377 | + | writeConfig(` | |
| 378 | + | [workspace.popups] | |
| 379 | + | behavior = "stacked" | |
| 380 | + | placement = "left" | |
| 381 | + | `) | |
| 382 | + | require.NoError(t, m.reload()) | |
| 383 | + | require.Equal(t, PopupBehaviorStacked, m.config.Workspace.BrowsingContexts.Behavior) | |
| 384 | + | require.Equal(t, "left", m.config.Workspace.BrowsingContexts.Placement) | |
| 385 | + | ||
| 386 | + | writeConfig(` | |
| 387 | + | [workspace.popups] | |
| 388 | + | behavior = "tabbed" | |
| 389 | + | placement = "bottom" | |
| 390 | + | `) | |
| 391 | + | require.NoError(t, m.reload()) | |
| 392 | + | assert.Equal(t, PopupBehaviorTabbed, m.config.Workspace.BrowsingContexts.Behavior) | |
| 393 | + | assert.Equal(t, "bottom", m.config.Workspace.BrowsingContexts.Placement) | |
| 394 | + | assert.Equal(t, m.config.Workspace.BrowsingContexts, m.config.Workspace.Popups) | |
| 395 | + | } | |
| 396 | + | ||
| 397 | + | func TestReload_MixedLegacyAndCanonicalBrowsingContexts_BackfillsMissingCanonicalFields(t *testing.T) { | |
| 398 | + | configFile := filepath.Join(t.TempDir(), "config.toml") | |
| 399 | + | require.NoError(t, os.WriteFile(configFile, []byte(` | |
| 400 | + | [workspace.popups] | |
| 401 | + | placement = "left" | |
| 402 | + | oauth_auto_close = false | |
| 403 | + | ||
| 404 | + | [workspace.browsing_contexts] | |
| 405 | + | behavior = "tabbed" | |
| 406 | + | `), 0o644)) | |
| 407 | + | ||
| 408 | + | m := &Manager{viper: viper.New()} | |
| 409 | + | m.viper.SetConfigType("toml") | |
| 410 | + | m.viper.SetConfigFile(configFile) | |
| 411 | + | m.viper.SetEnvPrefix("DUMBER") | |
| 412 | + | m.viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) | |
| 413 | + | m.viper.AutomaticEnv() | |
| 414 | + | m.setDefaults() | |
| 415 | + | ||
| 416 | + | require.NoError(t, m.reload()) | |
| 417 | + | assert.Equal(t, PopupBehaviorTabbed, m.config.Workspace.BrowsingContexts.Behavior) | |
| 418 | + | assert.Equal(t, "left", m.config.Workspace.BrowsingContexts.Placement) | |
| 419 | + | assert.False(t, m.config.Workspace.BrowsingContexts.OAuthAutoClose) | |
| 420 | + | assert.Equal(t, m.config.Workspace.BrowsingContexts, m.config.Workspace.Popups) | |
| 421 | + | } | |
| 422 | + | ||
| 423 | + | func TestSave_WhenWatching_SyncsPopupsAliasBeforeSkipReload(t *testing.T) { | |
| 424 | + | configFile := filepath.Join(t.TempDir(), "config.toml") | |
| 425 | + | m := &Manager{viper: viper.New(), watching: true} | |
| 426 | + | m.viper.SetConfigType("toml") | |
| 427 | + | m.viper.SetConfigFile(configFile) | |
| 428 | + | m.setDefaults() | |
| 429 | + | m.config = DefaultConfig() | |
| 430 | + | ||
| 431 | + | cfg := DefaultConfig() | |
| 432 | + | cfg.Workspace.BrowsingContexts.Behavior = PopupBehaviorTabbed | |
| 433 | + | cfg.Workspace.BrowsingContexts.Placement = "bottom" | |
| 434 | + | cfg.Workspace.Popups.Behavior = PopupBehaviorSplit | |
| 435 | + | cfg.Workspace.Popups.Placement = "left" | |
| 436 | + | ||
| 437 | + | require.NoError(t, m.Save(cfg)) | |
| 438 | + | assert.True(t, m.skipNextReload) | |
| 439 | + | assert.Equal(t, cfg.Workspace.BrowsingContexts.Behavior, m.config.Workspace.BrowsingContexts.Behavior) | |
| 440 | + | assert.Equal(t, cfg.Workspace.BrowsingContexts.Placement, m.config.Workspace.BrowsingContexts.Placement) | |
| 441 | + | assert.Equal(t, m.config.Workspace.BrowsingContexts, m.config.Workspace.Popups) | |
| 442 | + | } | |
M internal/infrastructure/config/defaults.go +9 -0
| @@ -254,6 +254,15 @@ func DefaultConfig() *Config { | |||
| 254 | 254 | }, | |
| 255 | 255 | TabBarPosition: defaultTabBarPosition, | |
| 256 | 256 | HideTabBarWhenSingleTab: true, | |
| 257 | + | BrowsingContexts: BrowsingContextConfig{ | |
| 258 | + | Behavior: PopupBehaviorSplit, // Default: open JavaScript popups in split panes | |
| 259 | + | Placement: defaultPopupPlacement, | |
| 260 | + | OpenInNewPane: true, | |
| 261 | + | FollowPaneContext: true, | |
| 262 | + | BlankTargetBehavior: "stacked", // Default: open _blank links in stacked mode | |
| 263 | + | EnableSmartDetection: true, // Use WindowProperties to detect popup vs tab | |
| 264 | + | OAuthAutoClose: true, // Auto-close OAuth popups on success | |
| 265 | + | }, | |
| 257 | 266 | Popups: PopupBehaviorConfig{ | |
| 258 | 267 | Behavior: PopupBehaviorSplit, // Default: open JavaScript popups in split panes | |
| 259 | 268 | Placement: defaultPopupPlacement, | |
M internal/infrastructure/config/loader.go +127 -12
| @@ -65,6 +65,9 @@ func NewManager() (*Manager, error) { | |||
| 65 | 65 | if err := v.BindEnv("logging.format", "DUMBER_LOG_FORMAT"); err != nil { | |
| 66 | 66 | return nil, fmt.Errorf("failed to bind DUMBER_LOG_FORMAT: %w", err) | |
| 67 | 67 | } | |
| 68 | + | if err := bindBrowsingContextEnvAliases(v); err != nil { | |
| 69 | + | return nil, err | |
| 70 | + | } | |
| 68 | 71 | ||
| 69 | 72 | return &Manager{ | |
| 70 | 73 | viper: v, | |
| @@ -145,17 +148,116 @@ func (m *Manager) readConfigFile() error { | |||
| 145 | 148 | // transformLegacyConfig converts old-format action bindings and engine config to the new format. | |
| 146 | 149 | // This is called after reading config but before unmarshaling. | |
| 147 | 150 | func (m *Manager) transformLegacyConfig() { | |
| 151 | + | legacyPopupsInConfig := m.viper.InConfig("workspace.popups") | |
| 152 | + | ||
| 148 | 153 | rawConfig := m.viper.AllSettings() | |
| 149 | 154 | transformer := NewLegacyConfigTransformer() | |
| 150 | 155 | transformer.TransformLegacyActions(rawConfig) | |
| 151 | 156 | transformer.TransformLegacyEngineConfig(rawConfig) | |
| 152 | 157 | ||
| 158 | + | // AllSettings includes defaults, so workspace.browsing_contexts may already be | |
| 159 | + | // present even when the user only configured legacy workspace.popups. Always | |
| 160 | + | // merge legacy popup values field-by-field into the canonical section while | |
| 161 | + | // preserving explicit canonical file/env overrides. | |
| 162 | + | if legacyPopupsInConfig { | |
| 163 | + | m.mergeLegacyPopupsIntoCanonicalBrowsingContexts(rawConfig) | |
| 164 | + | } else { | |
| 165 | + | transformer.TransformLegacyPopupsToBrowsingContexts(rawConfig) | |
| 166 | + | } | |
| 167 | + | ||
| 153 | 168 | // Apply transformed config back to viper | |
| 154 | 169 | for key, value := range rawConfig { | |
| 155 | 170 | m.viper.Set(key, value) | |
| 156 | 171 | } | |
| 157 | 172 | } | |
| 158 | 173 | ||
| 174 | + | func (m *Manager) mergeLegacyPopupsIntoCanonicalBrowsingContexts(rawConfig map[string]any) { | |
| 175 | + | workspace, ok := rawConfig["workspace"].(map[string]any) | |
| 176 | + | if !ok { | |
| 177 | + | return | |
| 178 | + | } | |
| 179 | + | ||
| 180 | + | legacyPopups, ok := workspace["popups"].(map[string]any) | |
| 181 | + | if !ok { | |
| 182 | + | return | |
| 183 | + | } | |
| 184 | + | ||
| 185 | + | canonical, ok := workspace["browsing_contexts"].(map[string]any) | |
| 186 | + | if !ok || canonical == nil { | |
| 187 | + | canonical = make(map[string]any, len(legacyPopups)) | |
| 188 | + | workspace["browsing_contexts"] = canonical | |
| 189 | + | } | |
| 190 | + | ||
| 191 | + | for key, value := range legacyPopups { | |
| 192 | + | if m.hasCanonicalBrowsingContextsOverride(key) { | |
| 193 | + | continue | |
| 194 | + | } | |
| 195 | + | canonical[key] = value | |
| 196 | + | } | |
| 197 | + | ||
| 198 | + | delete(workspace, "popups") | |
| 199 | + | } | |
| 200 | + | ||
| 201 | + | func (m *Manager) hasCanonicalBrowsingContextsOverride(field string) bool { | |
| 202 | + | if m.viper.InConfig("workspace.browsing_contexts." + field) { | |
| 203 | + | return true | |
| 204 | + | } | |
| 205 | + | return m.hasCanonicalBrowsingContextsEnvOverride(field) | |
| 206 | + | } | |
| 207 | + | ||
| 208 | + | func (*Manager) hasCanonicalBrowsingContextsEnvOverride(field string) bool { | |
| 209 | + | suffix := strings.ToUpper(strings.ReplaceAll(field, ".", "_")) | |
| 210 | + | for _, envKey := range []string{ | |
| 211 | + | "DUMBER_WORKSPACE_BROWSING_CONTEXTS_" + suffix, | |
| 212 | + | "DUMBER_WORKSPACE_POPUPS_" + suffix, | |
| 213 | + | } { | |
| 214 | + | if _, ok := os.LookupEnv(envKey); ok { | |
| 215 | + | return true | |
| 216 | + | } | |
| 217 | + | } | |
| 218 | + | return false | |
| 219 | + | } | |
| 220 | + | ||
| 221 | + | func bindBrowsingContextEnvAliases(v *viper.Viper) error { | |
| 222 | + | bindings := map[string][]string{ | |
| 223 | + | "workspace.browsing_contexts.behavior": { | |
| 224 | + | "DUMBER_WORKSPACE_BROWSING_CONTEXTS_BEHAVIOR", | |
| 225 | + | "DUMBER_WORKSPACE_POPUPS_BEHAVIOR", | |
| 226 | + | }, | |
| 227 | + | "workspace.browsing_contexts.placement": { | |
| 228 | + | "DUMBER_WORKSPACE_BROWSING_CONTEXTS_PLACEMENT", | |
| 229 | + | "DUMBER_WORKSPACE_POPUPS_PLACEMENT", | |
| 230 | + | }, | |
| 231 | + | "workspace.browsing_contexts.open_in_new_pane": { | |
| 232 | + | "DUMBER_WORKSPACE_BROWSING_CONTEXTS_OPEN_IN_NEW_PANE", | |
| 233 | + | "DUMBER_WORKSPACE_POPUPS_OPEN_IN_NEW_PANE", | |
| 234 | + | }, | |
| 235 | + | "workspace.browsing_contexts.follow_pane_context": { | |
| 236 | + | "DUMBER_WORKSPACE_BROWSING_CONTEXTS_FOLLOW_PANE_CONTEXT", | |
| 237 | + | "DUMBER_WORKSPACE_POPUPS_FOLLOW_PANE_CONTEXT", | |
| 238 | + | }, | |
| 239 | + | "workspace.browsing_contexts.blank_target_behavior": { | |
| 240 | + | "DUMBER_WORKSPACE_BROWSING_CONTEXTS_BLANK_TARGET_BEHAVIOR", | |
| 241 | + | "DUMBER_WORKSPACE_POPUPS_BLANK_TARGET_BEHAVIOR", | |
| 242 | + | }, | |
| 243 | + | "workspace.browsing_contexts.enable_smart_detection": { | |
| 244 | + | "DUMBER_WORKSPACE_BROWSING_CONTEXTS_ENABLE_SMART_DETECTION", | |
| 245 | + | "DUMBER_WORKSPACE_POPUPS_ENABLE_SMART_DETECTION", | |
| 246 | + | }, | |
| 247 | + | "workspace.browsing_contexts.oauth_auto_close": { | |
| 248 | + | "DUMBER_WORKSPACE_BROWSING_CONTEXTS_OAUTH_AUTO_CLOSE", | |
| 249 | + | "DUMBER_WORKSPACE_POPUPS_OAUTH_AUTO_CLOSE", | |
| 250 | + | }, | |
| 251 | + | } | |
| 252 | + | for key, envs := range bindings { | |
| 253 | + | args := append([]string{key}, envs...) | |
| 254 | + | if err := v.BindEnv(args...); err != nil { | |
| 255 | + | return fmt.Errorf("failed to bind browsing-context env aliases for %s: %w", key, err) | |
| 256 | + | } | |
| 257 | + | } | |
| 258 | + | return nil | |
| 259 | + | } | |
| 260 | + | ||
| 159 | 261 | // checkLegacyFormat detects old config format and returns an error directing user to migrate. | |
| 160 | 262 | func (m *Manager) checkLegacyFormat() error { | |
| 161 | 263 | // Check if old sections exist by looking for known keys. | |
| @@ -208,6 +310,13 @@ func normalizeConfig(config *Config) { | |||
| 208 | 310 | normalizeAppearance(config) | |
| 209 | 311 | normalizeMedia(config) | |
| 210 | 312 | normalizeEngineConfig(config) | |
| 313 | + | normalizeBrowsingContexts(config) | |
| 314 | + | } | |
| 315 | + | ||
| 316 | + | // normalizeBrowsingContexts ensures the deprecated Popups field mirrors BrowsingContexts | |
| 317 | + | // for runtime compatibility. Existing code can keep reading config.Workspace.Popups. | |
| 318 | + | func normalizeBrowsingContexts(config *Config) { | |
| 319 | + | config.Workspace.Popups = config.Workspace.BrowsingContexts | |
| 211 | 320 | } | |
| 212 | 321 | ||
| 213 | 322 | func normalizeEngineConfig(config *Config) { | |
| @@ -313,8 +422,13 @@ func (m *Manager) Save(cfg *Config) error { | |||
| 313 | 422 | return fmt.Errorf("config is nil") | |
| 314 | 423 | } | |
| 315 | 424 | ||
| 425 | + | // Normalize a copy before writing/publishing so deprecated runtime aliases | |
| 426 | + | // stay in sync even when the watcher skips reload after Save(). | |
| 427 | + | cfgToSave := *cfg | |
| 428 | + | normalizeConfig(&cfgToSave) | |
| 429 | + | ||
| 316 | 430 | // Validate before writing so UI gets immediate errors. | |
| 317 | - | if err := validateConfig(cfg); err != nil { | |
| 431 | + | if err := validateConfig(&cfgToSave); err != nil { | |
| 318 | 432 | return fmt.Errorf("configuration validation failed: %w", err) | |
| 319 | 433 | } | |
| 320 | 434 | ||
| @@ -332,14 +446,15 @@ func (m *Manager) Save(cfg *Config) error { | |||
| 332 | 446 | // overwriting our in-memory config with stale viper cache | |
| 333 | 447 | m.skipNextReload = true | |
| 334 | 448 | ||
| 335 | - | if err := WriteConfigOrdered(cfg, configFile); err != nil { | |
| 449 | + | if err := WriteConfigOrdered(&cfgToSave, configFile); err != nil { | |
| 336 | 450 | m.skipNextReload = false // Reset on error | |
| 337 | 451 | return fmt.Errorf("failed to write config: %w", err) | |
| 338 | 452 | } | |
| 339 | 453 | ||
| 340 | - | // Update in-memory config immediately (don't wait for file watcher) | |
| 341 | - | // This ensures subsequent Get() calls return the new values | |
| 342 | - | m.config = cfg | |
| 454 | + | // Update in-memory config immediately (don't wait for file watcher). | |
| 455 | + | // This ensures subsequent Get() calls return normalized values even when | |
| 456 | + | // the watcher intentionally skips reload for our own Save(). | |
| 457 | + | m.config = &cfgToSave | |
| 343 | 458 | ||
| 344 | 459 | // Also reload from disk to sync Viper's internal state (if not watching) | |
| 345 | 460 | if !m.watching { | |
| @@ -500,13 +615,13 @@ func (m *Manager) setWorkspaceDefaults(defaults *Config) { | |||
| 500 | 615 | m.viper.SetDefault("workspace.tab_bar_position", defaults.Workspace.TabBarPosition) | |
| 501 | 616 | m.viper.SetDefault("workspace.hide_tab_bar_when_single_tab", defaults.Workspace.HideTabBarWhenSingleTab) | |
| 502 | 617 | m.viper.SetDefault("workspace.switch_to_tab_on_move", defaults.Workspace.SwitchToTabOnMove) | |
| 503 | - | m.viper.SetDefault("workspace.popups.behavior", string(defaults.Workspace.Popups.Behavior)) | |
| 504 | - | m.viper.SetDefault("workspace.popups.placement", defaults.Workspace.Popups.Placement) | |
| 505 | - | m.viper.SetDefault("workspace.popups.open_in_new_pane", defaults.Workspace.Popups.OpenInNewPane) | |
| 506 | - | m.viper.SetDefault("workspace.popups.follow_pane_context", defaults.Workspace.Popups.FollowPaneContext) | |
| 507 | - | m.viper.SetDefault("workspace.popups.blank_target_behavior", defaults.Workspace.Popups.BlankTargetBehavior) | |
| 508 | - | m.viper.SetDefault("workspace.popups.enable_smart_detection", defaults.Workspace.Popups.EnableSmartDetection) | |
| 509 | - | m.viper.SetDefault("workspace.popups.oauth_auto_close", defaults.Workspace.Popups.OAuthAutoClose) | |
| 618 | + | m.viper.SetDefault("workspace.browsing_contexts.behavior", string(defaults.Workspace.BrowsingContexts.Behavior)) | |
| 619 | + | m.viper.SetDefault("workspace.browsing_contexts.placement", defaults.Workspace.BrowsingContexts.Placement) | |
| 620 | + | m.viper.SetDefault("workspace.browsing_contexts.open_in_new_pane", defaults.Workspace.BrowsingContexts.OpenInNewPane) | |
| 621 | + | m.viper.SetDefault("workspace.browsing_contexts.follow_pane_context", defaults.Workspace.BrowsingContexts.FollowPaneContext) | |
| 622 | + | m.viper.SetDefault("workspace.browsing_contexts.blank_target_behavior", defaults.Workspace.BrowsingContexts.BlankTargetBehavior) | |
| 623 | + | m.viper.SetDefault("workspace.browsing_contexts.enable_smart_detection", defaults.Workspace.BrowsingContexts.EnableSmartDetection) | |
| 624 | + | m.viper.SetDefault("workspace.browsing_contexts.oauth_auto_close", defaults.Workspace.BrowsingContexts.OAuthAutoClose) | |
| 510 | 625 | m.viper.SetDefault("workspace.styling.border_width", defaults.Workspace.Styling.BorderWidth) | |
| 511 | 626 | m.viper.SetDefault("workspace.styling.border_color", defaults.Workspace.Styling.BorderColor) | |
| 512 | 627 | m.viper.SetDefault("workspace.styling.mode_border_width", defaults.Workspace.Styling.ModeBorderWidth) | |
M internal/infrastructure/config/migrate.go +88 -13
| @@ -87,12 +87,19 @@ func (m *Migrator) DetectChanges() ([]port.KeyChange, error) { | |||
| 87 | 87 | return nil, nil | |
| 88 | 88 | } | |
| 89 | 89 | ||
| 90 | - | // Get user-defined keys with values | |
| 90 | + | // Get user-defined keys with values after compatibility transforms. | |
| 91 | 91 | userKeysWithValues, err := m.getUserConfigKeysWithValues(configFile) | |
| 92 | 92 | if err != nil { | |
| 93 | 93 | return nil, fmt.Errorf("failed to parse user config: %w", err) | |
| 94 | 94 | } | |
| 95 | 95 | ||
| 96 | + | // Get raw user-defined keys before popup compatibility transforms so legacy | |
| 97 | + | // workspace.popups -> workspace.browsing_contexts renames are still reported. | |
| 98 | + | rawUserKeysWithValues, err := m.getRawUserConfigKeysWithValues(configFile) | |
| 99 | + | if err != nil { | |
| 100 | + | return nil, fmt.Errorf("failed to parse raw user config: %w", err) | |
| 101 | + | } | |
| 102 | + | ||
| 96 | 103 | // Get all default keys | |
| 97 | 104 | defaultKeys := m.getAllDefaultKeys() | |
| 98 | 105 | defaultKeySet := make(map[string]bool) | |
| @@ -106,13 +113,22 @@ func (m *Migrator) DetectChanges() ([]port.KeyChange, error) { | |||
| 106 | 113 | userKeySet[k] = true | |
| 107 | 114 | } | |
| 108 | 115 | ||
| 116 | + | legacyBrowsingContextRenames := m.detectLegacyBrowsingContextRenames(rawUserKeysWithValues) | |
| 109 | 117 | var changes []port.KeyChange | |
| 118 | + | changes = append(changes, legacyBrowsingContextRenames...) | |
| 119 | + | ||
| 120 | + | handledLegacyDeprecatedKeys := make(map[string]bool, len(legacyBrowsingContextRenames)) | |
| 121 | + | handledLegacyMissingKeys := make(map[string]bool, len(legacyBrowsingContextRenames)) | |
| 122 | + | for _, change := range legacyBrowsingContextRenames { | |
| 123 | + | handledLegacyDeprecatedKeys[change.OldKey] = true | |
| 124 | + | handledLegacyMissingKeys[change.NewKey] = true | |
| 125 | + | } | |
| 110 | 126 | ||
| 111 | 127 | // Find deprecated keys (in user config but not in defaults) | |
| 112 | - | deprecatedKeys := m.findDeprecatedKeys(userKeysWithValues, defaultKeySet) | |
| 128 | + | deprecatedKeys := excludeKeys(m.findDeprecatedKeys(userKeysWithValues, defaultKeySet), handledLegacyDeprecatedKeys) | |
| 113 | 129 | ||
| 114 | 130 | // Find missing keys (in defaults but not in user config) | |
| 115 | - | missingKeys := m.findMissingKeys(defaultKeys, userKeySet) | |
| 131 | + | missingKeys := excludeKeys(m.findMissingKeys(defaultKeys, userKeySet), handledLegacyMissingKeys) | |
| 116 | 132 | missingKeys = appendUniqueKeys(missingKeys, m.detectMissingWorkspaceShortcutActions(userKeysWithValues)) | |
| 117 | 133 | ||
| 118 | 134 | // Try to match deprecated keys with missing keys (detect renames) | |
| @@ -378,9 +394,11 @@ func (m *Migrator) Migrate() ([]string, error) { | |||
| 378 | 394 | } | |
| 379 | 395 | ||
| 380 | 396 | // Transform legacy action bindings (slice -> ActionBinding struct) | |
| 397 | + | // and migrate popups -> browsing_contexts | |
| 381 | 398 | transformer := NewLegacyConfigTransformer() | |
| 382 | 399 | transformer.TransformLegacyActions(rawConfig) | |
| 383 | 400 | transformer.TransformLegacyEngineConfig(rawConfig) | |
| 401 | + | transformer.TransformLegacyPopupsToBrowsingContexts(rawConfig) | |
| 384 | 402 | ||
| 385 | 403 | // Build sets of keys to remove and renames to apply | |
| 386 | 404 | keysToRemove := make(map[string]bool) | |
| @@ -657,29 +675,71 @@ func (m *Migrator) getUserConfigKeys(configFile string) (map[string]bool, error) | |||
| 657 | 675 | ||
| 658 | 676 | // getUserConfigKeysWithValues parses the user's TOML file and returns keys with their values. | |
| 659 | 677 | func (m *Migrator) getUserConfigKeysWithValues(configFile string) (map[string]any, error) { | |
| 660 | - | data, err := os.ReadFile(configFile) | |
| 678 | + | rawConfig, err := m.readRawConfig(configFile) | |
| 661 | 679 | if err != nil { | |
| 662 | - | return nil, fmt.Errorf("failed to read config file: %w", err) | |
| 663 | - | } | |
| 664 | - | ||
| 665 | - | // Parse TOML into a generic map | |
| 666 | - | var rawConfig map[string]any | |
| 667 | - | if err := toml.Unmarshal(data, &rawConfig); err != nil { | |
| 668 | - | return nil, fmt.Errorf("failed to parse TOML: %w", err) | |
| 680 | + | return nil, err | |
| 669 | 681 | } | |
| 670 | 682 | ||
| 671 | - | // Transform legacy action bindings before flattening | |
| 683 | + | // Transform legacy action bindings and popups before flattening. | |
| 672 | 684 | transformer := NewLegacyConfigTransformer() | |
| 673 | 685 | transformer.TransformLegacyActions(rawConfig) | |
| 674 | 686 | transformer.TransformLegacyEngineConfig(rawConfig) | |
| 687 | + | transformer.TransformLegacyPopupsToBrowsingContexts(rawConfig) | |
| 675 | 688 | ||
| 676 | - | // Flatten the map to dot-notation keys with values | |
| 689 | + | // Flatten the map to dot-notation keys with values. | |
| 677 | 690 | result := make(map[string]any) | |
| 678 | 691 | m.flattenMapWithValues(rawConfig, "", result) | |
| 679 | 692 | ||
| 680 | 693 | return result, nil | |
| 681 | 694 | } | |
| 682 | 695 | ||
| 696 | + | func (m *Migrator) getRawUserConfigKeysWithValues(configFile string) (map[string]any, error) { | |
| 697 | + | rawConfig, err := m.readRawConfig(configFile) | |
| 698 | + | if err != nil { | |
| 699 | + | return nil, err | |
| 700 | + | } | |
| 701 | + | ||
| 702 | + | result := make(map[string]any) | |
| 703 | + | m.flattenMapWithValues(rawConfig, "", result) | |
| 704 | + | return result, nil | |
| 705 | + | } | |
| 706 | + | ||
| 707 | + | func (m *Migrator) detectLegacyBrowsingContextRenames(rawUserKeys map[string]any) []port.KeyChange { | |
| 708 | + | type renamePair struct { | |
| 709 | + | oldKey string | |
| 710 | + | newKey string | |
| 711 | + | } | |
| 712 | + | ||
| 713 | + | renamePairs := []renamePair{ | |
| 714 | + | {"workspace.popups.behavior", "workspace.browsing_contexts.behavior"}, | |
| 715 | + | {"workspace.popups.placement", "workspace.browsing_contexts.placement"}, | |
| 716 | + | {"workspace.popups.open_in_new_pane", "workspace.browsing_contexts.open_in_new_pane"}, | |
| 717 | + | {"workspace.popups.follow_pane_context", "workspace.browsing_contexts.follow_pane_context"}, | |
| 718 | + | {"workspace.popups.blank_target_behavior", "workspace.browsing_contexts.blank_target_behavior"}, | |
| 719 | + | {"workspace.popups.enable_smart_detection", "workspace.browsing_contexts.enable_smart_detection"}, | |
| 720 | + | {"workspace.popups.oauth_auto_close", "workspace.browsing_contexts.oauth_auto_close"}, | |
| 721 | + | } | |
| 722 | + | ||
| 723 | + | changes := make([]port.KeyChange, 0, len(renamePairs)) | |
| 724 | + | for _, pair := range renamePairs { | |
| 725 | + | oldValue, hasOld := rawUserKeys[pair.oldKey] | |
| 726 | + | if !hasOld { | |
| 727 | + | continue | |
| 728 | + | } | |
| 729 | + | if _, hasNew := rawUserKeys[pair.newKey]; hasNew { | |
| 730 | + | continue | |
| 731 | + | } | |
| 732 | + | changes = append(changes, port.KeyChange{ | |
| 733 | + | Type: port.KeyChangeRenamed, | |
| 734 | + | OldKey: pair.oldKey, | |
| 735 | + | NewKey: pair.newKey, | |
| 736 | + | OldValue: m.formatValue(oldValue), | |
| 737 | + | NewValue: m.formatValue(m.defaultValueForKey(pair.newKey)), | |
| 738 | + | }) | |
| 739 | + | } | |
| 740 | + | return changes | |
| 741 | + | } | |
| 742 | + | ||
| 683 | 743 | // flattenMapWithValues recursively flattens a nested map to dot-notation keys with values. | |
| 684 | 744 | func (m *Migrator) flattenMapWithValues(data map[string]any, prefix string, result map[string]any) { | |
| 685 | 745 | for k, v := range data { | |
| @@ -837,6 +897,21 @@ func appendUniqueKeys(base, extra []string) []string { | |||
| 837 | 897 | return base | |
| 838 | 898 | } | |
| 839 | 899 | ||
| 900 | + | func excludeKeys(keys []string, excluded map[string]bool) []string { | |
| 901 | + | if len(excluded) == 0 { | |
| 902 | + | return keys | |
| 903 | + | } | |
| 904 | + | ||
| 905 | + | filtered := make([]string, 0, len(keys)) | |
| 906 | + | for _, key := range keys { | |
| 907 | + | if excluded[key] { | |
| 908 | + | continue | |
| 909 | + | } | |
| 910 | + | filtered = append(filtered, key) | |
| 911 | + | } | |
| 912 | + | return filtered | |
| 913 | + | } | |
| 914 | + | ||
| 840 | 915 | func (*Migrator) toStringAnyMap(value any) (map[string]any, bool) { | |
| 841 | 916 | if value == nil { | |
| 842 | 917 | return nil, false | |
M internal/infrastructure/config/migrate_test.go +197 -0
| @@ -3,8 +3,10 @@ package config | |||
| 3 | 3 | import ( | |
| 4 | 4 | "os" | |
| 5 | 5 | "path/filepath" | |
| 6 | + | "strings" | |
| 6 | 7 | "testing" | |
| 7 | 8 | ||
| 9 | + | port "github.com/bnema/dumber/internal/application/port" | |
| 8 | 10 | "github.com/stretchr/testify/assert" | |
| 9 | 11 | "github.com/stretchr/testify/require" | |
| 10 | 12 | ) | |
| @@ -916,6 +918,201 @@ func TestMigrator_DeleteNestedKey(t *testing.T) { | |||
| 916 | 918 | }) | |
| 917 | 919 | } | |
| 918 | 920 | ||
| 921 | + | // --- Migrate-path popup → browsing_contexts transform tests --- | |
| 922 | + | ||
| 923 | + | func TestMigrator_GetUserConfigKeysWithValues_TransformsLegacyPopups(t *testing.T) { | |
| 924 | + | m := NewMigrator() | |
| 925 | + | ||
| 926 | + | tmpDir := t.TempDir() | |
| 927 | + | configFile := filepath.Join(tmpDir, "config.toml") | |
| 928 | + | ||
| 929 | + | content := ` | |
| 930 | + | [workspace] | |
| 931 | + | new_pane_url = "about:blank" | |
| 932 | + | ||
| 933 | + | [workspace.popups] | |
| 934 | + | behavior = "tabbed" | |
| 935 | + | placement = "left" | |
| 936 | + | oauth_auto_close = false | |
| 937 | + | ` | |
| 938 | + | err := os.WriteFile(configFile, []byte(content), 0o644) | |
| 939 | + | require.NoError(t, err) | |
| 940 | + | ||
| 941 | + | keysWithValues, err := m.getUserConfigKeysWithValues(configFile) | |
| 942 | + | require.NoError(t, err) | |
| 943 | + | ||
| 944 | + | // Legacy popups keys should NOT appear after transform. | |
| 945 | + | _, hasPopupsBehavior := keysWithValues["workspace.popups.behavior"] | |
| 946 | + | assert.False(t, hasPopupsBehavior, "workspace.popups.behavior should not exist after transform") | |
| 947 | + | ||
| 948 | + | // Browsing_contexts flattened keys should carry the legacy values. | |
| 949 | + | assert.Equal(t, "tabbed", keysWithValues["workspace.browsing_contexts.behavior"]) | |
| 950 | + | assert.Equal(t, "left", keysWithValues["workspace.browsing_contexts.placement"]) | |
| 951 | + | assert.Equal(t, false, keysWithValues["workspace.browsing_contexts.oauth_auto_close"]) | |
| 952 | + | } | |
| 953 | + | ||
| 954 | + | func TestMigrator_GetUserConfigKeysWithValues_BrowsingContextsAlreadyPresent(t *testing.T) { | |
| 955 | + | m := NewMigrator() | |
| 956 | + | ||
| 957 | + | tmpDir := t.TempDir() | |
| 958 | + | configFile := filepath.Join(tmpDir, "config.toml") | |
| 959 | + | ||
| 960 | + | // Both popups and browsing_contexts present — transform should be a no-op for popups. | |
| 961 | + | content := ` | |
| 962 | + | [workspace] | |
| 963 | + | new_pane_url = "about:blank" | |
| 964 | + | ||
| 965 | + | [workspace.popups] | |
| 966 | + | behavior = "tabbed" | |
| 967 | + | ||
| 968 | + | [workspace.browsing_contexts] | |
| 969 | + | behavior = "split" | |
| 970 | + | placement = "right" | |
| 971 | + | ` | |
| 972 | + | err := os.WriteFile(configFile, []byte(content), 0o644) | |
| 973 | + | require.NoError(t, err) | |
| 974 | + | ||
| 975 | + | keysWithValues, err := m.getUserConfigKeysWithValues(configFile) | |
| 976 | + | require.NoError(t, err) | |
| 977 | + | ||
| 978 | + | // Browsing_contexts flattened keys should retain their original values. | |
| 979 | + | assert.Equal(t, "split", keysWithValues["workspace.browsing_contexts.behavior"]) | |
| 980 | + | assert.Equal(t, "right", keysWithValues["workspace.browsing_contexts.placement"]) | |
| 981 | + | ||
| 982 | + | // Popups should still exist (transform skipped when browsing_contexts already present). | |
| 983 | + | _, hasPopups := keysWithValues["workspace.popups.behavior"] | |
| 984 | + | assert.True(t, hasPopups, "popups should still exist when browsing_contexts already present") | |
| 985 | + | } | |
| 986 | + | ||
| 987 | + | func TestMigrator_GetUserConfigKeysWithValues_NoPopupsKey(t *testing.T) { | |
| 988 | + | m := NewMigrator() | |
| 989 | + | ||
| 990 | + | tmpDir := t.TempDir() | |
| 991 | + | configFile := filepath.Join(tmpDir, "config.toml") | |
| 992 | + | ||
| 993 | + | content := ` | |
| 994 | + | [workspace] | |
| 995 | + | new_pane_url = "about:blank" | |
| 996 | + | ||
| 997 | + | [workspace.browsing_contexts] | |
| 998 | + | behavior = "stacked" | |
| 999 | + | ` | |
| 1000 | + | err := os.WriteFile(configFile, []byte(content), 0o644) | |
| 1001 | + | require.NoError(t, err) | |
| 1002 | + | ||
| 1003 | + | keysWithValues, err := m.getUserConfigKeysWithValues(configFile) | |
| 1004 | + | require.NoError(t, err) | |
| 1005 | + | ||
| 1006 | + | // Should only have browsing_contexts, no popups. | |
| 1007 | + | assert.Equal(t, "stacked", keysWithValues["workspace.browsing_contexts.behavior"]) | |
| 1008 | + | ||
| 1009 | + | _, hasPopups := keysWithValues["workspace.popups.behavior"] | |
| 1010 | + | assert.False(t, hasPopups, "popups should not exist when absent from config") | |
| 1011 | + | } | |
| 1012 | + | ||
| 1013 | + | func TestMigrator_DetectChanges_ReportsLegacyBrowsingContextsRename(t *testing.T) { | |
| 1014 | + | home := t.TempDir() | |
| 1015 | + | t.Setenv("HOME", home) | |
| 1016 | + | t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) | |
| 1017 | + | t.Setenv("XDG_DATA_HOME", filepath.Join(home, ".local", "share")) | |
| 1018 | + | t.Setenv("XDG_STATE_HOME", filepath.Join(home, ".local", "state")) | |
| 1019 | + | t.Setenv("XDG_CACHE_HOME", filepath.Join(home, ".cache")) | |
| 1020 | + | ||
| 1021 | + | configFile, err := GetConfigFile() | |
| 1022 | + | require.NoError(t, err) | |
| 1023 | + | require.NoError(t, os.MkdirAll(filepath.Dir(configFile), 0o755)) | |
| 1024 | + | ||
| 1025 | + | content, err := encodeConfigToTOML(DefaultConfig()) | |
| 1026 | + | require.NoError(t, err) | |
| 1027 | + | content = strings.Replace(content, "[workspace.browsing_contexts]", "[workspace.popups]", 1) | |
| 1028 | + | require.NoError(t, os.WriteFile(configFile, []byte(content), 0o644)) | |
| 1029 | + | ||
| 1030 | + | m := NewMigrator() | |
| 1031 | + | changes, err := m.DetectChanges() | |
| 1032 | + | require.NoError(t, err) | |
| 1033 | + | ||
| 1034 | + | found := false | |
| 1035 | + | for _, change := range changes { | |
| 1036 | + | if change.Type == port.KeyChangeRenamed && | |
| 1037 | + | change.OldKey == "workspace.popups.behavior" && | |
| 1038 | + | change.NewKey == "workspace.browsing_contexts.behavior" { | |
| 1039 | + | found = true | |
| 1040 | + | break | |
| 1041 | + | } | |
| 1042 | + | } | |
| 1043 | + | assert.True(t, found, "expected legacy workspace.popups.behavior rename to be reported") | |
| 1044 | + | } | |
| 1045 | + | ||
| 1046 | + | func TestMigrator_Migrate_AppliesLegacyBrowsingContextsRename(t *testing.T) { | |
| 1047 | + | home := t.TempDir() | |
| 1048 | + | t.Setenv("HOME", home) | |
| 1049 | + | t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) | |
| 1050 | + | t.Setenv("XDG_DATA_HOME", filepath.Join(home, ".local", "share")) | |
| 1051 | + | t.Setenv("XDG_STATE_HOME", filepath.Join(home, ".local", "state")) | |
| 1052 | + | t.Setenv("XDG_CACHE_HOME", filepath.Join(home, ".cache")) | |
| 1053 | + | ||
| 1054 | + | configFile, err := GetConfigFile() | |
| 1055 | + | require.NoError(t, err) | |
| 1056 | + | require.NoError(t, os.MkdirAll(filepath.Dir(configFile), 0o755)) | |
| 1057 | + | ||
| 1058 | + | content, err := encodeConfigToTOML(DefaultConfig()) | |
| 1059 | + | require.NoError(t, err) | |
| 1060 | + | content = strings.Replace(content, "[workspace.browsing_contexts]", "[workspace.popups]", 1) | |
| 1061 | + | require.NoError(t, os.WriteFile(configFile, []byte(content), 0o644)) | |
| 1062 | + | ||
| 1063 | + | m := NewMigrator() | |
| 1064 | + | appliedKeys, err := m.Migrate() | |
| 1065 | + | require.NoError(t, err) | |
| 1066 | + | assert.Contains(t, appliedKeys, "workspace.popups.behavior -> workspace.browsing_contexts.behavior") | |
| 1067 | + | ||
| 1068 | + | migrated, err := os.ReadFile(configFile) | |
| 1069 | + | require.NoError(t, err) | |
| 1070 | + | assert.Contains(t, string(migrated), "[workspace.browsing_contexts]") | |
| 1071 | + | assert.NotContains(t, string(migrated), "[workspace.popups]") | |
| 1072 | + | } | |
| 1073 | + | ||
| 1074 | + | func TestMigrator_DetectChanges_MixedBrowsingContextsDoesNotDoubleReportLegacyRename(t *testing.T) { | |
| 1075 | + | home := t.TempDir() | |
| 1076 | + | t.Setenv("HOME", home) | |
| 1077 | + | t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) | |
| 1078 | + | t.Setenv("XDG_DATA_HOME", filepath.Join(home, ".local", "share")) | |
| 1079 | + | t.Setenv("XDG_STATE_HOME", filepath.Join(home, ".local", "state")) | |
| 1080 | + | t.Setenv("XDG_CACHE_HOME", filepath.Join(home, ".cache")) | |
| 1081 | + | ||
| 1082 | + | configFile, err := GetConfigFile() | |
| 1083 | + | require.NoError(t, err) | |
| 1084 | + | require.NoError(t, os.MkdirAll(filepath.Dir(configFile), 0o755)) | |
| 1085 | + | require.NoError(t, os.WriteFile(configFile, []byte(` | |
| 1086 | + | [workspace.popups] | |
| 1087 | + | placement = "left" | |
| 1088 | + | ||
| 1089 | + | [workspace.browsing_contexts] | |
| 1090 | + | behavior = "tabbed" | |
| 1091 | + | `), 0o644)) | |
| 1092 | + | ||
| 1093 | + | m := NewMigrator() | |
| 1094 | + | changes, err := m.DetectChanges() | |
| 1095 | + | require.NoError(t, err) | |
| 1096 | + | ||
| 1097 | + | renameCount := 0 | |
| 1098 | + | for _, change := range changes { | |
| 1099 | + | if change.Type == port.KeyChangeRenamed && | |
| 1100 | + | change.OldKey == "workspace.popups.placement" && | |
| 1101 | + | change.NewKey == "workspace.browsing_contexts.placement" { | |
| 1102 | + | renameCount++ | |
| 1103 | + | } | |
| 1104 | + | assert.False(t, | |
| 1105 | + | change.Type == port.KeyChangeRemoved && change.OldKey == "workspace.popups.placement", | |
| 1106 | + | "legacy popup placement should be reported as a rename, not also a removal", | |
| 1107 | + | ) | |
| 1108 | + | assert.False(t, | |
| 1109 | + | change.Type == port.KeyChangeAdded && change.NewKey == "workspace.browsing_contexts.placement", | |
| 1110 | + | "canonical browsing_contexts placement should be reported as a rename, not also an addition", | |
| 1111 | + | ) | |
| 1112 | + | } | |
| 1113 | + | assert.Equal(t, 1, renameCount) | |
| 1114 | + | } | |
| 1115 | + | ||
| 919 | 1116 | func TestMigrator_ReadRawConfig(t *testing.T) { | |
| 920 | 1117 | m := NewMigrator() | |
| 921 | 1118 | ||
M internal/infrastructure/config/schema.go +5 -4
| @@ -68,8 +68,11 @@ const ( | |||
| 68 | 68 | OmniboxInitialBehaviorNone = entity.OmniboxInitialBehaviorNone | |
| 69 | 69 | ) | |
| 70 | 70 | ||
| 71 | - | // PopupBehaviorConfig defines handling for popup windows. | |
| 72 | - | type PopupBehaviorConfig = entity.PopupBehaviorConfig | |
| 71 | + | // BrowsingContextConfig defines handling for browsing contexts (popups, tabs, new windows). | |
| 72 | + | type BrowsingContextConfig = entity.BrowsingContextConfig | |
| 73 | + | ||
| 74 | + | // Deprecated: PopupBehaviorConfig is a compatibility alias for BrowsingContextConfig. | |
| 75 | + | type PopupBehaviorConfig = entity.BrowsingContextConfig | |
| 73 | 76 | ||
| 74 | 77 | // WorkspaceConfig captures layout, pane, and tab behavior preferences. | |
| 75 | 78 | type WorkspaceConfig = entity.WorkspaceConfig | |
| @@ -381,5 +384,3 @@ type DownloadsConfig struct { | |||
| 381 | 384 | // Empty string means use XDG_DOWNLOAD_DIR or ~/Downloads. | |
| 382 | 385 | Path string `mapstructure:"path" yaml:"path" toml:"path"` | |
| 383 | 386 | } | |
| 384 | - | ||
| 385 | - | ||
M internal/infrastructure/config/schema_provider.go +15 -15
| @@ -485,56 +485,56 @@ func (*SchemaProvider) getWorkspaceKeys(defaults *Config) []entity.ConfigKeyInfo | |||
| 485 | 485 | Description: "Named floating pane URL profiles with keys, url, and optional desc", | |
| 486 | 486 | Section: SectionWorkspace, | |
| 487 | 487 | }, | |
| 488 | - | // Popups | |
| 488 | + | // Browsing contexts | |
| 489 | 489 | { | |
| 490 | - | Key: "workspace.popups.behavior", | |
| 490 | + | Key: "workspace.browsing_contexts.behavior", | |
| 491 | 491 | Type: "string", | |
| 492 | - | Default: string(defaults.Workspace.Popups.Behavior), | |
| 492 | + | Default: string(defaults.Workspace.BrowsingContexts.Behavior), | |
| 493 | 493 | Description: "How to handle JavaScript popup windows", | |
| 494 | 494 | Values: []string{"split", "stacked", "tabbed", "windowed"}, | |
| 495 | 495 | Section: SectionWorkspace, | |
| 496 | 496 | }, | |
| 497 | 497 | { | |
| 498 | - | Key: "workspace.popups.placement", | |
| 498 | + | Key: "workspace.browsing_contexts.placement", | |
| 499 | 499 | Type: "string", | |
| 500 | - | Default: defaults.Workspace.Popups.Placement, | |
| 500 | + | Default: defaults.Workspace.BrowsingContexts.Placement, | |
| 501 | 501 | Description: "Placement direction for split popups", | |
| 502 | 502 | Values: []string{"right", "left", "top", "bottom"}, | |
| 503 | 503 | Section: SectionWorkspace, | |
| 504 | 504 | }, | |
| 505 | 505 | { | |
| 506 | - | Key: "workspace.popups.blank_target_behavior", | |
| 506 | + | Key: "workspace.browsing_contexts.blank_target_behavior", | |
| 507 | 507 | Type: "string", | |
| 508 | - | Default: defaults.Workspace.Popups.BlankTargetBehavior, | |
| 508 | + | Default: defaults.Workspace.BrowsingContexts.BlankTargetBehavior, | |
| 509 | 509 | Description: "How to handle target=\"_blank\" links", | |
| 510 | 510 | Values: []string{"split", "stacked", "tabbed"}, | |
| 511 | 511 | Section: SectionWorkspace, | |
| 512 | 512 | }, | |
| 513 | 513 | { | |
| 514 | - | Key: "workspace.popups.open_in_new_pane", | |
| 514 | + | Key: "workspace.browsing_contexts.open_in_new_pane", | |
| 515 | 515 | Type: "bool", | |
| 516 | - | Default: fmt.Sprintf("%t", defaults.Workspace.Popups.OpenInNewPane), | |
| 516 | + | Default: fmt.Sprintf("%t", defaults.Workspace.BrowsingContexts.OpenInNewPane), | |
| 517 | 517 | Description: "Open popups in workspace instead of blocking", | |
| 518 | 518 | Section: SectionWorkspace, | |
| 519 | 519 | }, | |
| 520 | 520 | { | |
| 521 | - | Key: "workspace.popups.follow_pane_context", | |
| 521 | + | Key: "workspace.browsing_contexts.follow_pane_context", | |
| 522 | 522 | Type: "bool", | |
| 523 | - | Default: fmt.Sprintf("%t", defaults.Workspace.Popups.FollowPaneContext), | |
| 523 | + | Default: fmt.Sprintf("%t", defaults.Workspace.BrowsingContexts.FollowPaneContext), | |
| 524 | 524 | Description: "Popup placement follows parent pane context", | |
| 525 | 525 | Section: SectionWorkspace, | |
| 526 | 526 | }, | |
| 527 | 527 | { | |
| 528 | - | Key: "workspace.popups.enable_smart_detection", | |
| 528 | + | Key: "workspace.browsing_contexts.enable_smart_detection", | |
| 529 | 529 | Type: "bool", | |
| 530 | - | Default: fmt.Sprintf("%t", defaults.Workspace.Popups.EnableSmartDetection), | |
| 530 | + | Default: fmt.Sprintf("%t", defaults.Workspace.BrowsingContexts.EnableSmartDetection), | |
| 531 | 531 | Description: "Use window properties to detect popup intent", | |
| 532 | 532 | Section: SectionWorkspace, | |
| 533 | 533 | }, | |
| 534 | 534 | { | |
| 535 | - | Key: "workspace.popups.oauth_auto_close", | |
| 535 | + | Key: "workspace.browsing_contexts.oauth_auto_close", | |
| 536 | 536 | Type: "bool", | |
| 537 | - | Default: fmt.Sprintf("%t", defaults.Workspace.Popups.OAuthAutoClose), | |
| 537 | + | Default: fmt.Sprintf("%t", defaults.Workspace.BrowsingContexts.OAuthAutoClose), | |
| 538 | 538 | Description: "Auto-close OAuth popups after success", | |
| 539 | 539 | Section: SectionWorkspace, | |
| 540 | 540 | }, | |
M internal/infrastructure/config/transform_legacy.go +37 -0
| @@ -97,6 +97,43 @@ func (t *LegacyConfigTransformer) transformActionSection(rawConfig map[string]an | |||
| 97 | 97 | } | |
| 98 | 98 | } | |
| 99 | 99 | ||
| 100 | + | // TransformLegacyPopupsToBrowsingContexts maps old workspace.popups.* keys to | |
| 101 | + | // workspace.browsing_contexts.* when the new section is absent. This allows | |
| 102 | + | // existing user configs with [workspace.popups] to be silently migrated. | |
| 103 | + | func (t *LegacyConfigTransformer) TransformLegacyPopupsToBrowsingContexts(rawConfig map[string]any) { | |
| 104 | + | workspace, ok := rawConfig["workspace"].(map[string]any) | |
| 105 | + | if !ok { | |
| 106 | + | return | |
| 107 | + | } | |
| 108 | + | ||
| 109 | + | // Only transform if the new key does not already exist. | |
| 110 | + | if _, hasNew := workspace["browsing_contexts"]; hasNew { | |
| 111 | + | return | |
| 112 | + | } | |
| 113 | + | ||
| 114 | + | t.ForceLegacyPopupsToBrowsingContexts(rawConfig) | |
| 115 | + | } | |
| 116 | + | ||
| 117 | + | // ForceLegacyPopupsToBrowsingContexts maps old workspace.popups.* keys to | |
| 118 | + | // workspace.browsing_contexts.* even when the new section already exists in the | |
| 119 | + | // raw map. This is used by the loader because Viper's AllSettings includes | |
| 120 | + | // defaults for the new section that do not indicate user intent. | |
| 121 | + | func (*LegacyConfigTransformer) ForceLegacyPopupsToBrowsingContexts(rawConfig map[string]any) { | |
| 122 | + | workspace, ok := rawConfig["workspace"].(map[string]any) | |
| 123 | + | if !ok { | |
| 124 | + | return | |
| 125 | + | } | |
| 126 | + | ||
| 127 | + | oldPopups, ok := workspace["popups"] | |
| 128 | + | if !ok { | |
| 129 | + | return | |
| 130 | + | } | |
| 131 | + | ||
| 132 | + | // Move the popups map to browsing_contexts and delete the old key. | |
| 133 | + | workspace["browsing_contexts"] = oldPopups | |
| 134 | + | delete(workspace, "popups") | |
| 135 | + | } | |
| 136 | + | ||
| 100 | 137 | func (t *LegacyConfigTransformer) getDefaultDesc(path []string, actionName string) string { | |
| 101 | 138 | switch { | |
| 102 | 139 | case path[0] == sectionWorkspace && path[1] == "pane_mode": | |
M internal/infrastructure/config/validation.go +12 -10
| @@ -117,32 +117,34 @@ func validateSearchEngine(config *Config) []string { | |||
| 117 | 117 | ||
| 118 | 118 | func validatePopups(config *Config) []string { | |
| 119 | 119 | var validationErrors []string | |
| 120 | - | switch config.Workspace.Popups.Behavior { | |
| 120 | + | // Validate against BrowsingContexts (canonical field). | |
| 121 | + | // Popups is a runtime alias synced during normalization. | |
| 122 | + | switch config.Workspace.BrowsingContexts.Behavior { | |
| 121 | 123 | case PopupBehaviorSplit, PopupBehaviorStacked, PopupBehaviorTabbed, PopupBehaviorWindowed: | |
| 122 | 124 | default: | |
| 123 | 125 | validationErrors = append(validationErrors, fmt.Sprintf( | |
| 124 | - | "workspace.popups.behavior must be one of: split, stacked, tabbed, windowed (got: %s)", | |
| 125 | - | config.Workspace.Popups.Behavior, | |
| 126 | + | "workspace.browsing_contexts.behavior must be one of: split, stacked, tabbed, windowed (got: %s)", | |
| 127 | + | config.Workspace.BrowsingContexts.Behavior, | |
| 126 | 128 | )) | |
| 127 | 129 | } | |
| 128 | 130 | ||
| 129 | - | if config.Workspace.Popups.Behavior == PopupBehaviorSplit { | |
| 130 | - | switch config.Workspace.Popups.Placement { | |
| 131 | + | if config.Workspace.BrowsingContexts.Behavior == PopupBehaviorSplit { | |
| 132 | + | switch config.Workspace.BrowsingContexts.Placement { | |
| 131 | 133 | case "right", "left", "top", "bottom": | |
| 132 | 134 | default: | |
| 133 | 135 | validationErrors = append(validationErrors, fmt.Sprintf( | |
| 134 | - | "workspace.popups.placement must be one of: right, left, top, bottom (got: %s)", | |
| 135 | - | config.Workspace.Popups.Placement, | |
| 136 | + | "workspace.browsing_contexts.placement must be one of: right, left, top, bottom (got: %s)", | |
| 137 | + | config.Workspace.BrowsingContexts.Placement, | |
| 136 | 138 | )) | |
| 137 | 139 | } | |
| 138 | 140 | } | |
| 139 | 141 | ||
| 140 | - | switch config.Workspace.Popups.BlankTargetBehavior { | |
| 142 | + | switch config.Workspace.BrowsingContexts.BlankTargetBehavior { | |
| 141 | 143 | case "split", "stacked", "tabbed": | |
| 142 | 144 | default: | |
| 143 | 145 | validationErrors = append(validationErrors, fmt.Sprintf( | |
| 144 | - | "workspace.popups.blank_target_behavior must be one of: split, stacked, tabbed (got: %s)", | |
| 145 | - | config.Workspace.Popups.BlankTargetBehavior, | |
| 146 | + | "workspace.browsing_contexts.blank_target_behavior must be one of: split, stacked, tabbed (got: %s)", | |
| 147 | + | config.Workspace.BrowsingContexts.BlankTargetBehavior, | |
| 146 | 148 | )) | |
| 147 | 149 | } | |
| 148 | 150 | return validationErrors | |
M internal/infrastructure/config/watcher.go +2 -0
| @@ -87,6 +87,8 @@ func (m *Manager) reload() error { | |||
| 87 | 87 | return err | |
| 88 | 88 | } | |
| 89 | 89 | ||
| 90 | + | m.transformLegacyConfig() | |
| 91 | + | ||
| 90 | 92 | config := &Config{} | |
| 91 | 93 | if err := m.viper.Unmarshal(config); err != nil { | |
| 92 | 94 | return err | |
M internal/infrastructure/webkit/webview.go +45 -14
| @@ -147,20 +147,23 @@ type WebView struct { | |||
| 147 | 147 | generation atomic.Uint64 | |
| 148 | 148 | ||
| 149 | 149 | // Callbacks (set by UI layer) | |
| 150 | - | OnLoadChanged func(LoadEvent) | |
| 151 | - | OnTitleChanged func(string) | |
| 152 | - | OnURIChanged func(string) | |
| 153 | - | OnProgressChanged func(float64) | |
| 154 | - | OnFaviconChanged func(*gdk.Texture) // Called when page favicon changes | |
| 155 | - | OnClose func() | |
| 156 | - | OnCreate func(PopupRequest) *WebView // Return new WebView or nil to block popup | |
| 157 | - | OnReadyToShow func() // Called when popup is ready to display | |
| 158 | - | OnLinkMiddleClick func(uri string) bool // Return true if handled (blocks navigation) | |
| 159 | - | OnEnterFullscreen func() bool // Return true to prevent fullscreen | |
| 160 | - | OnLeaveFullscreen func() bool // Return true to prevent leaving fullscreen | |
| 161 | - | OnAudioStateChanged func(playing bool) // Called when audio playback starts/stops | |
| 162 | - | OnLinkHover func(uri string) // Called when hovering over a link/image/media (empty string when leaving) | |
| 163 | - | OnWebProcessTerminated func(reason webkit.WebProcessTerminationReason, reasonLabel string, uri string) | |
| 150 | + | OnLoadChanged func(LoadEvent) | |
| 151 | + | OnTitleChanged func(string) | |
| 152 | + | OnURIChanged func(string) | |
| 153 | + | OnProgressChanged func(float64) | |
| 154 | + | OnFaviconChanged func(*gdk.Texture) // Called when page favicon changes | |
| 155 | + | OnClose func() | |
| 156 | + | OnCreate func(PopupRequest) *WebView // Return new WebView or nil to block popup | |
| 157 | + | OnReadyToShow func() // Called when popup is ready to display | |
| 158 | + | OnLinkMiddleClick func(uri string) bool // Return true if handled (blocks navigation) | |
| 159 | + | OnEnterFullscreen func() bool // Return true to prevent fullscreen | |
| 160 | + | OnLeaveFullscreen func() bool // Return true to prevent leaving fullscreen | |
| 161 | + | OnAudioStateChanged func(playing bool) // Called when audio playback starts/stops | |
| 162 | + | OnLinkHover func(uri string) // Called when hovering over a link/image/media (empty string when leaving) | |
| 163 | + | OnWebProcessTerminated func(reason webkit.WebProcessTerminationReason, reasonLabel string, uri string) | |
| 164 | + | browsingContextDecision port.HostDecision | |
| 165 | + | hasBrowsingContextDecision bool | |
| 166 | + | nativePopupHostAbort func() | |
| 164 | 167 | ||
| 165 | 168 | // PermissionRequest is called when a site requests permission (mic, camera, screen sharing). | |
| 166 | 169 | // Return true to indicate the request was handled. Call allow()/deny() to respond. | |
| @@ -518,6 +521,34 @@ func (wv *WebView) connectCloseSignal() { | |||
| 518 | 521 | wv.signalIDs = append(wv.signalIDs, uintptr(sigID)) | |
| 519 | 522 | } | |
| 520 | 523 | ||
| 524 | + | func (wv *WebView) SetBrowsingContextHostDecision(decision port.HostDecision) { | |
| 525 | + | wv.mu.Lock() | |
| 526 | + | wv.browsingContextDecision = decision | |
| 527 | + | wv.hasBrowsingContextDecision = true | |
| 528 | + | wv.mu.Unlock() | |
| 529 | + | } | |
| 530 | + | ||
| 531 | + | func (wv *WebView) BrowsingContextHostDecision() (port.HostDecision, bool) { | |
| 532 | + | wv.mu.RLock() | |
| 533 | + | defer wv.mu.RUnlock() | |
| 534 | + | return wv.browsingContextDecision, wv.hasBrowsingContextDecision | |
| 535 | + | } | |
| 536 | + | ||
| 537 | + | func (wv *WebView) SetNativePopupHostAbort(fn func()) { | |
| 538 | + | wv.mu.Lock() | |
| 539 | + | wv.nativePopupHostAbort = fn | |
| 540 | + | wv.mu.Unlock() | |
| 541 | + | } | |
| 542 | + | ||
| 543 | + | func (wv *WebView) AbortNativePopupHost() { | |
| 544 | + | wv.mu.RLock() | |
| 545 | + | fn := wv.nativePopupHostAbort | |
| 546 | + | wv.mu.RUnlock() | |
| 547 | + | if fn != nil { | |
| 548 | + | fn() | |
| 549 | + | } | |
| 550 | + | } | |
| 551 | + | ||
| 521 | 552 | func (wv *WebView) connectCreateSignal() { | |
| 522 | 553 | createCb := func(_ webkit.WebView, navActionPtr uintptr) gtk.Widget { | |
| 523 | 554 | if wv.OnCreate == nil { | |
M internal/ui/app.go +25 -1
| @@ -79,6 +79,7 @@ type App struct { | |||
| 79 | 79 | browserWindows map[string]*browserWindow | |
| 80 | 80 | browserWindowOrder []string // registration order of window IDs | |
| 81 | 81 | lastFocusedWindowID string | |
| 82 | + | nativePopupWindows map[port.WebViewID]*nativePopupWindow | |
| 82 | 83 | browserWindowFactory func(context.Context, string) (*browserWindow, error) | |
| 83 | 84 | dispatchOnMainThread func(func()) | |
| 84 | 85 | ||
| @@ -2918,6 +2919,15 @@ func (a *App) onShutdown(ctx context.Context) { | |||
| 2918 | 2919 | for _, tabID := range tabIDs { | |
| 2919 | 2920 | a.releaseFloatingSessionsForTab(ctx, tabID) | |
| 2920 | 2921 | } | |
| 2922 | + | if len(a.nativePopupWindows) > 0 { | |
| 2923 | + | popupIDs := make([]port.WebViewID, 0, len(a.nativePopupWindows)) | |
| 2924 | + | for popupID := range a.nativePopupWindows { | |
| 2925 | + | popupIDs = append(popupIDs, popupID) | |
| 2926 | + | } | |
| 2927 | + | for _, popupID := range popupIDs { | |
| 2928 | + | a.releaseNativePopupWindow(popupID, false, true) | |
| 2929 | + | } | |
| 2930 | + | } | |
| 2921 | 2931 | if a.engine != nil { | |
| 2922 | 2932 | if err := a.engine.Close(); err != nil { | |
| 2923 | 2933 | log.Warn().Err(err).Msg("failed to close engine") | |
| @@ -3097,15 +3107,29 @@ func (a *App) initCoordinators(ctx context.Context) { | |||
| 3097 | 3107 | } | |
| 3098 | 3108 | a.contentCoord.SetPopupConfig( | |
| 3099 | 3109 | a.engine.Factory(), | |
| 3100 | - | &a.deps.Config.Workspace.Popups, | |
| 3110 | + | &a.deps.Config.Workspace.BrowsingContexts, | |
| 3101 | 3111 | a.generateID, | |
| 3102 | 3112 | ) | |
| 3113 | + | a.contentCoord.SetPopupWindowIDResolver(func(paneID entity.PaneID) (string, bool) { | |
| 3114 | + | bw := a.browserWindowForAnyPane(paneID) | |
| 3115 | + | if bw == nil { | |
| 3116 | + | return "", false | |
| 3117 | + | } | |
| 3118 | + | return bw.id, true | |
| 3119 | + | }) | |
| 3103 | 3120 | a.contentCoord.SetOnInsertPopup(func(ctx context.Context, input content.InsertPopupInput) error { | |
| 3121 | + | if bw := a.browserWindowForAnyPane(input.ParentPaneID); bw != nil { | |
| 3122 | + | a.activateBrowserWindow(bw) | |
| 3123 | + | } | |
| 3104 | 3124 | return a.wsCoord.InsertPopup(ctx, input) | |
| 3105 | 3125 | }) | |
| 3106 | 3126 | a.contentCoord.SetOnClosePane(func(ctx context.Context, paneID entity.PaneID) error { | |
| 3127 | + | if bw := a.browserWindowForAnyPane(paneID); bw != nil { | |
| 3128 | + | a.activateBrowserWindow(bw) | |
| 3129 | + | } | |
| 3107 | 3130 | return a.wsCoord.ClosePaneByID(ctx, paneID) | |
| 3108 | 3131 | }) | |
| 3132 | + | a.contentCoord.SetOnOpenNativePopup(a.openNativePopupWindow) | |
| 3109 | 3133 | // Wire tabbed popup behavior to create new tabs in the originating window. | |
| 3110 | 3134 | a.wsCoord.SetOnCreatePopupTab(a.createPopupTab) | |
| 3111 | 3135 | ||
M internal/ui/browser_window.go +3 -0
| @@ -287,6 +287,9 @@ func (a *App) removeBrowserWindow(id string) { | |||
| 287 | 287 | if removed != nil { | |
| 288 | 288 | removed.clearShellState() | |
| 289 | 289 | } | |
| 290 | + | if a.contentCoord != nil { | |
| 291 | + | a.contentCoord.ClearPopupNamedContextsForWindow(id) | |
| 292 | + | } | |
| 290 | 293 | delete(a.browserWindows, id) | |
| 291 | 294 | fallback := a.deterministicBrowserWindowFallback() | |
| 292 | 295 | if a.lastFocusedWindowID == id { | |
A internal/ui/coordinator/content/auth_window_observer.go +61 -0
| @@ -0,0 +1,61 @@ | |||
| 1 | + | package content | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "time" | |
| 6 | + | ||
| 7 | + | "github.com/bnema/dumber/internal/application/port" | |
| 8 | + | "github.com/bnema/dumber/internal/logging" | |
| 9 | + | "github.com/bnema/puregotk/v4/glib" | |
| 10 | + | ) | |
| 11 | + | ||
| 12 | + | const nativePopupOAuthCloseDelay = 500 * time.Millisecond | |
| 13 | + | ||
| 14 | + | func (c *Coordinator) ObserveNativePopupAuth(ctx context.Context, input NativePopupInput) { | |
| 15 | + | if c == nil || input.PopupWebView == nil || !IsOAuthURL(input.TargetURI) { | |
| 16 | + | return | |
| 17 | + | } | |
| 18 | + | ||
| 19 | + | popupID := input.PopupWebView.ID() | |
| 20 | + | c.trackOAuthPopup(popupID, input.ParentPaneID, input.ParentURIAtOpen) | |
| 21 | + | ||
| 22 | + | oauthWV, ok := input.PopupWebView.(port.OAuthCallbackCapable) | |
| 23 | + | if !ok { | |
| 24 | + | logging.FromContext(ctx).Debug(). | |
| 25 | + | Uint64("popup_id", uint64(popupID)). | |
| 26 | + | Msg("native popup auth observation skipped: webview lacks OAuth callbacks") | |
| 27 | + | return | |
| 28 | + | } | |
| 29 | + | ||
| 30 | + | requestClose := func(reason string) { | |
| 31 | + | logging.FromContext(ctx).Info(). | |
| 32 | + | Uint64("popup_id", uint64(popupID)). | |
| 33 | + | Str("reason", reason). | |
| 34 | + | Msg("native popup oauth callback detected, closing popup") | |
| 35 | + | time.AfterFunc(nativePopupOAuthCloseDelay, func() { | |
| 36 | + | cb := glib.SourceFunc(func(_ uintptr) bool { | |
| 37 | + | if input.PopupWebView != nil && !input.PopupWebView.IsDestroyed() { | |
| 38 | + | oauthWV.Close() | |
| 39 | + | } | |
| 40 | + | return false | |
| 41 | + | }) | |
| 42 | + | glib.IdleAdd(&cb, 0) | |
| 43 | + | }) | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | oauthWV.AddNavigationCallback(func(uri string) { | |
| 47 | + | if !ShouldAutoClose(uri) { | |
| 48 | + | return | |
| 49 | + | } | |
| 50 | + | c.capturePopupOAuthState(popupID, uri) | |
| 51 | + | requestClose("navigation") | |
| 52 | + | }) | |
| 53 | + | oauthWV.AddCloseCallback(func() { | |
| 54 | + | c.handlePopupOAuthClose(context.Background(), popupID) | |
| 55 | + | }) | |
| 56 | + | ||
| 57 | + | logging.FromContext(ctx).Debug(). | |
| 58 | + | Uint64("popup_id", uint64(popupID)). | |
| 59 | + | Str("target_uri", logging.TruncateURL(input.TargetURI, logURLMaxLen)). | |
| 60 | + | Msg("native popup auth observer configured") | |
| 61 | + | } | |
A internal/ui/coordinator/content/browsing_context_policy.go +130 -0
| @@ -0,0 +1,130 @@ | |||
| 1 | + | package content | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "strings" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/dumber/internal/application/port" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | type browsingContextPolicy struct{} | |
| 10 | + | ||
| 11 | + | func (browsingContextPolicy) Decide(req port.NewBrowsingContextRequest, namedContextExists bool) port.HostDecision { | |
| 12 | + | decision := port.HostDecision{ | |
| 13 | + | RequestContextDisposition: req.RequestContextDisposition, | |
| 14 | + | RequiresNativeOpener: req.RequiresNativeOpener, | |
| 15 | + | } | |
| 16 | + | if decision.RequestContextDisposition == "" { | |
| 17 | + | decision.RequestContextDisposition = port.RequestContextInheritParent | |
| 18 | + | } | |
| 19 | + | ||
| 20 | + | if strings.TrimSpace(req.TargetURI) == "" { | |
| 21 | + | decision.Kind = port.HostDecisionDeny | |
| 22 | + | decision.Reason = "empty target URI" | |
| 23 | + | return decision | |
| 24 | + | } | |
| 25 | + | ||
| 26 | + | if req.AuthIntent { | |
| 27 | + | decision.Kind = port.HostDecisionCreateNativeWin | |
| 28 | + | decision.Reason = "auth intent requires native window" | |
| 29 | + | return decision | |
| 30 | + | } | |
| 31 | + | if req.RequiresNativeOpener { | |
| 32 | + | decision.Kind = port.HostDecisionCreateNativeWin | |
| 33 | + | decision.Reason = "native opener required" | |
| 34 | + | return decision | |
| 35 | + | } | |
| 36 | + | if isAmbiguousNativeBrowsingContext(req) { | |
| 37 | + | decision.Kind = port.HostDecisionCreateNativeWin | |
| 38 | + | decision.Reason = "ambiguous opener-coupled request prefers native window" | |
| 39 | + | return decision | |
| 40 | + | } | |
| 41 | + | ||
| 42 | + | name := reusableBrowsingContextName(req.TargetFrameName) | |
| 43 | + | if namedContextExists && name != "" { | |
| 44 | + | decision.Kind = port.HostDecisionReuseNamedPane | |
| 45 | + | decision.ReuseContextName = name | |
| 46 | + | decision.BrowsingContextName = name | |
| 47 | + | decision.Reason = "reuse named browsing context in current window" | |
| 48 | + | return decision | |
| 49 | + | } | |
| 50 | + | ||
| 51 | + | decision.Kind = port.HostDecisionCreatePane | |
| 52 | + | decision.BrowsingContextName = name | |
| 53 | + | decision.Reason = "pane-hosted browsing context" | |
| 54 | + | return decision | |
| 55 | + | } | |
| 56 | + | ||
| 57 | + | func buildPopupBrowsingContextRequest(req port.PopupRequest) port.NewBrowsingContextRequest { | |
| 58 | + | return port.NewBrowsingContextRequest{ | |
| 59 | + | ParentWebViewID: req.ParentViewID, | |
| 60 | + | SourceBrowserID: req.SourceBrowserID, | |
| 61 | + | SourceFrameID: req.SourceFrameID, | |
| 62 | + | SourceFrameURL: req.SourceFrameURL, | |
| 63 | + | TargetURI: req.TargetURI, | |
| 64 | + | TargetFrameName: req.FrameName, | |
| 65 | + | TargetDisposition: inferPopupWindowDisposition(req), | |
| 66 | + | IsUserGesture: req.IsUserGesture, | |
| 67 | + | NoJavaScriptAccess: req.NoJavaScriptAccess, | |
| 68 | + | WindowFeatures: req.WindowFeatures, | |
| 69 | + | TriggerKind: inferPopupTriggerKind(req), | |
| 70 | + | AuthIntent: IsOAuthURL(req.TargetURI), | |
| 71 | + | RequiresNativeOpener: false, | |
| 72 | + | RequestContextDisposition: port.RequestContextInheritParent, | |
| 73 | + | } | |
| 74 | + | } | |
| 75 | + | ||
| 76 | + | func buildLinkBrowsingContextRequest(parentWebViewID port.WebViewID, uri string) port.NewBrowsingContextRequest { | |
| 77 | + | return port.NewBrowsingContextRequest{ | |
| 78 | + | ParentWebViewID: parentWebViewID, | |
| 79 | + | TargetURI: uri, | |
| 80 | + | TargetFrameName: "_blank", | |
| 81 | + | TargetDisposition: port.WindowDispositionNewTab, | |
| 82 | + | IsUserGesture: true, | |
| 83 | + | NoJavaScriptAccess: true, | |
| 84 | + | TriggerKind: port.TriggerLinkNewPage, | |
| 85 | + | AuthIntent: IsOAuthURL(uri), | |
| 86 | + | RequestContextDisposition: port.RequestContextInheritParent, | |
| 87 | + | } | |
| 88 | + | } | |
| 89 | + | ||
| 90 | + | func inferPopupWindowDisposition(req port.PopupRequest) port.WindowDisposition { | |
| 91 | + | if req.TargetDisposition != "" { | |
| 92 | + | return req.TargetDisposition | |
| 93 | + | } | |
| 94 | + | if req.FrameName == "_blank" { | |
| 95 | + | return port.WindowDispositionNewTab | |
| 96 | + | } | |
| 97 | + | return port.WindowDispositionNewPopup | |
| 98 | + | } | |
| 99 | + | ||
| 100 | + | func inferPopupTriggerKind(req port.PopupRequest) port.TriggerKind { | |
| 101 | + | switch inferPopupWindowDisposition(req) { | |
| 102 | + | case port.WindowDispositionNewTab: | |
| 103 | + | if reusableBrowsingContextName(req.FrameName) != "" { | |
| 104 | + | return port.TriggerNamedTargetNavigation | |
| 105 | + | } | |
| 106 | + | return port.TriggerLinkNewPage | |
| 107 | + | case port.WindowDispositionNewPopup, port.WindowDispositionNewWindow: | |
| 108 | + | if reusableBrowsingContextName(req.FrameName) != "" { | |
| 109 | + | return port.TriggerNamedTargetNavigation | |
| 110 | + | } | |
| 111 | + | return port.TriggerScriptWindowOpen | |
| 112 | + | default: | |
| 113 | + | if reusableBrowsingContextName(req.FrameName) != "" { | |
| 114 | + | return port.TriggerNamedTargetNavigation | |
| 115 | + | } | |
| 116 | + | return port.TriggerUnknown | |
| 117 | + | } | |
| 118 | + | } | |
| 119 | + | ||
| 120 | + | func isAmbiguousNativeBrowsingContext(req port.NewBrowsingContextRequest) bool { | |
| 121 | + | return req.TriggerKind == port.TriggerUnknown && !req.NoJavaScriptAccess | |
| 122 | + | } | |
| 123 | + | ||
| 124 | + | func reusableBrowsingContextName(frameName string) string { | |
| 125 | + | name := strings.TrimSpace(frameName) | |
| 126 | + | if name == "" || strings.EqualFold(name, "_blank") { | |
| 127 | + | return "" | |
| 128 | + | } | |
| 129 | + | return name | |
| 130 | + | } | |
A internal/ui/coordinator/content/browsing_context_policy_test.go +110 -0
| @@ -0,0 +1,110 @@ | |||
| 1 | + | package content | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/dumber/internal/application/port" | |
| 7 | + | "github.com/stretchr/testify/assert" | |
| 8 | + | ) | |
| 9 | + | ||
| 10 | + | func TestBrowsingContextPolicyDecide(t *testing.T) { | |
| 11 | + | t.Parallel() | |
| 12 | + | ||
| 13 | + | policy := browsingContextPolicy{} | |
| 14 | + | tests := []struct { | |
| 15 | + | name string | |
| 16 | + | request port.NewBrowsingContextRequest | |
| 17 | + | namedContextExist bool | |
| 18 | + | wantKind port.HostDecisionKind | |
| 19 | + | wantName string | |
| 20 | + | }{ | |
| 21 | + | { | |
| 22 | + | name: "blank target stays pane-hosted", | |
| 23 | + | request: port.NewBrowsingContextRequest{ | |
| 24 | + | TargetURI: "https://example.com/docs", | |
| 25 | + | TargetFrameName: "_blank", | |
| 26 | + | TriggerKind: port.TriggerLinkNewPage, | |
| 27 | + | TargetDisposition: port.WindowDispositionNewTab, | |
| 28 | + | RequestContextDisposition: port.RequestContextInheritParent, | |
| 29 | + | }, | |
| 30 | + | wantKind: port.HostDecisionCreatePane, | |
| 31 | + | }, | |
| 32 | + | { | |
| 33 | + | name: "named target reuses existing pane in same window", | |
| 34 | + | request: port.NewBrowsingContextRequest{ | |
| 35 | + | TargetURI: "https://example.com/second", | |
| 36 | + | TargetFrameName: "shared-pane", | |
| 37 | + | TriggerKind: port.TriggerNamedTargetNavigation, | |
| 38 | + | TargetDisposition: port.WindowDispositionNewPopup, | |
| 39 | + | RequestContextDisposition: port.RequestContextInheritParent, | |
| 40 | + | }, | |
| 41 | + | namedContextExist: true, | |
| 42 | + | wantKind: port.HostDecisionReuseNamedPane, | |
| 43 | + | wantName: "shared-pane", | |
| 44 | + | }, | |
| 45 | + | { | |
| 46 | + | name: "ordinary window.open stays pane-hosted", | |
| 47 | + | request: port.NewBrowsingContextRequest{ | |
| 48 | + | TargetURI: "https://example.com/help", | |
| 49 | + | TriggerKind: port.TriggerScriptWindowOpen, | |
| 50 | + | TargetDisposition: port.WindowDispositionNewPopup, | |
| 51 | + | NoJavaScriptAccess: false, | |
| 52 | + | RequestContextDisposition: port.RequestContextInheritParent, | |
| 53 | + | }, | |
| 54 | + | wantKind: port.HostDecisionCreatePane, | |
| 55 | + | }, | |
| 56 | + | { | |
| 57 | + | name: "auth intent forces native window", | |
| 58 | + | request: port.NewBrowsingContextRequest{ | |
| 59 | + | TargetURI: "https://accounts.google.com/o/oauth2/v2/auth", | |
| 60 | + | TriggerKind: port.TriggerScriptWindowOpen, | |
| 61 | + | TargetDisposition: port.WindowDispositionNewPopup, | |
| 62 | + | AuthIntent: true, | |
| 63 | + | RequestContextDisposition: port.RequestContextInheritParent, | |
| 64 | + | }, | |
| 65 | + | wantKind: port.HostDecisionCreateNativeWin, | |
| 66 | + | }, | |
| 67 | + | { | |
| 68 | + | name: "ambiguous opener-coupled request prefers native", | |
| 69 | + | request: port.NewBrowsingContextRequest{ | |
| 70 | + | TargetURI: "https://example.com/popup", | |
| 71 | + | TriggerKind: port.TriggerUnknown, | |
| 72 | + | TargetDisposition: port.WindowDispositionNewPopup, | |
| 73 | + | NoJavaScriptAccess: false, | |
| 74 | + | RequestContextDisposition: port.RequestContextInheritParent, | |
| 75 | + | }, | |
| 76 | + | wantKind: port.HostDecisionCreateNativeWin, | |
| 77 | + | }, | |
| 78 | + | { | |
| 79 | + | name: "empty target is denied", | |
| 80 | + | request: port.NewBrowsingContextRequest{ | |
| 81 | + | TargetURI: "", | |
| 82 | + | TriggerKind: port.TriggerLinkNewPage, | |
| 83 | + | RequestContextDisposition: port.RequestContextInheritParent, | |
| 84 | + | }, | |
| 85 | + | wantKind: port.HostDecisionDeny, | |
| 86 | + | }, | |
| 87 | + | } | |
| 88 | + | ||
| 89 | + | for _, tt := range tests { | |
| 90 | + | t.Run(tt.name, func(t *testing.T) { | |
| 91 | + | t.Parallel() | |
| 92 | + | got := policy.Decide(tt.request, tt.namedContextExist) | |
| 93 | + | assert.Equal(t, tt.wantKind, got.Kind) | |
| 94 | + | assert.Equal(t, tt.wantName, got.BrowsingContextName) | |
| 95 | + | assert.Equal(t, port.RequestContextInheritParent, got.RequestContextDisposition) | |
| 96 | + | }) | |
| 97 | + | } | |
| 98 | + | } | |
| 99 | + | ||
| 100 | + | func TestInferPopupTriggerKind_DoesNotTreatNoJavaScriptAccessFalseAsNativeSignal(t *testing.T) { | |
| 101 | + | t.Parallel() | |
| 102 | + | ||
| 103 | + | req := port.PopupRequest{ | |
| 104 | + | TargetURI: "https://example.com/window-open", | |
| 105 | + | NoJavaScriptAccess: false, | |
| 106 | + | TargetDisposition: port.WindowDispositionNewPopup, | |
| 107 | + | } | |
| 108 | + | ||
| 109 | + | assert.Equal(t, port.TriggerScriptWindowOpen, inferPopupTriggerKind(req)) | |
| 110 | + | } | |
M internal/ui/coordinator/content/lifecycle.go +2 -0
| @@ -58,6 +58,8 @@ func (c *Coordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) | |||
| 58 | 58 | if wv == nil { | |
| 59 | 59 | return | |
| 60 | 60 | } | |
| 61 | + | c.ensurePopupManager().clearReusableNamedPopupByPaneID(paneID) | |
| 62 | + | c.ensurePopupManager().clearReusableNamedPopupByWebViewID(wv.ID()) | |
| 61 | 63 | c.clearPendingAppearance(paneID) | |
| 62 | 64 | ||
| 63 | 65 | // CRITICAL: If this webview was inhibiting idle (fullscreen or audio playing), | |
M internal/ui/coordinator/content/lifecycle_test.go +39 -0
| @@ -131,6 +131,7 @@ func TestLifecycle_ReleaseWebView_ReleasesToPool(t *testing.T) { | |||
| 131 | 131 | t.Parallel() | |
| 132 | 132 | ||
| 133 | 133 | wv := mocks.NewMockWebView(t) | |
| 134 | + | wv.EXPECT().ID().Return(port.WebViewID(11)).Maybe() | |
| 134 | 135 | // idleInhibitor is nil so these won't be called | |
| 135 | 136 | wv.EXPECT().IsFullscreen().Return(false).Maybe() | |
| 136 | 137 | wv.EXPECT().IsPlayingAudio().Return(false).Maybe() | |
| @@ -148,6 +149,41 @@ func TestLifecycle_ReleaseWebView_ReleasesToPool(t *testing.T) { | |||
| 148 | 149 | assert.False(t, exists, "webview should be removed from map after release") | |
| 149 | 150 | } | |
| 150 | 151 | ||
| 152 | + | func TestLifecycle_ReleaseWebView_ClearsNamedBrowsingContextRegistration(t *testing.T) { | |
| 153 | + | t.Parallel() | |
| 154 | + | ||
| 155 | + | paneID := entity.PaneID("pane-1") | |
| 156 | + | windowID := "window-1" | |
| 157 | + | wv := mocks.NewMockWebView(t) | |
| 158 | + | wv.EXPECT().ID().Return(port.WebViewID(15)).Maybe() | |
| 159 | + | wv.EXPECT().IsDestroyed().Return(false).Maybe() | |
| 160 | + | wv.EXPECT().IsFullscreen().Return(false).Maybe() | |
| 161 | + | wv.EXPECT().IsPlayingAudio().Return(false).Maybe() | |
| 162 | + | ||
| 163 | + | pool := mocks.NewMockWebViewPool(t) | |
| 164 | + | pool.EXPECT().Release(wv) | |
| 165 | + | ||
| 166 | + | c := newMinimalCoordinator() | |
| 167 | + | c.pool = pool | |
| 168 | + | c.popups = newPopupManager() | |
| 169 | + | c.popups.setWindowIDResolver(func(id entity.PaneID) (string, bool) { | |
| 170 | + | if id == paneID { | |
| 171 | + | return windowID, true | |
| 172 | + | } | |
| 173 | + | return "", false | |
| 174 | + | }) | |
| 175 | + | c.webViews[paneID] = wv | |
| 176 | + | c.popups.namedContexts.Register(windowID, "named-popup", paneID, wv.ID()) | |
| 177 | + | ||
| 178 | + | _, _, okBefore := c.popups.namedContexts.Lookup(windowID, "named-popup", c.getWebViewLocked, c.popups.windowIDForPane) | |
| 179 | + | require.True(t, okBefore) | |
| 180 | + | ||
| 181 | + | c.ReleaseWebView(context.Background(), paneID) | |
| 182 | + | ||
| 183 | + | _, _, okAfter := c.popups.namedContexts.Lookup(windowID, "named-popup", c.getWebViewLocked, c.popups.windowIDForPane) | |
| 184 | + | assert.False(t, okAfter) | |
| 185 | + | } | |
| 186 | + | ||
| 151 | 187 | func TestLifecycle_ReleaseWebView_NoopForUnknownPane(t *testing.T) { | |
| 152 | 188 | t.Parallel() | |
| 153 | 189 | ||
| @@ -165,6 +201,7 @@ func TestLifecycle_ReleaseWebView_UninhibitsIdleIfFullscreen(t *testing.T) { | |||
| 165 | 201 | t.Parallel() | |
| 166 | 202 | ||
| 167 | 203 | wv := mocks.NewMockWebView(t) | |
| 204 | + | wv.EXPECT().ID().Return(port.WebViewID(12)).Maybe() | |
| 168 | 205 | wv.EXPECT().IsFullscreen().Return(true) | |
| 169 | 206 | wv.EXPECT().IsPlayingAudio().Return(false) | |
| 170 | 207 | ||
| @@ -186,6 +223,7 @@ func TestLifecycle_ReleaseWebView_UninhibitsIdleIfPlayingAudio(t *testing.T) { | |||
| 186 | 223 | t.Parallel() | |
| 187 | 224 | ||
| 188 | 225 | wv := mocks.NewMockWebView(t) | |
| 226 | + | wv.EXPECT().ID().Return(port.WebViewID(13)).Maybe() | |
| 189 | 227 | wv.EXPECT().IsFullscreen().Return(false) | |
| 190 | 228 | wv.EXPECT().IsPlayingAudio().Return(true) | |
| 191 | 229 | ||
| @@ -207,6 +245,7 @@ func TestLifecycle_ReleaseWebView_DestroysWebViewWhenPoolIsNil(t *testing.T) { | |||
| 207 | 245 | t.Parallel() | |
| 208 | 246 | ||
| 209 | 247 | wv := mocks.NewMockWebView(t) | |
| 248 | + | wv.EXPECT().ID().Return(port.WebViewID(14)).Maybe() | |
| 210 | 249 | // idleInhibitor is nil so these won't be called | |
| 211 | 250 | wv.EXPECT().IsFullscreen().Return(false).Maybe() | |
| 212 | 251 | wv.EXPECT().IsPlayingAudio().Return(false).Maybe() | |
A internal/ui/coordinator/content/named_context_registry.go +116 -0
| @@ -0,0 +1,116 @@ | |||
| 1 | + | package content | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "sync" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/dumber/internal/application/port" | |
| 7 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 8 | + | ) | |
| 9 | + | ||
| 10 | + | type namedBrowsingContextKey struct { | |
| 11 | + | WindowID string | |
| 12 | + | Name string | |
| 13 | + | } | |
| 14 | + | ||
| 15 | + | type namedBrowsingContextState struct { | |
| 16 | + | PaneID entity.PaneID | |
| 17 | + | WebViewID port.WebViewID | |
| 18 | + | } | |
| 19 | + | ||
| 20 | + | type namedBrowsingContextRegistry struct { | |
| 21 | + | mu sync.RWMutex | |
| 22 | + | contexts map[namedBrowsingContextKey]namedBrowsingContextState | |
| 23 | + | } | |
| 24 | + | ||
| 25 | + | func newNamedBrowsingContextRegistry() *namedBrowsingContextRegistry { | |
| 26 | + | return &namedBrowsingContextRegistry{ | |
| 27 | + | contexts: make(map[namedBrowsingContextKey]namedBrowsingContextState), | |
| 28 | + | } | |
| 29 | + | } | |
| 30 | + | ||
| 31 | + | func (r *namedBrowsingContextRegistry) Register(windowID, name string, paneID entity.PaneID, webViewID port.WebViewID) { | |
| 32 | + | if r == nil || windowID == "" || name == "" || paneID == "" || webViewID == 0 { | |
| 33 | + | return | |
| 34 | + | } | |
| 35 | + | r.mu.Lock() | |
| 36 | + | r.contexts[namedBrowsingContextKey{WindowID: windowID, Name: name}] = namedBrowsingContextState{ | |
| 37 | + | PaneID: paneID, | |
| 38 | + | WebViewID: webViewID, | |
| 39 | + | } | |
| 40 | + | r.mu.Unlock() | |
| 41 | + | } | |
| 42 | + | ||
| 43 | + | func (r *namedBrowsingContextRegistry) Lookup( | |
| 44 | + | windowID, name string, | |
| 45 | + | lookupWebView func(entity.PaneID) port.WebView, | |
| 46 | + | resolveWindowID func(entity.PaneID) (string, bool), | |
| 47 | + | ) (namedBrowsingContextState, port.WebView, bool) { | |
| 48 | + | if r == nil || windowID == "" || name == "" || lookupWebView == nil || resolveWindowID == nil { | |
| 49 | + | return namedBrowsingContextState{}, nil, false | |
| 50 | + | } | |
| 51 | + | ||
| 52 | + | key := namedBrowsingContextKey{WindowID: windowID, Name: name} | |
| 53 | + | r.mu.RLock() | |
| 54 | + | state, ok := r.contexts[key] | |
| 55 | + | r.mu.RUnlock() | |
| 56 | + | if !ok { | |
| 57 | + | return namedBrowsingContextState{}, nil, false | |
| 58 | + | } | |
| 59 | + | ||
| 60 | + | currentWindowID, ok := resolveWindowID(state.PaneID) | |
| 61 | + | if !ok || currentWindowID != windowID { | |
| 62 | + | r.mu.Lock() | |
| 63 | + | delete(r.contexts, key) | |
| 64 | + | r.mu.Unlock() | |
| 65 | + | return namedBrowsingContextState{}, nil, false | |
| 66 | + | } | |
| 67 | + | ||
| 68 | + | wv := lookupWebView(state.PaneID) | |
| 69 | + | if wv == nil || wv.IsDestroyed() || wv.ID() != state.WebViewID { | |
| 70 | + | r.mu.Lock() | |
| 71 | + | delete(r.contexts, key) | |
| 72 | + | r.mu.Unlock() | |
| 73 | + | return namedBrowsingContextState{}, nil, false | |
| 74 | + | } | |
| 75 | + | ||
| 76 | + | return state, wv, true | |
| 77 | + | } | |
| 78 | + | ||
| 79 | + | func (r *namedBrowsingContextRegistry) UnregisterByPaneID(paneID entity.PaneID) { | |
| 80 | + | if r == nil || paneID == "" { | |
| 81 | + | return | |
| 82 | + | } | |
| 83 | + | r.mu.Lock() | |
| 84 | + | for key, state := range r.contexts { | |
| 85 | + | if state.PaneID == paneID { | |
| 86 | + | delete(r.contexts, key) | |
| 87 | + | } | |
| 88 | + | } | |
| 89 | + | r.mu.Unlock() | |
| 90 | + | } | |
| 91 | + | ||
| 92 | + | func (r *namedBrowsingContextRegistry) UnregisterByWebViewID(webViewID port.WebViewID) { | |
| 93 | + | if r == nil || webViewID == 0 { | |
| 94 | + | return | |
| 95 | + | } | |
| 96 | + | r.mu.Lock() | |
| 97 | + | for key, state := range r.contexts { | |
| 98 | + | if state.WebViewID == webViewID { | |
| 99 | + | delete(r.contexts, key) | |
| 100 | + | } | |
| 101 | + | } | |
| 102 | + | r.mu.Unlock() | |
| 103 | + | } | |
| 104 | + | ||
| 105 | + | func (r *namedBrowsingContextRegistry) UnregisterWindow(windowID string) { | |
| 106 | + | if r == nil || windowID == "" { | |
| 107 | + | return | |
| 108 | + | } | |
| 109 | + | r.mu.Lock() | |
| 110 | + | for key := range r.contexts { | |
| 111 | + | if key.WindowID == windowID { | |
| 112 | + | delete(r.contexts, key) | |
| 113 | + | } | |
| 114 | + | } | |
| 115 | + | r.mu.Unlock() | |
| 116 | + | } | |
A internal/ui/coordinator/content/named_context_registry_test.go +80 -0
| @@ -0,0 +1,80 @@ | |||
| 1 | + | package content | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/dumber/internal/application/port" | |
| 7 | + | "github.com/bnema/dumber/internal/application/port/mocks" | |
| 8 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 9 | + | "github.com/stretchr/testify/assert" | |
| 10 | + | ) | |
| 11 | + | ||
| 12 | + | func TestNamedBrowsingContextRegistry_LookupReusesWithinSameWindow(t *testing.T) { | |
| 13 | + | t.Parallel() | |
| 14 | + | ||
| 15 | + | reg := newNamedBrowsingContextRegistry() | |
| 16 | + | paneID := entity.PaneID("pane-1") | |
| 17 | + | webViewID := port.WebViewID(41) | |
| 18 | + | reg.Register("window-1", "shared-pane", paneID, webViewID) | |
| 19 | + | ||
| 20 | + | wv := mocks.NewMockWebView(t) | |
| 21 | + | wv.EXPECT().IsDestroyed().Return(false).Once() | |
| 22 | + | wv.EXPECT().ID().Return(webViewID).Once() | |
| 23 | + | ||
| 24 | + | state, gotWV, ok := reg.Lookup( | |
| 25 | + | "window-1", | |
| 26 | + | "shared-pane", | |
| 27 | + | func(gotPaneID entity.PaneID) port.WebView { | |
| 28 | + | assert.Equal(t, paneID, gotPaneID) | |
| 29 | + | return wv | |
| 30 | + | }, | |
| 31 | + | func(gotPaneID entity.PaneID) (string, bool) { | |
| 32 | + | assert.Equal(t, paneID, gotPaneID) | |
| 33 | + | return "window-1", true | |
| 34 | + | }, | |
| 35 | + | ) | |
| 36 | + | ||
| 37 | + | assert.True(t, ok) | |
| 38 | + | assert.Equal(t, paneID, state.PaneID) | |
| 39 | + | assert.Same(t, wv, gotWV) | |
| 40 | + | } | |
| 41 | + | ||
| 42 | + | func TestNamedBrowsingContextRegistry_LookupIsolatedAcrossWindows(t *testing.T) { | |
| 43 | + | t.Parallel() | |
| 44 | + | ||
| 45 | + | reg := newNamedBrowsingContextRegistry() | |
| 46 | + | reg.Register("window-1", "shared-pane", entity.PaneID("pane-1"), port.WebViewID(41)) | |
| 47 | + | ||
| 48 | + | _, _, ok := reg.Lookup( | |
| 49 | + | "window-2", | |
| 50 | + | "shared-pane", | |
| 51 | + | func(entity.PaneID) port.WebView { return nil }, | |
| 52 | + | func(entity.PaneID) (string, bool) { return "window-2", true }, | |
| 53 | + | ) | |
| 54 | + | ||
| 55 | + | assert.False(t, ok) | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | func TestNamedBrowsingContextRegistry_DropsStaleWindowOwnershipOnLookup(t *testing.T) { | |
| 59 | + | t.Parallel() | |
| 60 | + | ||
| 61 | + | reg := newNamedBrowsingContextRegistry() | |
| 62 | + | paneID := entity.PaneID("pane-1") | |
| 63 | + | reg.Register("window-1", "shared-pane", paneID, port.WebViewID(41)) | |
| 64 | + | ||
| 65 | + | _, _, ok := reg.Lookup( | |
| 66 | + | "window-1", | |
| 67 | + | "shared-pane", | |
| 68 | + | func(entity.PaneID) port.WebView { return nil }, | |
| 69 | + | func(entity.PaneID) (string, bool) { return "window-2", true }, | |
| 70 | + | ) | |
| 71 | + | assert.False(t, ok) | |
| 72 | + | ||
| 73 | + | _, _, ok = reg.Lookup( | |
| 74 | + | "window-1", | |
| 75 | + | "shared-pane", | |
| 76 | + | func(entity.PaneID) port.WebView { return nil }, | |
| 77 | + | func(entity.PaneID) (string, bool) { return "window-1", true }, | |
| 78 | + | ) | |
| 79 | + | assert.False(t, ok) | |
| 80 | + | } | |
M internal/ui/coordinator/content/oauth.go +1 -30
| @@ -221,17 +221,7 @@ func composeOnLoadChanged(existing, next func(port.LoadEvent)) func(port.LoadEve | |||
| 221 | 221 | ||
| 222 | 222 | // setupOAuthAutoClose monitors the popup for OAuth callback URLs and auto-closes. | |
| 223 | 223 | // It uses URL pattern detection for standard OAuth callbacks (code=, access_token=, etc.) | |
| 224 | - | // For providers using postMessage (like Google Sign-In), we rely on the provider calling | |
| 225 | - | // window.close() which triggers WebKit's close signal. | |
| 226 | - | // A long safety timeout (30s) catches popups that get stuck. | |
| 227 | - | func popupUsesSyntheticOpenerSignals(wv port.WebView) bool { | |
| 228 | - | if wv == nil { | |
| 229 | - | return false | |
| 230 | - | } | |
| 231 | - | opener, ok := wv.(port.PopupOpenerCapable) | |
| 232 | - | return ok && opener.HasActivePopupOpenerBridge() | |
| 233 | - | } | |
| 234 | - | ||
| 224 | + | // and a long safety timeout for stuck popups. | |
| 235 | 225 | func (c *Coordinator) setupOAuthAutoClose( | |
| 236 | 226 | ctx context.Context, | |
| 237 | 227 | paneID entity.PaneID, | |
| @@ -313,11 +303,9 @@ func (c *Coordinator) setupOAuthAutoClose( | |||
| 313 | 303 | ||
| 314 | 304 | // Start safety timer immediately. | |
| 315 | 305 | startSafetyTimer() | |
| 316 | - | deferCloseOnNavigation := popupUsesSyntheticOpenerSignals(wv) | |
| 317 | 306 | log.Debug(). | |
| 318 | 307 | Str("pane", string(paneID)). | |
| 319 | 308 | Uint64("popup_id", uint64(popupID)). | |
| 320 | - | Bool("synthetic_opener_active", deferCloseOnNavigation). | |
| 321 | 309 | Msg("oauth auto-close configured") | |
| 322 | 310 | ||
| 323 | 311 | // Register navigation callback to check for OAuth callbacks on URI changes and committed loads. | |
| @@ -326,25 +314,8 @@ func (c *Coordinator) setupOAuthAutoClose( | |||
| 326 | 314 | return | |
| 327 | 315 | } | |
| 328 | 316 | c.capturePopupOAuthState(popupID, uri) | |
| 329 | - | if deferCloseOnNavigation { | |
| 330 | - | cancelSafetyTimer() | |
| 331 | - | return | |
| 332 | - | } | |
| 333 | 317 | requestOAuthClose("navigation") | |
| 334 | 318 | }) | |
| 335 | - | if opener, ok := wv.(port.PopupOpenerCapable); ok { | |
| 336 | - | opener.AddOpenerNavigationCallback(func(uri string) { | |
| 337 | - | if !ShouldAutoClose(uri) { | |
| 338 | - | return | |
| 339 | - | } | |
| 340 | - | c.capturePopupOAuthState(popupID, uri) | |
| 341 | - | requestOAuthClose("opener-navigation") | |
| 342 | - | }) | |
| 343 | - | opener.AddOpenerMessageCallback(func() { | |
| 344 | - | c.capturePopupOAuthMessage(popupID) | |
| 345 | - | requestOAuthClose("opener-message") | |
| 346 | - | }) | |
| 347 | - | } | |
| 348 | 319 | ||
| 349 | 320 | // Cancel safety timer on any close path. | |
| 350 | 321 | oauthWV.AddCloseCallback(func() { | |
M internal/ui/coordinator/content/oauth_test.go +36 -29
| @@ -14,17 +14,6 @@ import ( | |||
| 14 | 14 | "github.com/bnema/dumber/internal/domain/entity" | |
| 15 | 15 | ) | |
| 16 | 16 | ||
| 17 | - | type popupOpenerBridgeStateStub struct { | |
| 18 | - | *portmocks.MockWebView | |
| 19 | - | active bool | |
| 20 | - | } | |
| 21 | - | ||
| 22 | - | func (s *popupOpenerBridgeStateStub) EnablePopupOpenerBridge(port.WebView, bool) {} | |
| 23 | - | func (s *popupOpenerBridgeStateStub) AddOpenerMessageCallback(func()) {} | |
| 24 | - | func (s *popupOpenerBridgeStateStub) AddOpenerNavigationCallback(func(string)) { | |
| 25 | - | } | |
| 26 | - | func (s *popupOpenerBridgeStateStub) HasActivePopupOpenerBridge() bool { return s.active } | |
| 27 | - | ||
| 28 | 17 | type popupOAuthAutoCloseWebViewStub struct { | |
| 29 | 18 | *portmocks.MockWebView | |
| 30 | 19 | active bool | |
| @@ -32,6 +21,7 @@ type popupOAuthAutoCloseWebViewStub struct { | |||
| 32 | 21 | closeCallbacks []func() | |
| 33 | 22 | openerMessageCallbacks []func() | |
| 34 | 23 | openerNavigateCallbacks []func(string) | |
| 24 | + | closeCount int | |
| 35 | 25 | } | |
| 36 | 26 | ||
| 37 | 27 | func (s *popupOAuthAutoCloseWebViewStub) AddNavigationCallback(fn func(string)) { | |
| @@ -42,7 +32,7 @@ func (s *popupOAuthAutoCloseWebViewStub) AddCloseCallback(fn func()) { | |||
| 42 | 32 | s.closeCallbacks = append(s.closeCallbacks, fn) | |
| 43 | 33 | } | |
| 44 | 34 | ||
| 45 | - | func (*popupOAuthAutoCloseWebViewStub) Close() {} | |
| 35 | + | func (s *popupOAuthAutoCloseWebViewStub) Close() { s.closeCount++ } | |
| 46 | 36 | ||
| 47 | 37 | func (*popupOAuthAutoCloseWebViewStub) EnablePopupOpenerBridge(port.WebView, bool) {} | |
| 48 | 38 | ||
| @@ -56,16 +46,6 @@ func (s *popupOAuthAutoCloseWebViewStub) AddOpenerNavigationCallback(fn func(str | |||
| 56 | 46 | ||
| 57 | 47 | func (s *popupOAuthAutoCloseWebViewStub) HasActivePopupOpenerBridge() bool { return s.active } | |
| 58 | 48 | ||
| 59 | - | func TestPopupUsesSyntheticOpenerSignals_DetectsActiveBridge(t *testing.T) { | |
| 60 | - | wv := &popupOpenerBridgeStateStub{MockWebView: portmocks.NewMockWebView(t), active: true} | |
| 61 | - | assert.True(t, popupUsesSyntheticOpenerSignals(wv)) | |
| 62 | - | } | |
| 63 | - | ||
| 64 | - | func TestPopupUsesSyntheticOpenerSignals_RejectsPlainWebView(t *testing.T) { | |
| 65 | - | wv := portmocks.NewMockWebView(t) | |
| 66 | - | assert.False(t, popupUsesSyntheticOpenerSignals(wv)) | |
| 67 | - | } | |
| 68 | - | ||
| 69 | 49 | func TestComposeOnClose_Order(t *testing.T) { | |
| 70 | 50 | var calls []string | |
| 71 | 51 | ||
| @@ -119,19 +99,46 @@ func TestCapturePopupOAuthMessage_MarksPopupSeenAndSuccessful(t *testing.T) { | |||
| 119 | 99 | assert.Equal(t, "postmessage://oauth-complete", state.CallbackURI) | |
| 120 | 100 | } | |
| 121 | 101 | ||
| 122 | - | func TestSetupOAuthAutoClose_OpenerNavigationIgnoresNonTerminalURI(t *testing.T) { | |
| 102 | + | func TestObserveNativePopupAuth_CapturesOAuthStateAndSchedulesParentResumeOnClose(t *testing.T) { | |
| 103 | + | popupID := port.WebViewID(999) | |
| 104 | + | wv := &popupOAuthAutoCloseWebViewStub{MockWebView: portmocks.NewMockWebView(t)} | |
| 105 | + | wv.EXPECT().ID().Return(popupID).Once() | |
| 106 | + | wv.EXPECT().IsDestroyed().Return(false).Maybe() | |
| 107 | + | ||
| 108 | + | c := &Coordinator{popups: newPopupManager()} | |
| 109 | + | c.ObserveNativePopupAuth(context.Background(), NativePopupInput{ | |
| 110 | + | ParentPaneID: entity.PaneID("parent-pane"), | |
| 111 | + | ParentURIAtOpen: "https://x.com/i/flow/login", | |
| 112 | + | PopupWebView: wv, | |
| 113 | + | TargetURI: "https://accounts.google.com/o/oauth2/v2/auth", | |
| 114 | + | }) | |
| 115 | + | require.Len(t, wv.navigationCallbacks, 1) | |
| 116 | + | require.Len(t, wv.closeCallbacks, 1) | |
| 117 | + | ||
| 118 | + | wv.navigationCallbacks[0]("https://x.com/googlepopupcallback?code=123") | |
| 119 | + | state := c.popups.popupOAuth[popupID] | |
| 120 | + | require.NotNil(t, state) | |
| 121 | + | assert.True(t, state.Seen) | |
| 122 | + | assert.True(t, state.Success) | |
| 123 | + | assert.Equal(t, "https://x.com/googlepopupcallback?code=123", state.CallbackURI) | |
| 124 | + | ||
| 125 | + | wv.closeCallbacks[0]() | |
| 126 | + | c.popups.mu.RLock() | |
| 127 | + | refreshTimer := c.popups.popupRefresh[entity.PaneID("parent-pane")] | |
| 128 | + | c.popups.mu.RUnlock() | |
| 129 | + | assert.NotNil(t, refreshTimer) | |
| 130 | + | } | |
| 131 | + | ||
| 132 | + | func TestSetupOAuthAutoClose_NavigationIgnoresNonTerminalURI(t *testing.T) { | |
| 123 | 133 | popupID := port.WebViewID(1000) | |
| 124 | - | wv := &popupOAuthAutoCloseWebViewStub{ | |
| 125 | - | MockWebView: portmocks.NewMockWebView(t), | |
| 126 | - | active: true, | |
| 127 | - | } | |
| 134 | + | wv := &popupOAuthAutoCloseWebViewStub{MockWebView: portmocks.NewMockWebView(t)} | |
| 128 | 135 | c := &Coordinator{popups: newPopupManager()} | |
| 129 | 136 | c.trackOAuthPopup(popupID, entity.PaneID("parent-pane"), "https://example.com/start") | |
| 130 | 137 | ||
| 131 | 138 | c.setupOAuthAutoClose(context.Background(), entity.PaneID("popup-pane"), popupID, wv) | |
| 132 | - | require.Len(t, wv.openerNavigateCallbacks, 1) | |
| 139 | + | require.Len(t, wv.navigationCallbacks, 1) | |
| 133 | 140 | ||
| 134 | - | wv.openerNavigateCallbacks[0]("https://example.com/intermediate") | |
| 141 | + | wv.navigationCallbacks[0]("https://example.com/intermediate") | |
| 135 | 142 | ||
| 136 | 143 | state, ok := c.popups.popupOAuth[popupID] | |
| 137 | 144 | require.True(t, ok) | |
M internal/ui/coordinator/content/popup.go +26 -15
| @@ -78,19 +78,6 @@ type PendingPopup struct { | |||
| 78 | 78 | CreatedAt time.Time | |
| 79 | 79 | } | |
| 80 | 80 | ||
| 81 | - | type namedPopupKey struct { | |
| 82 | - | ParentPaneID entity.PaneID | |
| 83 | - | FrameName string | |
| 84 | - | } | |
| 85 | - | ||
| 86 | - | type namedPopupState struct { | |
| 87 | - | WebView port.WebView | |
| 88 | - | } | |
| 89 | - | ||
| 90 | - | func isReusableNamedPopupFrame(frameName string) bool { | |
| 91 | - | return frameName != "" && frameName != "_blank" | |
| 92 | - | } | |
| 93 | - | ||
| 94 | 81 | // InsertPopupInput contains the data needed to insert a popup into the workspace. | |
| 95 | 82 | type InsertPopupInput struct { | |
| 96 | 83 | // ParentPaneID is the pane that spawned this popup. | |
| @@ -116,8 +103,20 @@ type InsertPopupInput struct { | |||
| 116 | 103 | TargetURI string | |
| 117 | 104 | } | |
| 118 | 105 | ||
| 106 | + | // NativePopupInput contains the data needed to host a native-required popup in | |
| 107 | + | // a dedicated top-level GTK window instead of the workspace. | |
| 108 | + | type NativePopupInput struct { | |
| 109 | + | ParentPaneID entity.PaneID | |
| 110 | + | ParentWebViewID port.WebViewID | |
| 111 | + | ParentURIAtOpen string | |
| 112 | + | PopupWebView port.WebView | |
| 113 | + | TargetURI string | |
| 114 | + | Request port.PopupRequest | |
| 115 | + | ObserveOAuthAutoClose bool | |
| 116 | + | } | |
| 117 | + | ||
| 119 | 118 | // GetBehavior returns the appropriate behavior based on popup type and config. | |
| 120 | - | func GetBehavior(popupType PopupType, cfg *entity.PopupBehaviorConfig) entity.PopupBehavior { | |
| 119 | + | func GetBehavior(popupType PopupType, cfg *entity.BrowsingContextConfig) entity.PopupBehavior { | |
| 121 | 120 | if cfg == nil { | |
| 122 | 121 | return entity.PopupBehaviorSplit // Default | |
| 123 | 122 | } | |
| @@ -146,12 +145,20 @@ func GetBehavior(popupType PopupType, cfg *entity.PopupBehaviorConfig) entity.Po | |||
| 146 | 145 | // SetPopupConfig configures popup handling. | |
| 147 | 146 | func (c *Coordinator) SetPopupConfig( | |
| 148 | 147 | factory port.WebViewFactory, | |
| 149 | - | popupConfig *entity.PopupBehaviorConfig, | |
| 148 | + | popupConfig *entity.BrowsingContextConfig, | |
| 150 | 149 | generateID func() string, | |
| 151 | 150 | ) { | |
| 152 | 151 | c.ensurePopupManager().setConfig(factory, popupConfig, generateID) | |
| 153 | 152 | } | |
| 154 | 153 | ||
| 154 | + | func (c *Coordinator) SetPopupWindowIDResolver(fn func(entity.PaneID) (string, bool)) { | |
| 155 | + | c.ensurePopupManager().setWindowIDResolver(fn) | |
| 156 | + | } | |
| 157 | + | ||
| 158 | + | func (c *Coordinator) ClearPopupNamedContextsForWindow(windowID string) { | |
| 159 | + | c.ensurePopupManager().clearReusableNamedPopupsForWindow(windowID) | |
| 160 | + | } | |
| 161 | + | ||
| 155 | 162 | // SetOnInsertPopup sets the callback to insert popups into the workspace. | |
| 156 | 163 | func (c *Coordinator) SetOnInsertPopup(fn func(ctx context.Context, input InsertPopupInput) error) { | |
| 157 | 164 | c.ensurePopupManager().setOnInsertPopup(fn) | |
| @@ -162,6 +169,10 @@ func (c *Coordinator) SetOnClosePane(fn func(ctx context.Context, paneID entity. | |||
| 162 | 169 | c.ensurePopupManager().setOnClosePane(fn) | |
| 163 | 170 | } | |
| 164 | 171 | ||
| 172 | + | func (c *Coordinator) SetOnOpenNativePopup(fn func(ctx context.Context, input NativePopupInput) error) { | |
| 173 | + | c.ensurePopupManager().setOnOpenNativePopup(fn) | |
| 174 | + | } | |
| 175 | + | ||
| 165 | 176 | // buildPopupCreateHandler returns the OnCreate callback for a WebView. | |
| 166 | 177 | // Returns nil if popup handling is not configured. | |
| 167 | 178 | func (c *Coordinator) buildPopupCreateHandler( | |
M internal/ui/coordinator/content/popup_manager.go +216 -173
| @@ -2,7 +2,6 @@ package content | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | - | "errors" | |
| 6 | 5 | "fmt" | |
| 7 | 6 | "strings" | |
| 8 | 7 | "sync" | |
| @@ -10,7 +9,6 @@ import ( | |||
| 10 | 9 | ||
| 11 | 10 | "github.com/bnema/dumber/internal/application/port" | |
| 12 | 11 | "github.com/bnema/dumber/internal/domain/entity" | |
| 13 | - | domainerrors "github.com/bnema/dumber/internal/domain/errors" | |
| 14 | 12 | "github.com/bnema/dumber/internal/logging" | |
| 15 | 13 | ) | |
| 16 | 14 | ||
| @@ -18,18 +16,19 @@ import ( | |||
| 18 | 16 | // adapter layer so popup pane bookkeeping and workspace orchestration do not | |
| 19 | 17 | // leak into the application/usecase or domain layers. | |
| 20 | 18 | type popupManager struct { | |
| 21 | - | factory port.WebViewFactory | |
| 22 | - | popupConfig *entity.PopupBehaviorConfig | |
| 23 | - | onInsertPopup func(ctx context.Context, input InsertPopupInput) error | |
| 24 | - | onClosePane func(ctx context.Context, paneID entity.PaneID) error | |
| 25 | - | generatePaneID func() string | |
| 26 | - | pendingPopups map[port.WebViewID]*PendingPopup | |
| 27 | - | namedPopups map[namedPopupKey]*namedPopupState | |
| 28 | - | popupOAuth map[port.WebViewID]*popupOAuthState | |
| 29 | - | popupRefresh map[entity.PaneID]*time.Timer | |
| 30 | - | relatedPopupUnsupported bool | |
| 31 | - | relatedPopupSupportDetected bool | |
| 32 | - | mu sync.RWMutex | |
| 19 | + | factory port.WebViewFactory | |
| 20 | + | popupConfig *entity.BrowsingContextConfig | |
| 21 | + | onInsertPopup func(ctx context.Context, input InsertPopupInput) error | |
| 22 | + | onOpenNativePopup func(ctx context.Context, input NativePopupInput) error | |
| 23 | + | onClosePane func(ctx context.Context, paneID entity.PaneID) error | |
| 24 | + | generatePaneID func() string | |
| 25 | + | windowIDForPane func(entity.PaneID) (string, bool) | |
| 26 | + | policy browsingContextPolicy | |
| 27 | + | namedContexts *namedBrowsingContextRegistry | |
| 28 | + | pendingPopups map[port.WebViewID]*PendingPopup | |
| 29 | + | popupOAuth map[port.WebViewID]*popupOAuthState | |
| 30 | + | popupRefresh map[entity.PaneID]*time.Timer | |
| 31 | + | mu sync.RWMutex | |
| 33 | 32 | } | |
| 34 | 33 | ||
| 35 | 34 | type popupOAuthState struct { | |
| @@ -82,8 +81,8 @@ func (pm *popupManager) ensureInitialized() { | |||
| 82 | 81 | if pm.pendingPopups == nil { | |
| 83 | 82 | pm.pendingPopups = make(map[port.WebViewID]*PendingPopup) | |
| 84 | 83 | } | |
| 85 | - | if pm.namedPopups == nil { | |
| 86 | - | pm.namedPopups = make(map[namedPopupKey]*namedPopupState) | |
| 84 | + | if pm.namedContexts == nil { | |
| 85 | + | pm.namedContexts = newNamedBrowsingContextRegistry() | |
| 87 | 86 | } | |
| 88 | 87 | if pm.popupOAuth == nil { | |
| 89 | 88 | pm.popupOAuth = make(map[port.WebViewID]*popupOAuthState) | |
| @@ -95,7 +94,7 @@ func (pm *popupManager) ensureInitialized() { | |||
| 95 | 94 | ||
| 96 | 95 | func (pm *popupManager) setConfig( | |
| 97 | 96 | factory port.WebViewFactory, | |
| 98 | - | popupConfig *entity.PopupBehaviorConfig, | |
| 97 | + | popupConfig *entity.BrowsingContextConfig, | |
| 99 | 98 | generateID func() string, | |
| 100 | 99 | ) { | |
| 101 | 100 | if pm == nil { | |
| @@ -105,11 +104,19 @@ func (pm *popupManager) setConfig( | |||
| 105 | 104 | pm.factory = factory | |
| 106 | 105 | pm.popupConfig = popupConfig | |
| 107 | 106 | pm.generatePaneID = generateID | |
| 107 | + | } | |
| 108 | 108 | ||
| 109 | - | pm.mu.Lock() | |
| 110 | - | pm.relatedPopupUnsupported = false | |
| 111 | - | pm.relatedPopupSupportDetected = false | |
| 112 | - | pm.mu.Unlock() | |
| 109 | + | func (pm *popupManager) setWindowIDResolver(fn func(entity.PaneID) (string, bool)) { | |
| 110 | + | if pm == nil { | |
| 111 | + | return | |
| 112 | + | } | |
| 113 | + | pm.windowIDForPane = fn | |
| 114 | + | } | |
| 115 | + | ||
| 116 | + | func (*popupManager) setBrowsingContextDecision(wv port.WebView, decision port.HostDecision) { | |
| 117 | + | if carrier, ok := wv.(port.BrowsingContextHostDecisionCapable); ok { | |
| 118 | + | carrier.SetBrowsingContextHostDecision(decision) | |
| 119 | + | } | |
| 113 | 120 | } | |
| 114 | 121 | ||
| 115 | 122 | func (pm *popupManager) setOnInsertPopup(fn func(ctx context.Context, input InsertPopupInput) error) { | |
| @@ -126,6 +133,13 @@ func (pm *popupManager) setOnClosePane(fn func(ctx context.Context, paneID entit | |||
| 126 | 133 | pm.onClosePane = fn | |
| 127 | 134 | } | |
| 128 | 135 | ||
| 136 | + | func (pm *popupManager) setOnOpenNativePopup(fn func(ctx context.Context, input NativePopupInput) error) { | |
| 137 | + | if pm == nil { | |
| 138 | + | return | |
| 139 | + | } | |
| 140 | + | pm.onOpenNativePopup = fn | |
| 141 | + | } | |
| 142 | + | ||
| 129 | 143 | func (pm *popupManager) createPopupPane( | |
| 130 | 144 | popupID port.WebViewID, | |
| 131 | 145 | parentPaneID entity.PaneID, | |
| @@ -147,70 +161,45 @@ func (pm *popupManager) createPopupPane( | |||
| 147 | 161 | return paneID, popupPane | |
| 148 | 162 | } | |
| 149 | 163 | ||
| 150 | - | func (pm *popupManager) relatedPopupSupportDisabled() bool { | |
| 151 | - | if pm == nil { | |
| 152 | - | return false | |
| 153 | - | } | |
| 154 | - | pm.mu.RLock() | |
| 155 | - | defer pm.mu.RUnlock() | |
| 156 | - | return pm.relatedPopupSupportDetected && pm.relatedPopupUnsupported | |
| 157 | - | } | |
| 158 | - | ||
| 159 | - | func (pm *popupManager) markRelatedPopupUnsupported() { | |
| 160 | - | if pm == nil { | |
| 161 | - | return | |
| 162 | - | } | |
| 163 | - | pm.mu.Lock() | |
| 164 | - | defer pm.mu.Unlock() | |
| 165 | - | pm.relatedPopupSupportDetected = true | |
| 166 | - | pm.relatedPopupUnsupported = true | |
| 167 | - | } | |
| 168 | - | ||
| 169 | - | func (pm *popupManager) markRelatedPopupSupported() { | |
| 170 | - | if pm == nil { | |
| 171 | - | return | |
| 164 | + | func (pm *popupManager) lookupReusableNamedPopup( | |
| 165 | + | parentPaneID entity.PaneID, | |
| 166 | + | frameName string, | |
| 167 | + | hooks popupCoordinatorHooks, | |
| 168 | + | ) (port.WebView, bool) { | |
| 169 | + | if pm == nil || pm.namedContexts == nil { | |
| 170 | + | return nil, false | |
| 172 | 171 | } | |
| 173 | - | pm.mu.Lock() | |
| 174 | - | defer pm.mu.Unlock() | |
| 175 | - | pm.relatedPopupSupportDetected = true | |
| 176 | - | pm.relatedPopupUnsupported = false | |
| 177 | - | } | |
| 178 | 172 | ||
| 179 | - | func (pm *popupManager) lookupReusableNamedPopup(parentPaneID entity.PaneID, frameName string) (*namedPopupState, bool) { | |
| 180 | - | if pm == nil || !isReusableNamedPopupFrame(frameName) { | |
| 173 | + | name := reusableBrowsingContextName(frameName) | |
| 174 | + | if name == "" || pm.windowIDForPane == nil || hooks.getWebView == nil { | |
| 181 | 175 | return nil, false | |
| 182 | 176 | } | |
| 183 | 177 | ||
| 184 | - | key := namedPopupKey{ParentPaneID: parentPaneID, FrameName: frameName} | |
| 185 | - | ||
| 186 | - | pm.mu.RLock() | |
| 187 | - | state, ok := pm.namedPopups[key] | |
| 188 | - | pm.mu.RUnlock() | |
| 189 | - | if !ok || state == nil || state.WebView == nil { | |
| 178 | + | windowID, ok := pm.windowIDForPane(parentPaneID) | |
| 179 | + | if !ok || windowID == "" { | |
| 190 | 180 | return nil, false | |
| 191 | 181 | } | |
| 192 | 182 | ||
| 193 | - | if state.WebView.IsDestroyed() { | |
| 194 | - | pm.mu.Lock() | |
| 195 | - | if current, ok := pm.namedPopups[key]; ok && current == state { | |
| 196 | - | delete(pm.namedPopups, key) | |
| 197 | - | } | |
| 198 | - | pm.mu.Unlock() | |
| 183 | + | _, wv, ok := pm.namedContexts.Lookup(windowID, name, hooks.getWebView, pm.windowIDForPane) | |
| 184 | + | if !ok { | |
| 199 | 185 | return nil, false | |
| 200 | 186 | } | |
| 201 | - | ||
| 202 | - | return state, true | |
| 187 | + | return wv, true | |
| 203 | 188 | } | |
| 204 | 189 | ||
| 205 | - | func (pm *popupManager) storeReusableNamedPopup(parentPaneID entity.PaneID, frameName string, wv port.WebView) { | |
| 206 | - | if pm == nil || !isReusableNamedPopupFrame(frameName) || wv == nil { | |
| 190 | + | func (pm *popupManager) storeReusableNamedPopup(parentPaneID entity.PaneID, frameName string, paneID entity.PaneID, wv port.WebView) { | |
| 191 | + | if pm == nil || pm.namedContexts == nil || wv == nil { | |
| 207 | 192 | return | |
| 208 | 193 | } | |
| 209 | - | ||
| 210 | - | key := namedPopupKey{ParentPaneID: parentPaneID, FrameName: frameName} | |
| 211 | - | pm.mu.Lock() | |
| 212 | - | pm.namedPopups[key] = &namedPopupState{WebView: wv} | |
| 213 | - | pm.mu.Unlock() | |
| 194 | + | name := reusableBrowsingContextName(frameName) | |
| 195 | + | if name == "" || pm.windowIDForPane == nil { | |
| 196 | + | return | |
| 197 | + | } | |
| 198 | + | windowID, ok := pm.windowIDForPane(parentPaneID) | |
| 199 | + | if !ok || windowID == "" { | |
| 200 | + | return | |
| 201 | + | } | |
| 202 | + | pm.namedContexts.Register(windowID, name, paneID, wv.ID()) | |
| 214 | 203 | } | |
| 215 | 204 | ||
| 216 | 205 | func (pm *popupManager) updatePendingPopupTarget(popupID port.WebViewID, targetURI string) { | |
| @@ -225,16 +214,24 @@ func (pm *popupManager) updatePendingPopupTarget(popupID port.WebViewID, targetU | |||
| 225 | 214 | } | |
| 226 | 215 | ||
| 227 | 216 | func (pm *popupManager) clearReusableNamedPopupByWebViewID(popupID port.WebViewID) { | |
| 228 | - | if pm == nil { | |
| 217 | + | if pm == nil || pm.namedContexts == nil { | |
| 229 | 218 | return | |
| 230 | 219 | } | |
| 231 | - | pm.mu.Lock() | |
| 232 | - | for key, state := range pm.namedPopups { | |
| 233 | - | if state != nil && state.WebView != nil && state.WebView.ID() == popupID { | |
| 234 | - | delete(pm.namedPopups, key) | |
| 235 | - | } | |
| 220 | + | pm.namedContexts.UnregisterByWebViewID(popupID) | |
| 221 | + | } | |
| 222 | + | ||
| 223 | + | func (pm *popupManager) clearReusableNamedPopupByPaneID(paneID entity.PaneID) { | |
| 224 | + | if pm == nil || pm.namedContexts == nil { | |
| 225 | + | return | |
| 236 | 226 | } | |
| 237 | - | pm.mu.Unlock() | |
| 227 | + | pm.namedContexts.UnregisterByPaneID(paneID) | |
| 228 | + | } | |
| 229 | + | ||
| 230 | + | func (pm *popupManager) clearReusableNamedPopupsForWindow(windowID string) { | |
| 231 | + | if pm == nil || pm.namedContexts == nil { | |
| 232 | + | return | |
| 233 | + | } | |
| 234 | + | pm.namedContexts.UnregisterWindow(windowID) | |
| 238 | 235 | } | |
| 239 | 236 | ||
| 240 | 237 | func (pm *popupManager) storePendingPopup(popupID port.WebViewID, pending *PendingPopup) { | |
| @@ -347,96 +344,109 @@ func (pm *popupManager) createPopupWebView( | |||
| 347 | 344 | ctx context.Context, | |
| 348 | 345 | parentID port.WebViewID, | |
| 349 | 346 | targetURI string, | |
| 350 | - | noJavaScriptAccess bool, | |
| 351 | - | ) (port.WebView, bool, error) { | |
| 347 | + | preparePaneHosted bool, | |
| 348 | + | ) (port.WebView, error) { | |
| 352 | 349 | if pm == nil || pm.factory == nil { | |
| 353 | - | return nil, false, fmt.Errorf("no webview factory configured") | |
| 354 | - | } | |
| 355 | - | ||
| 356 | - | log := logging.FromContext(ctx) | |
| 357 | - | var relatedErr error | |
| 358 | - | ||
| 359 | - | if !noJavaScriptAccess && !pm.relatedPopupSupportDisabled() { | |
| 360 | - | popupWV, err := pm.factory.CreateRelated(ctx, parentID) | |
| 361 | - | if err == nil && popupWV != nil { | |
| 362 | - | pm.markRelatedPopupSupported() | |
| 363 | - | return popupWV, false, nil | |
| 364 | - | } | |
| 365 | - | if err == nil { | |
| 366 | - | relatedErr = fmt.Errorf("related popup webview factory returned nil without error") | |
| 367 | - | log.Warn(). | |
| 368 | - | Uint64("parent_webview_id", uint64(parentID)). | |
| 369 | - | Str("target_uri", logging.TruncateURL(targetURI, logURLMaxLen)). | |
| 370 | - | Msg("related popup webview factory returned nil popup, falling back to regular webview") | |
| 371 | - | } else if errors.Is(err, domainerrors.ErrRelatedWebViewUnsupported) { | |
| 372 | - | relatedErr = err | |
| 373 | - | pm.markRelatedPopupUnsupported() | |
| 374 | - | log.Debug(). | |
| 375 | - | Err(err). | |
| 376 | - | Uint64("parent_webview_id", uint64(parentID)). | |
| 377 | - | Str("target_uri", logging.TruncateURL(targetURI, logURLMaxLen)). | |
| 378 | - | Msg("related popup webview unavailable, falling back to regular webview") | |
| 379 | - | } else { | |
| 380 | - | relatedErr = err | |
| 381 | - | log.Warn(). | |
| 382 | - | Err(err). | |
| 383 | - | Uint64("parent_webview_id", uint64(parentID)). | |
| 384 | - | Str("target_uri", logging.TruncateURL(targetURI, logURLMaxLen)). | |
| 385 | - | Msg("related popup webview creation failed, falling back to regular webview") | |
| 386 | - | } | |
| 387 | - | } else if noJavaScriptAccess { | |
| 388 | - | log.Debug(). | |
| 389 | - | Uint64("parent_webview_id", uint64(parentID)). | |
| 390 | - | Str("target_uri", logging.TruncateURL(targetURI, logURLMaxLen)). | |
| 391 | - | Msg("popup requested no JavaScript opener access, creating regular webview") | |
| 392 | - | } else { | |
| 393 | - | log.Debug(). | |
| 394 | - | Uint64("parent_webview_id", uint64(parentID)). | |
| 395 | - | Str("target_uri", logging.TruncateURL(targetURI, logURLMaxLen)). | |
| 396 | - | Msg("related popup webviews known unsupported, creating regular webview") | |
| 350 | + | return nil, fmt.Errorf("no webview factory configured") | |
| 397 | 351 | } | |
| 398 | 352 | ||
| 399 | - | popupWV, fallbackErr := pm.factory.Create(ctx) | |
| 400 | - | if fallbackErr != nil { | |
| 401 | - | if relatedErr != nil { | |
| 402 | - | return nil, false, fmt.Errorf("create popup webview: related failed: %w; fallback failed: %w", relatedErr, fallbackErr) | |
| 403 | - | } | |
| 404 | - | return nil, false, fmt.Errorf("create popup webview: fallback failed: %w", fallbackErr) | |
| 353 | + | popupWV, err := pm.factory.CreateRelated(ctx, parentID) | |
| 354 | + | if err != nil { | |
| 355 | + | return nil, fmt.Errorf("create related popup webview for %s: %w", logging.TruncateURL(targetURI, logURLMaxLen), err) | |
| 405 | 356 | } | |
| 406 | 357 | if popupWV == nil { | |
| 407 | - | if relatedErr != nil { | |
| 408 | - | return nil, false, fmt.Errorf("create popup webview: related failed: %w; fallback returned nil", relatedErr) | |
| 358 | + | return nil, fmt.Errorf("related popup webview factory returned nil for %s", logging.TruncateURL(targetURI, logURLMaxLen)) | |
| 359 | + | } | |
| 360 | + | if preparePaneHosted { | |
| 361 | + | if paneHosted, ok := popupWV.(port.PaneHostedBrowsingContextCapable); ok { | |
| 362 | + | paneHosted.PreparePaneHostedBrowsingContext() | |
| 409 | 363 | } | |
| 410 | - | return nil, false, fmt.Errorf("create popup webview: fallback returned nil") | |
| 411 | 364 | } | |
| 412 | - | ||
| 413 | - | return popupWV, true, nil | |
| 365 | + | return popupWV, nil | |
| 414 | 366 | } | |
| 415 | 367 | ||
| 416 | 368 | func (pm *popupManager) reuseNamedPopup( | |
| 417 | 369 | ctx context.Context, | |
| 370 | + | hooks popupCoordinatorHooks, | |
| 418 | 371 | parentPaneID entity.PaneID, | |
| 419 | 372 | frameName string, | |
| 420 | 373 | targetURI string, | |
| 421 | 374 | ) (port.WebView, bool) { | |
| 422 | 375 | log := logging.FromContext(ctx) | |
| 423 | 376 | ||
| 424 | - | if existing, ok := pm.lookupReusableNamedPopup(parentPaneID, frameName); ok { | |
| 425 | - | pm.updatePendingPopupTarget(existing.WebView.ID(), targetURI) | |
| 426 | - | if err := existing.WebView.LoadURI(ctx, targetURI); err != nil { | |
| 427 | - | log.Warn().Err(err). | |
| 428 | - | Str("target_uri", logging.TruncateURL(targetURI, logURLMaxLen)). | |
| 429 | - | Msg("failed to load target URI in reused popup") | |
| 430 | - | } | |
| 431 | - | log.Info(). | |
| 432 | - | Str("parent_pane", string(parentPaneID)). | |
| 433 | - | Str("frame_name", frameName). | |
| 377 | + | existingWV, ok := pm.lookupReusableNamedPopup(parentPaneID, frameName, hooks) | |
| 378 | + | if !ok { | |
| 379 | + | return nil, false | |
| 380 | + | } | |
| 381 | + | ||
| 382 | + | pm.updatePendingPopupTarget(existingWV.ID(), targetURI) | |
| 383 | + | if err := existingWV.LoadURI(ctx, targetURI); err != nil { | |
| 384 | + | log.Warn().Err(err). | |
| 434 | 385 | Str("target_uri", logging.TruncateURL(targetURI, logURLMaxLen)). | |
| 435 | - | Msg("reused named popup") | |
| 436 | - | return existing.WebView, true | |
| 386 | + | Msg("failed to load target URI in reused popup") | |
| 437 | 387 | } | |
| 388 | + | log.Info(). | |
| 389 | + | Str("parent_pane", string(parentPaneID)). | |
| 390 | + | Str("frame_name", frameName). | |
| 391 | + | Str("target_uri", logging.TruncateURL(targetURI, logURLMaxLen)). | |
| 392 | + | Msg("reused named popup") | |
| 393 | + | return existingWV, true | |
| 394 | + | } | |
| 395 | + | ||
| 396 | + | func (pm *popupManager) popupParentURIAtOpen( | |
| 397 | + | parentPaneID entity.PaneID, | |
| 398 | + | parentWV port.WebView, | |
| 399 | + | hooks popupCoordinatorHooks, | |
| 400 | + | targetURI string, | |
| 401 | + | ) string { | |
| 402 | + | if pm.popupConfig == nil || !pm.popupConfig.OAuthAutoClose || !IsOAuthURL(targetURI) { | |
| 403 | + | return "" | |
| 404 | + | } | |
| 405 | + | parentURIAtOpen := parentWV.URI() | |
| 406 | + | if parentURIAtOpen == "" && hooks.getWebView != nil { | |
| 407 | + | if parent := hooks.getWebView(parentPaneID); parent != nil { | |
| 408 | + | parentURIAtOpen = parent.URI() | |
| 409 | + | } | |
| 410 | + | } | |
| 411 | + | return parentURIAtOpen | |
| 412 | + | } | |
| 438 | 413 | ||
| 439 | - | return nil, false | |
| 414 | + | func (pm *popupManager) openNativePopup( | |
| 415 | + | ctx context.Context, | |
| 416 | + | parentPaneID entity.PaneID, | |
| 417 | + | parentID port.WebViewID, | |
| 418 | + | parentURIAtOpen string, | |
| 419 | + | req port.PopupRequest, | |
| 420 | + | decision port.HostDecision, | |
| 421 | + | ) port.WebView { | |
| 422 | + | log := logging.FromContext(ctx) | |
| 423 | + | if pm.onOpenNativePopup == nil { | |
| 424 | + | log.Warn().Msg("native popup host callback not configured") | |
| 425 | + | return nil | |
| 426 | + | } | |
| 427 | + | popupWV, err := pm.createPopupWebView(ctx, parentID, req.TargetURI, false) | |
| 428 | + | if err != nil { | |
| 429 | + | log.Error().Err(err).Msg("failed to create webview for native popup") | |
| 430 | + | return nil | |
| 431 | + | } | |
| 432 | + | pm.setBrowsingContextDecision(popupWV, decision) | |
| 433 | + | if err := pm.onOpenNativePopup(ctx, NativePopupInput{ | |
| 434 | + | ParentPaneID: parentPaneID, | |
| 435 | + | ParentWebViewID: parentID, | |
| 436 | + | ParentURIAtOpen: parentURIAtOpen, | |
| 437 | + | PopupWebView: popupWV, | |
| 438 | + | TargetURI: req.TargetURI, | |
| 439 | + | Request: req, | |
| 440 | + | ObserveOAuthAutoClose: pm.popupConfig != nil && pm.popupConfig.OAuthAutoClose && IsOAuthURL(req.TargetURI), | |
| 441 | + | }); err != nil { | |
| 442 | + | log.Error().Err(err).Msg("failed to open native popup host") | |
| 443 | + | popupWV.Destroy() | |
| 444 | + | return nil | |
| 445 | + | } | |
| 446 | + | if lifecycle, ok := popupWV.(port.PopupLifecycleCapable); ok { | |
| 447 | + | lifecycle.PrimePopupNavigation(req.TargetURI) | |
| 448 | + | } | |
| 449 | + | return popupWV | |
| 440 | 450 | } | |
| 441 | 451 | ||
| 442 | 452 | func (pm *popupManager) handlePopupCreate( | |
| @@ -466,31 +476,49 @@ func (pm *popupManager) handlePopupCreate( | |||
| 466 | 476 | } | |
| 467 | 477 | ||
| 468 | 478 | parentID := parentWV.ID() | |
| 469 | - | parentURIAtOpen := "" | |
| 470 | - | if pm.popupConfig != nil && pm.popupConfig.OAuthAutoClose && IsOAuthURL(req.TargetURI) { | |
| 471 | - | parentURIAtOpen = parentWV.URI() | |
| 472 | - | if parentURIAtOpen == "" && hooks.getWebView != nil { | |
| 473 | - | if parent := hooks.getWebView(parentPaneID); parent != nil { | |
| 474 | - | parentURIAtOpen = parent.URI() | |
| 475 | - | } | |
| 476 | - | } | |
| 477 | - | } | |
| 479 | + | parentURIAtOpen := pm.popupParentURIAtOpen(parentPaneID, parentWV, hooks, req.TargetURI) | |
| 480 | + | ||
| 481 | + | request := buildPopupBrowsingContextRequest(req) | |
| 482 | + | namedContextExists := false | |
| 478 | 483 | if !req.NoJavaScriptAccess { | |
| 479 | - | if reused, ok := pm.reuseNamedPopup(ctx, parentPaneID, req.FrameName, req.TargetURI); ok { | |
| 480 | - | return reused | |
| 484 | + | _, namedContextExists = pm.lookupReusableNamedPopup(parentPaneID, req.FrameName, hooks) | |
| 485 | + | } | |
| 486 | + | decision := pm.policy.Decide(request, namedContextExists) | |
| 487 | + | log.Debug(). | |
| 488 | + | Str("decision", string(decision.Kind)). | |
| 489 | + | Str("reason", decision.Reason). | |
| 490 | + | Str("context_name", decision.BrowsingContextName). | |
| 491 | + | Msg("browsing context host decision") | |
| 492 | + | ||
| 493 | + | switch decision.Kind { | |
| 494 | + | case port.HostDecisionDeny: | |
| 495 | + | return nil | |
| 496 | + | case port.HostDecisionReuseNamedPane: | |
| 497 | + | if req.NoJavaScriptAccess { | |
| 498 | + | log.Warn().Str("frame_name", req.FrameName).Msg("named browsing context reuse denied for noopener popup") | |
| 499 | + | return nil | |
| 500 | + | } | |
| 501 | + | reused, ok := pm.reuseNamedPopup(ctx, hooks, parentPaneID, req.FrameName, req.TargetURI) | |
| 502 | + | if !ok { | |
| 503 | + | log.Warn().Str("frame_name", req.FrameName).Msg("named browsing context reuse requested but target was unavailable") | |
| 504 | + | return nil | |
| 481 | 505 | } | |
| 506 | + | pm.setBrowsingContextDecision(reused, decision) | |
| 507 | + | return reused | |
| 508 | + | case port.HostDecisionCreateNativeWin: | |
| 509 | + | return pm.openNativePopup(ctx, parentPaneID, parentID, parentURIAtOpen, req, decision) | |
| 510 | + | case port.HostDecisionCreatePane: | |
| 511 | + | // Continue below. | |
| 512 | + | break | |
| 482 | 513 | } | |
| 483 | 514 | ||
| 484 | - | popupWV, usedRegularFallback, err := pm.createPopupWebView(ctx, parentID, req.TargetURI, req.NoJavaScriptAccess) | |
| 515 | + | popupWV, err := pm.createPopupWebView(ctx, parentID, req.TargetURI, true) | |
| 485 | 516 | if err != nil { | |
| 486 | 517 | log.Error().Err(err).Msg("failed to create webview for popup") | |
| 487 | 518 | return nil | |
| 488 | 519 | } | |
| 489 | - | if usedRegularFallback { | |
| 490 | - | if openerBridge, ok := popupWV.(port.PopupOpenerCapable); ok { | |
| 491 | - | openerBridge.EnablePopupOpenerBridge(parentWV, req.NoJavaScriptAccess) | |
| 492 | - | } | |
| 493 | - | } | |
| 520 | + | ||
| 521 | + | pm.setBrowsingContextDecision(popupWV, decision) | |
| 494 | 522 | ||
| 495 | 523 | popupType := DetectPopupType(req.FrameName) | |
| 496 | 524 | popupID := popupWV.ID() | |
| @@ -574,7 +602,7 @@ func (pm *popupManager) finishPopupCreate( | |||
| 574 | 602 | lifecycle.PrimePopupNavigation(create.Request.TargetURI) | |
| 575 | 603 | } | |
| 576 | 604 | if !create.Request.NoJavaScriptAccess { | |
| 577 | - | pm.storeReusableNamedPopup(create.ParentPaneID, create.Request.FrameName, create.PopupWebView) | |
| 605 | + | pm.storeReusableNamedPopup(create.ParentPaneID, create.Request.FrameName, create.PopupPaneID, create.PopupWebView) | |
| 578 | 606 | } | |
| 579 | 607 | if _, hasNativePopupLifecycle := create.PopupWebView.(port.PopupLifecycleCapable); !hasNativePopupLifecycle { | |
| 580 | 608 | if closeCapable, ok := create.PopupWebView.(port.OAuthCallbackCapable); ok { | |
| @@ -673,8 +701,7 @@ func (pm *popupManager) logPopupCloseSignal(ctx context.Context, hooks popupCoor | |||
| 673 | 701 | if wv := hooks.getWebView(paneID); wv != nil { | |
| 674 | 702 | fields = fields. | |
| 675 | 703 | Str("current_uri", logging.TruncateURL(wv.URI(), logURLMaxLen)). | |
| 676 | - | Bool("is_loading", wv.IsLoading()). | |
| 677 | - | Bool("synthetic_opener_active", popupUsesSyntheticOpenerSignals(wv)) | |
| 704 | + | Bool("is_loading", wv.IsLoading()) | |
| 678 | 705 | } | |
| 679 | 706 | } | |
| 680 | 707 | } | |
| @@ -761,8 +788,27 @@ func (pm *popupManager) handleLinkMiddleClick( | |||
| 761 | 788 | log.Warn().Msg("no webview factory, cannot handle middle-click") | |
| 762 | 789 | return false | |
| 763 | 790 | } | |
| 791 | + | if hooks.getWebView == nil { | |
| 792 | + | log.Warn().Msg("no parent webview lookup available for middle-click browsing context") | |
| 793 | + | return false | |
| 794 | + | } | |
| 795 | + | parentWV := hooks.getWebView(parentPaneID) | |
| 796 | + | if parentWV == nil { | |
| 797 | + | log.Warn().Str("parent_pane", string(parentPaneID)).Msg("parent webview not found for middle-click browsing context") | |
| 798 | + | return false | |
| 799 | + | } | |
| 800 | + | ||
| 801 | + | decision := pm.policy.Decide(buildLinkBrowsingContextRequest(parentWV.ID(), uri), false) | |
| 802 | + | log.Debug(). | |
| 803 | + | Str("decision", string(decision.Kind)). | |
| 804 | + | Str("reason", decision.Reason). | |
| 805 | + | Msg("middle-click browsing context host decision") | |
| 806 | + | if decision.Kind != port.HostDecisionCreatePane { | |
| 807 | + | log.Info().Str("decision", string(decision.Kind)).Msg("middle-click browsing context not pane-hosted") | |
| 808 | + | return false | |
| 809 | + | } | |
| 764 | 810 | ||
| 765 | - | newWV, err := pm.factory.Create(ctx) | |
| 811 | + | newWV, err := pm.createPopupWebView(ctx, parentWV.ID(), uri, true) | |
| 766 | 812 | if err != nil { | |
| 767 | 813 | log.Error().Err(err).Msg("failed to create webview for middle-click") | |
| 768 | 814 | return false | |
| @@ -786,10 +832,7 @@ func (pm *popupManager) handleLinkMiddleClick( | |||
| 786 | 832 | hooks.setupWebViewCallbacks(ctx, paneID, newWV) | |
| 787 | 833 | } | |
| 788 | 834 | ||
| 789 | - | behavior := entity.PopupBehaviorStacked | |
| 790 | - | if pm.popupConfig != nil && pm.popupConfig.BlankTargetBehavior != "" { | |
| 791 | - | behavior = entity.PopupBehavior(pm.popupConfig.BlankTargetBehavior) | |
| 792 | - | } | |
| 835 | + | behavior := GetBehavior(PopupTypeTab, pm.popupConfig) | |
| 793 | 836 | placement := "right" | |
| 794 | 837 | if pm.popupConfig != nil { | |
| 795 | 838 | placement = pm.popupConfig.Placement | |
M internal/ui/coordinator/content/popup_test.go +155 -119
| @@ -75,35 +75,35 @@ func TestGetBehavior_TabPopup_DefaultConfig(t *testing.T) { | |||
| 75 | 75 | t.Parallel() | |
| 76 | 76 | ||
| 77 | 77 | // BlankTargetBehavior is empty → falls through to default "stacked" | |
| 78 | - | cfg := &entity.PopupBehaviorConfig{} | |
| 78 | + | cfg := &entity.BrowsingContextConfig{} | |
| 79 | 79 | assert.Equal(t, entity.PopupBehaviorStacked, GetBehavior(PopupTypeTab, cfg)) | |
| 80 | 80 | } | |
| 81 | 81 | ||
| 82 | 82 | func TestGetBehavior_TabPopup_SplitConfig(t *testing.T) { | |
| 83 | 83 | t.Parallel() | |
| 84 | 84 | ||
| 85 | - | cfg := &entity.PopupBehaviorConfig{BlankTargetBehavior: "split"} | |
| 85 | + | cfg := &entity.BrowsingContextConfig{BlankTargetBehavior: "split"} | |
| 86 | 86 | assert.Equal(t, entity.PopupBehaviorSplit, GetBehavior(PopupTypeTab, cfg)) | |
| 87 | 87 | } | |
| 88 | 88 | ||
| 89 | 89 | func TestGetBehavior_TabPopup_StackedConfig(t *testing.T) { | |
| 90 | 90 | t.Parallel() | |
| 91 | 91 | ||
| 92 | - | cfg := &entity.PopupBehaviorConfig{BlankTargetBehavior: "stacked"} | |
| 92 | + | cfg := &entity.BrowsingContextConfig{BlankTargetBehavior: "stacked"} | |
| 93 | 93 | assert.Equal(t, entity.PopupBehaviorStacked, GetBehavior(PopupTypeTab, cfg)) | |
| 94 | 94 | } | |
| 95 | 95 | ||
| 96 | 96 | func TestGetBehavior_TabPopup_TabbedConfig(t *testing.T) { | |
| 97 | 97 | t.Parallel() | |
| 98 | 98 | ||
| 99 | - | cfg := &entity.PopupBehaviorConfig{BlankTargetBehavior: "tabbed"} | |
| 99 | + | cfg := &entity.BrowsingContextConfig{BlankTargetBehavior: "tabbed"} | |
| 100 | 100 | assert.Equal(t, entity.PopupBehaviorTabbed, GetBehavior(PopupTypeTab, cfg)) | |
| 101 | 101 | } | |
| 102 | 102 | ||
| 103 | 103 | func TestGetBehavior_JSPopup_UsesBehaviorField(t *testing.T) { | |
| 104 | 104 | t.Parallel() | |
| 105 | 105 | ||
| 106 | - | cfg := &entity.PopupBehaviorConfig{Behavior: entity.PopupBehaviorWindowed} | |
| 106 | + | cfg := &entity.BrowsingContextConfig{Behavior: entity.PopupBehaviorWindowed} | |
| 107 | 107 | assert.Equal(t, entity.PopupBehaviorWindowed, GetBehavior(PopupTypePopup, cfg)) | |
| 108 | 108 | } | |
| 109 | 109 | ||
| @@ -111,7 +111,7 @@ func TestGetBehavior_JSPopup_DefaultConfig(t *testing.T) { | |||
| 111 | 111 | t.Parallel() | |
| 112 | 112 | ||
| 113 | 113 | // Behavior zero-value is empty string; GetBehavior returns it as-is. | |
| 114 | - | cfg := &entity.PopupBehaviorConfig{} | |
| 114 | + | cfg := &entity.BrowsingContextConfig{} | |
| 115 | 115 | assert.Equal(t, entity.PopupBehavior(""), GetBehavior(PopupTypePopup, cfg)) | |
| 116 | 116 | } | |
| 117 | 117 | ||
| @@ -120,7 +120,7 @@ func TestGetBehavior_TabPopup_BlocksWhenOpenInNewPaneFalse(t *testing.T) { | |||
| 120 | 120 | ||
| 121 | 121 | // GetBehavior itself does not honor OpenInNewPane — that is enforced by | |
| 122 | 122 | // handlePopupCreate. GetBehavior still returns the configured value. | |
| 123 | - | cfg := &entity.PopupBehaviorConfig{ | |
| 123 | + | cfg := &entity.BrowsingContextConfig{ | |
| 124 | 124 | OpenInNewPane: false, | |
| 125 | 125 | BlankTargetBehavior: "split", | |
| 126 | 126 | } | |
| @@ -139,7 +139,7 @@ func newPopupCreateCoordinatorForTest(t *testing.T, popupID port.WebViewID) (con | |||
| 139 | 139 | popupWV.EXPECT().ID().Return(popupID).Once() | |
| 140 | 140 | ||
| 141 | 141 | factory := mocks.NewMockWebViewFactory(t) | |
| 142 | - | factory.EXPECT().Create(mock.Anything).Return(popupWV, nil).Once() | |
| 142 | + | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once() | |
| 143 | 143 | ||
| 144 | 144 | c := &Coordinator{ | |
| 145 | 145 | webViews: make(map[entity.PaneID]port.WebView), | |
| @@ -162,7 +162,7 @@ func TestHandlePopupCreate_PrimesPopupNavigationCapability(t *testing.T) { | |||
| 162 | 162 | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | |
| 163 | 163 | ||
| 164 | 164 | factory := mocks.NewMockWebViewFactory(t) | |
| 165 | - | factory.EXPECT().Create(mock.Anything).Return(popupWV, nil).Once() | |
| 165 | + | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once() | |
| 166 | 166 | ||
| 167 | 167 | c := &Coordinator{ | |
| 168 | 168 | webViews: make(map[entity.PaneID]port.WebView), | |
| @@ -230,7 +230,7 @@ func TestHandlePopupCreate_CleansUpPreRegisteredPopupWebViewWhenInsertionFails(t | |||
| 230 | 230 | require.Nil(t, c.getWebViewLocked(entity.PaneID("popup-pane"))) | |
| 231 | 231 | } | |
| 232 | 232 | ||
| 233 | - | func TestHandlePopupCreate_UsesRegularWebViewWhenPopupDisablesJavaScriptAccess(t *testing.T) { | |
| 233 | + | func TestHandlePopupCreate_UsesRelatedWebViewWhenPopupDisablesJavaScriptAccess(t *testing.T) { | |
| 234 | 234 | ctx := context.Background() | |
| 235 | 235 | parentPaneID := entity.PaneID("parent-pane") | |
| 236 | 236 | parentWV := mocks.NewMockWebView(t) | |
| @@ -245,7 +245,7 @@ func TestHandlePopupCreate_UsesRegularWebViewWhenPopupDisablesJavaScriptAccess(t | |||
| 245 | 245 | popupWV.EXPECT().LoadURI(mock.Anything, "https://example.com/noopener").Return(nil).Once() | |
| 246 | 246 | ||
| 247 | 247 | factory := mocks.NewMockWebViewFactory(t) | |
| 248 | - | factory.EXPECT().Create(mock.Anything).Return(popupWV, nil).Once() | |
| 248 | + | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once() | |
| 249 | 249 | ||
| 250 | 250 | c := &Coordinator{ | |
| 251 | 251 | webViews: make(map[entity.PaneID]port.WebView), | |
| @@ -267,74 +267,44 @@ func TestHandlePopupCreate_UsesRegularWebViewWhenPopupDisablesJavaScriptAccess(t | |||
| 267 | 267 | require.Same(t, popupWV, created) | |
| 268 | 268 | } | |
| 269 | 269 | ||
| 270 | - | func TestHandlePopupCreate_SkipsRelatedCreateAfterUnsupportedFactoryError(t *testing.T) { | |
| 270 | + | func TestHandlePopupCreate_DeniesWhenRelatedCreateIsUnsupported(t *testing.T) { | |
| 271 | 271 | ctx := context.Background() | |
| 272 | 272 | parentPaneID := entity.PaneID("parent-pane") | |
| 273 | 273 | parentWV := mocks.NewMockWebView(t) | |
| 274 | - | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Twice() | |
| 275 | - | ||
| 276 | - | firstPopupWV := mocks.NewMockWebView(t) | |
| 277 | - | firstPopupWV.EXPECT().ID().Return(port.WebViewID(202)).Once() | |
| 278 | - | firstPopupWV.EXPECT().Generation().Return(uint64(1)).Once() | |
| 279 | - | firstPopupWV.EXPECT().SetCallbacks(mock.Anything).Once() | |
| 280 | - | firstPopupWV.EXPECT().IsLoading().Return(false).Once() | |
| 281 | - | firstPopupWV.EXPECT().URI().Return("").Once() | |
| 282 | - | firstPopupWV.EXPECT().LoadURI(mock.Anything, "https://example.com/first").Return(nil).Once() | |
| 283 | - | ||
| 284 | - | secondPopupWV := mocks.NewMockWebView(t) | |
| 285 | - | secondPopupWV.EXPECT().ID().Return(port.WebViewID(203)).Once() | |
| 286 | - | secondPopupWV.EXPECT().Generation().Return(uint64(1)).Once() | |
| 287 | - | secondPopupWV.EXPECT().SetCallbacks(mock.Anything).Once() | |
| 288 | - | secondPopupWV.EXPECT().IsLoading().Return(false).Once() | |
| 289 | - | secondPopupWV.EXPECT().URI().Return("").Once() | |
| 290 | - | secondPopupWV.EXPECT().LoadURI(mock.Anything, "https://example.com/second").Return(nil).Once() | |
| 274 | + | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once() | |
| 291 | 275 | ||
| 292 | 276 | factory := mocks.NewMockWebViewFactory(t) | |
| 293 | 277 | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(nil, domainerrors.ErrRelatedWebViewUnsupported).Once() | |
| 294 | - | factory.EXPECT().Create(mock.Anything).Return(firstPopupWV, nil).Once() | |
| 295 | - | factory.EXPECT().Create(mock.Anything).Return(secondPopupWV, nil).Once() | |
| 296 | 278 | ||
| 279 | + | insertCalls := 0 | |
| 297 | 280 | c := &Coordinator{ | |
| 298 | 281 | webViews: make(map[entity.PaneID]port.WebView), | |
| 299 | 282 | popups: newPopupManager(), | |
| 300 | 283 | } | |
| 301 | 284 | c.SetPopupConfig(factory, nil, nil) | |
| 302 | 285 | c.SetOnInsertPopup(func(_ context.Context, input InsertPopupInput) error { | |
| 286 | + | insertCalls++ | |
| 303 | 287 | return nil | |
| 304 | 288 | }) | |
| 305 | 289 | ||
| 306 | - | first := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{ | |
| 307 | - | TargetURI: "https://example.com/first", | |
| 308 | - | FrameName: "_blank", | |
| 309 | - | IsUserGesture: true, | |
| 310 | - | }) | |
| 311 | - | second := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{ | |
| 312 | - | TargetURI: "https://example.com/second", | |
| 290 | + | created := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{ | |
| 291 | + | TargetURI: "https://example.com/edit", | |
| 313 | 292 | FrameName: "_blank", | |
| 314 | 293 | IsUserGesture: true, | |
| 315 | 294 | }) | |
| 316 | 295 | ||
| 317 | - | require.Same(t, firstPopupWV, first) | |
| 318 | - | require.Same(t, secondPopupWV, second) | |
| 296 | + | require.Nil(t, created) | |
| 297 | + | assert.Equal(t, 0, insertCalls) | |
| 319 | 298 | } | |
| 320 | 299 | ||
| 321 | - | func TestHandlePopupCreate_FallsBackToRegularWebViewWhenRelatedCreateIsUnsupported(t *testing.T) { | |
| 300 | + | func TestHandlePopupCreate_DeniesWhenRelatedCreateFailsUnexpectedly(t *testing.T) { | |
| 322 | 301 | ctx := context.Background() | |
| 323 | 302 | parentPaneID := entity.PaneID("parent-pane") | |
| 324 | 303 | parentWV := mocks.NewMockWebView(t) | |
| 325 | 304 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once() | |
| 326 | 305 | ||
| 327 | - | popupWV := mocks.NewMockWebView(t) | |
| 328 | - | popupWV.EXPECT().ID().Return(port.WebViewID(202)).Once() | |
| 329 | - | popupWV.EXPECT().Generation().Return(uint64(1)).Once() | |
| 330 | - | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | |
| 331 | - | popupWV.EXPECT().IsLoading().Return(false).Once() | |
| 332 | - | popupWV.EXPECT().URI().Return("").Once() | |
| 333 | - | popupWV.EXPECT().LoadURI(mock.Anything, "https://example.com/edit").Return(nil).Once() | |
| 334 | - | ||
| 335 | 306 | factory := mocks.NewMockWebViewFactory(t) | |
| 336 | - | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(nil, domainerrors.ErrRelatedWebViewUnsupported).Once() | |
| 337 | - | factory.EXPECT().Create(mock.Anything).Return(popupWV, nil).Once() | |
| 307 | + | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(nil, errors.New("boom")).Once() | |
| 338 | 308 | ||
| 339 | 309 | insertCalls := 0 | |
| 340 | 310 | c := &Coordinator{ | |
| @@ -344,9 +314,6 @@ func TestHandlePopupCreate_FallsBackToRegularWebViewWhenRelatedCreateIsUnsupport | |||
| 344 | 314 | c.SetPopupConfig(factory, nil, nil) | |
| 345 | 315 | c.SetOnInsertPopup(func(_ context.Context, input InsertPopupInput) error { | |
| 346 | 316 | insertCalls++ | |
| 347 | - | assert.Equal(t, parentPaneID, input.ParentPaneID) | |
| 348 | - | assert.Equal(t, popupWV, input.WebView) | |
| 349 | - | assert.Equal(t, "https://example.com/edit", input.TargetURI) | |
| 350 | 317 | return nil | |
| 351 | 318 | }) | |
| 352 | 319 | ||
| @@ -356,67 +323,145 @@ func TestHandlePopupCreate_FallsBackToRegularWebViewWhenRelatedCreateIsUnsupport | |||
| 356 | 323 | IsUserGesture: true, | |
| 357 | 324 | }) | |
| 358 | 325 | ||
| 359 | - | require.Same(t, popupWV, created) | |
| 326 | + | require.Nil(t, created) | |
| 327 | + | assert.Equal(t, 0, insertCalls) | |
| 328 | + | } | |
| 329 | + | ||
| 330 | + | func TestHandleLinkMiddleClick_UsesRelatedWebView(t *testing.T) { | |
| 331 | + | ctx := context.Background() | |
| 332 | + | parentPaneID := entity.PaneID("parent-pane") | |
| 333 | + | parentWV := mocks.NewMockWebView(t) | |
| 334 | + | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Twice() | |
| 335 | + | ||
| 336 | + | newWV := mocks.NewMockWebView(t) | |
| 337 | + | newWV.EXPECT().ID().Return(port.WebViewID(301)).Maybe() | |
| 338 | + | newWV.EXPECT().Generation().Return(uint64(1)).Maybe() | |
| 339 | + | newWV.EXPECT().SetCallbacks(mock.Anything).Maybe() | |
| 340 | + | newWV.EXPECT().LoadURI(mock.Anything, "https://example.com/middle-click").Return(nil).Maybe() | |
| 341 | + | ||
| 342 | + | factory := mocks.NewMockWebViewFactory(t) | |
| 343 | + | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(newWV, nil).Maybe() | |
| 344 | + | ||
| 345 | + | insertCalls := 0 | |
| 346 | + | c := &Coordinator{ | |
| 347 | + | webViews: map[entity.PaneID]port.WebView{parentPaneID: parentWV}, | |
| 348 | + | popups: newPopupManager(), | |
| 349 | + | } | |
| 350 | + | c.SetPopupConfig(factory, nil, nil) | |
| 351 | + | c.SetOnInsertPopup(func(_ context.Context, input InsertPopupInput) error { | |
| 352 | + | insertCalls++ | |
| 353 | + | assert.Equal(t, parentPaneID, input.ParentPaneID) | |
| 354 | + | assert.Same(t, newWV, input.WebView) | |
| 355 | + | assert.Equal(t, entity.PopupBehaviorSplit, input.Behavior) | |
| 356 | + | return nil | |
| 357 | + | }) | |
| 358 | + | ||
| 359 | + | handled := c.handleLinkMiddleClick(ctx, parentPaneID, "https://example.com/middle-click") | |
| 360 | + | assert.True(t, handled) | |
| 360 | 361 | assert.Equal(t, 1, insertCalls) | |
| 361 | 362 | } | |
| 362 | 363 | ||
| 363 | - | func TestHandlePopupCreate_FallsBackToRegularWebViewWhenRelatedCreateFailsUnexpectedly(t *testing.T) { | |
| 364 | + | func TestHandlePopupCreate_OpensNativePopupForAuthIntent(t *testing.T) { | |
| 364 | 365 | ctx := context.Background() | |
| 365 | 366 | parentPaneID := entity.PaneID("parent-pane") | |
| 366 | 367 | parentWV := mocks.NewMockWebView(t) | |
| 367 | 368 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once() | |
| 369 | + | parentWV.EXPECT().URI().Return("https://x.com/i/flow/login").Once() | |
| 368 | 370 | ||
| 369 | - | popupWV := mocks.NewMockWebView(t) | |
| 370 | - | popupWV.EXPECT().ID().Return(port.WebViewID(204)).Once() | |
| 371 | - | popupWV.EXPECT().Generation().Return(uint64(1)).Once() | |
| 372 | - | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | |
| 373 | - | popupWV.EXPECT().IsLoading().Return(false).Once() | |
| 374 | - | popupWV.EXPECT().URI().Return("").Once() | |
| 375 | - | popupWV.EXPECT().LoadURI(mock.Anything, "https://example.com/edit").Return(nil).Once() | |
| 371 | + | popupWV := &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)} | |
| 372 | + | popupWV.EXPECT().ID().Return(port.WebViewID(401)).Maybe() | |
| 376 | 373 | ||
| 377 | 374 | factory := mocks.NewMockWebViewFactory(t) | |
| 378 | - | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(nil, errors.New("boom")).Once() | |
| 379 | - | factory.EXPECT().Create(mock.Anything).Return(popupWV, nil).Once() | |
| 375 | + | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once() | |
| 380 | 376 | ||
| 381 | 377 | insertCalls := 0 | |
| 378 | + | nativeCalls := 0 | |
| 382 | 379 | c := &Coordinator{ | |
| 383 | 380 | webViews: make(map[entity.PaneID]port.WebView), | |
| 384 | 381 | popups: newPopupManager(), | |
| 385 | 382 | } | |
| 386 | - | c.SetPopupConfig(factory, nil, nil) | |
| 387 | - | c.SetOnInsertPopup(func(_ context.Context, input InsertPopupInput) error { | |
| 383 | + | c.SetPopupConfig(factory, &entity.BrowsingContextConfig{OpenInNewPane: true, OAuthAutoClose: true}, nil) | |
| 384 | + | c.SetOnInsertPopup(func(context.Context, InsertPopupInput) error { | |
| 388 | 385 | insertCalls++ | |
| 389 | - | assert.Equal(t, popupWV, input.WebView) | |
| 386 | + | return nil | |
| 387 | + | }) | |
| 388 | + | c.SetOnOpenNativePopup(func(_ context.Context, input NativePopupInput) error { | |
| 389 | + | nativeCalls++ | |
| 390 | + | assert.Equal(t, parentPaneID, input.ParentPaneID) | |
| 391 | + | assert.Equal(t, popupWV, input.PopupWebView) | |
| 392 | + | assert.Equal(t, "https://accounts.google.com/o/oauth2/v2/auth", input.TargetURI) | |
| 393 | + | assert.True(t, input.ObserveOAuthAutoClose) | |
| 390 | 394 | return nil | |
| 391 | 395 | }) | |
| 392 | 396 | ||
| 393 | 397 | created := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{ | |
| 394 | - | TargetURI: "https://example.com/edit", | |
| 395 | - | FrameName: "_blank", | |
| 398 | + | TargetURI: "https://accounts.google.com/o/oauth2/v2/auth", | |
| 399 | + | FrameName: "oauth-popup", | |
| 396 | 400 | IsUserGesture: true, | |
| 397 | 401 | }) | |
| 398 | 402 | ||
| 399 | - | require.Same(t, popupWV, created) | |
| 400 | - | assert.Equal(t, 1, insertCalls) | |
| 403 | + | require.Equal(t, popupWV, created) | |
| 404 | + | assert.Equal(t, 0, insertCalls) | |
| 405 | + | assert.Equal(t, 1, nativeCalls) | |
| 406 | + | assert.Equal(t, []string{"https://accounts.google.com/o/oauth2/v2/auth"}, popupWV.primed) | |
| 401 | 407 | } | |
| 402 | 408 | ||
| 403 | - | func TestHandlePopupCreate_PreservesOriginalTargetWhenRelatedCreateIsUnsupported(t *testing.T) { | |
| 409 | + | func TestHandlePopupCreate_NativePopupDoesNotForceOAuthObservationWhenDisabled(t *testing.T) { | |
| 404 | 410 | ctx := context.Background() | |
| 405 | 411 | parentPaneID := entity.PaneID("parent-pane") | |
| 406 | 412 | parentWV := mocks.NewMockWebView(t) | |
| 407 | 413 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once() | |
| 408 | 414 | ||
| 409 | - | popupWV := &popupOpenerBridgeWebViewStub{MockWebView: mocks.NewMockWebView(t)} | |
| 410 | - | popupWV.EXPECT().ID().Return(port.WebViewID(303)).Once() | |
| 411 | - | popupWV.EXPECT().Generation().Return(uint64(1)).Once() | |
| 412 | - | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | |
| 413 | - | popupWV.EXPECT().IsLoading().Return(false).Once() | |
| 414 | - | popupWV.EXPECT().URI().Return("").Once() | |
| 415 | - | popupWV.EXPECT().LoadURI(mock.Anything, "https://example.com/popup?redirect=%2Fcallback").Return(nil).Once() | |
| 415 | + | popupWV := &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)} | |
| 416 | + | popupWV.EXPECT().ID().Return(port.WebViewID(402)).Maybe() | |
| 416 | 417 | ||
| 417 | 418 | factory := mocks.NewMockWebViewFactory(t) | |
| 418 | - | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(nil, domainerrors.ErrRelatedWebViewUnsupported).Once() | |
| 419 | - | factory.EXPECT().Create(mock.Anything).Return(popupWV, nil).Once() | |
| 419 | + | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once() | |
| 420 | + | ||
| 421 | + | c := &Coordinator{ | |
| 422 | + | webViews: make(map[entity.PaneID]port.WebView), | |
| 423 | + | popups: newPopupManager(), | |
| 424 | + | } | |
| 425 | + | c.SetPopupConfig(factory, &entity.BrowsingContextConfig{OpenInNewPane: true, OAuthAutoClose: false}, nil) | |
| 426 | + | c.SetOnOpenNativePopup(func(_ context.Context, input NativePopupInput) error { | |
| 427 | + | assert.False(t, input.ObserveOAuthAutoClose) | |
| 428 | + | return nil | |
| 429 | + | }) | |
| 430 | + | ||
| 431 | + | created := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{ | |
| 432 | + | TargetURI: "https://accounts.google.com/o/oauth2/v2/auth", | |
| 433 | + | FrameName: "oauth-popup", | |
| 434 | + | IsUserGesture: true, | |
| 435 | + | }) | |
| 436 | + | ||
| 437 | + | require.Equal(t, popupWV, created) | |
| 438 | + | } | |
| 439 | + | ||
| 440 | + | func TestHandlePopupCreate_DoesNotReuseNamedPopupWhenNoJavaScriptAccess(t *testing.T) { | |
| 441 | + | ctx := context.Background() | |
| 442 | + | parentPaneID := entity.PaneID("parent-pane") | |
| 443 | + | parentWV := mocks.NewMockWebView(t) | |
| 444 | + | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Twice() | |
| 445 | + | ||
| 446 | + | firstWV := mocks.NewMockWebView(t) | |
| 447 | + | firstWV.EXPECT().ID().Return(port.WebViewID(501)).Maybe() | |
| 448 | + | firstWV.EXPECT().Generation().Return(uint64(1)).Maybe() | |
| 449 | + | firstWV.EXPECT().SetCallbacks(mock.Anything).Maybe() | |
| 450 | + | firstWV.EXPECT().IsLoading().Return(false).Maybe() | |
| 451 | + | firstWV.EXPECT().URI().Return("").Maybe() | |
| 452 | + | firstWV.EXPECT().LoadURI(mock.Anything, "https://example.com/first").Return(nil).Maybe() | |
| 453 | + | ||
| 454 | + | secondWV := mocks.NewMockWebView(t) | |
| 455 | + | secondWV.EXPECT().ID().Return(port.WebViewID(502)).Maybe() | |
| 456 | + | secondWV.EXPECT().Generation().Return(uint64(1)).Maybe() | |
| 457 | + | secondWV.EXPECT().SetCallbacks(mock.Anything).Maybe() | |
| 458 | + | secondWV.EXPECT().IsLoading().Return(false).Maybe() | |
| 459 | + | secondWV.EXPECT().URI().Return("").Maybe() | |
| 460 | + | secondWV.EXPECT().LoadURI(mock.Anything, "https://example.com/second").Return(nil).Maybe() | |
| 461 | + | ||
| 462 | + | factory := mocks.NewMockWebViewFactory(t) | |
| 463 | + | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(firstWV, nil).Once() | |
| 464 | + | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(secondWV, nil).Once() | |
| 420 | 465 | ||
| 421 | 466 | insertCalls := 0 | |
| 422 | 467 | c := &Coordinator{ | |
| @@ -424,22 +469,28 @@ func TestHandlePopupCreate_PreservesOriginalTargetWhenRelatedCreateIsUnsupported | |||
| 424 | 469 | popups: newPopupManager(), | |
| 425 | 470 | } | |
| 426 | 471 | c.SetPopupConfig(factory, nil, nil) | |
| 427 | - | c.SetOnInsertPopup(func(_ context.Context, input InsertPopupInput) error { | |
| 472 | + | c.SetPopupWindowIDResolver(func(entity.PaneID) (string, bool) { return "window-1", true }) | |
| 473 | + | c.SetOnInsertPopup(func(context.Context, InsertPopupInput) error { | |
| 428 | 474 | insertCalls++ | |
| 429 | - | assert.Equal(t, "https://example.com/popup?redirect=%2Fcallback", input.TargetURI) | |
| 430 | 475 | return nil | |
| 431 | 476 | }) | |
| 432 | 477 | ||
| 433 | - | created := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{ | |
| 434 | - | TargetURI: "https://example.com/popup?redirect=%2Fcallback", | |
| 435 | - | FrameName: "auth-popup", | |
| 436 | - | IsUserGesture: true, | |
| 478 | + | first := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{ | |
| 479 | + | TargetURI: "https://example.com/first", | |
| 480 | + | FrameName: "shared-pane", | |
| 481 | + | IsUserGesture: true, | |
| 482 | + | NoJavaScriptAccess: true, | |
| 483 | + | }) | |
| 484 | + | second := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{ | |
| 485 | + | TargetURI: "https://example.com/second", | |
| 486 | + | FrameName: "shared-pane", | |
| 487 | + | IsUserGesture: true, | |
| 488 | + | NoJavaScriptAccess: true, | |
| 437 | 489 | }) | |
| 438 | 490 | ||
| 439 | - | require.Same(t, popupWV, created) | |
| 440 | - | assert.Equal(t, 1, insertCalls) | |
| 441 | - | assert.Same(t, parentWV, popupWV.parent) | |
| 442 | - | assert.False(t, popupWV.noJavaScriptAccess) | |
| 491 | + | require.Same(t, firstWV, first) | |
| 492 | + | require.Same(t, secondWV, second) | |
| 493 | + | assert.Equal(t, 2, insertCalls) | |
| 443 | 494 | } | |
| 444 | 495 | ||
| 445 | 496 | func TestHandlePopupCreate_ReusesNamedPopup(t *testing.T) { | |
| @@ -449,19 +500,19 @@ func TestHandlePopupCreate_ReusesNamedPopup(t *testing.T) { | |||
| 449 | 500 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Twice() | |
| 450 | 501 | ||
| 451 | 502 | popupWV := mocks.NewMockWebView(t) | |
| 452 | - | popupWV.EXPECT().ID().Return(port.WebViewID(202)).Twice() | |
| 503 | + | popupWV.EXPECT().ID().Return(port.WebViewID(202)).Maybe() | |
| 453 | 504 | popupWV.EXPECT().Generation().Return(uint64(1)).Maybe() | |
| 454 | 505 | popupWV.EXPECT().SetCallbacks(mock.Anything).Maybe() | |
| 455 | 506 | popupWV.EXPECT().IsLoading().Return(false).Maybe() | |
| 456 | 507 | currentURI := "" | |
| 457 | 508 | popupWV.EXPECT().URI().RunAndReturn(func() string { return currentURI }).Maybe() | |
| 458 | 509 | popupWV.EXPECT().IsDestroyed().Return(false).Maybe() | |
| 459 | - | popupWV.EXPECT().LoadURI(mock.Anything, "https://accounts.google.com/o/oauth2/v2/auth?first").RunAndReturn(func(context.Context, string) error { | |
| 460 | - | currentURI = "https://accounts.google.com/o/oauth2/v2/auth?first" | |
| 510 | + | popupWV.EXPECT().LoadURI(mock.Anything, "https://example.com/first").RunAndReturn(func(context.Context, string) error { | |
| 511 | + | currentURI = "https://example.com/first" | |
| 461 | 512 | return nil | |
| 462 | 513 | }).Once() | |
| 463 | - | popupWV.EXPECT().LoadURI(mock.Anything, "https://accounts.google.com/o/oauth2/v2/auth?second").RunAndReturn(func(context.Context, string) error { | |
| 464 | - | currentURI = "https://accounts.google.com/o/oauth2/v2/auth?second" | |
| 514 | + | popupWV.EXPECT().LoadURI(mock.Anything, "https://example.com/second").RunAndReturn(func(context.Context, string) error { | |
| 515 | + | currentURI = "https://example.com/second" | |
| 465 | 516 | return nil | |
| 466 | 517 | }).Once() | |
| 467 | 518 | ||
| @@ -474,27 +525,28 @@ func TestHandlePopupCreate_ReusesNamedPopup(t *testing.T) { | |||
| 474 | 525 | popups: newPopupManager(), | |
| 475 | 526 | } | |
| 476 | 527 | c.SetPopupConfig(factory, nil, nil) | |
| 528 | + | c.SetPopupWindowIDResolver(func(entity.PaneID) (string, bool) { return "window-1", true }) | |
| 477 | 529 | c.SetOnInsertPopup(func(context.Context, InsertPopupInput) error { | |
| 478 | 530 | insertCalls++ | |
| 479 | 531 | return nil | |
| 480 | 532 | }) | |
| 481 | 533 | ||
| 482 | 534 | first := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{ | |
| 483 | - | TargetURI: "https://accounts.google.com/o/oauth2/v2/auth?first", | |
| 484 | - | FrameName: "g_credential_picker_x", | |
| 535 | + | TargetURI: "https://example.com/first", | |
| 536 | + | FrameName: "shared-pane", | |
| 485 | 537 | IsUserGesture: true, | |
| 486 | 538 | }) | |
| 487 | 539 | require.Same(t, popupWV, first) | |
| 488 | 540 | ||
| 489 | 541 | second := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{ | |
| 490 | - | TargetURI: "https://accounts.google.com/o/oauth2/v2/auth?second", | |
| 491 | - | FrameName: "g_credential_picker_x", | |
| 542 | + | TargetURI: "https://example.com/second", | |
| 543 | + | FrameName: "shared-pane", | |
| 492 | 544 | IsUserGesture: true, | |
| 493 | 545 | }) | |
| 494 | 546 | require.Same(t, popupWV, second) | |
| 495 | 547 | ||
| 496 | 548 | assert.Equal(t, 1, insertCalls) | |
| 497 | - | assert.Equal(t, "https://accounts.google.com/o/oauth2/v2/auth?second", currentURI) | |
| 549 | + | assert.Equal(t, "https://example.com/second", currentURI) | |
| 498 | 550 | } | |
| 499 | 551 | ||
| 500 | 552 | // --------------------------------------------------------------------------- | |
| @@ -598,7 +650,7 @@ func TestSetPopupConfig_SetsFields(t *testing.T) { | |||
| 598 | 650 | c := &Coordinator{} | |
| 599 | 651 | ||
| 600 | 652 | factory := &stubFactory{} | |
| 601 | - | cfg := &entity.PopupBehaviorConfig{Behavior: entity.PopupBehaviorSplit} | |
| 653 | + | cfg := &entity.BrowsingContextConfig{Behavior: entity.PopupBehaviorSplit} | |
| 602 | 654 | genID := func() string { return "test-id" } | |
| 603 | 655 | ||
| 604 | 656 | c.SetPopupConfig(factory, cfg, genID) | |
| @@ -633,22 +685,6 @@ func (*popupNavigationWebViewStub) SetOnReadyToShow(func()) {} | |||
| 633 | 685 | func (*popupNavigationWebViewStub) SetOnClose(func()) {} | |
| 634 | 686 | func (*popupNavigationWebViewStub) Show() {} | |
| 635 | 687 | ||
| 636 | - | type popupOpenerBridgeWebViewStub struct { | |
| 637 | - | *mocks.MockWebView | |
| 638 | - | parent port.WebView | |
| 639 | - | noJavaScriptAccess bool | |
| 640 | - | } | |
| 641 | - | ||
| 642 | - | func (s *popupOpenerBridgeWebViewStub) EnablePopupOpenerBridge(parent port.WebView, noJavaScriptAccess bool) { | |
| 643 | - | s.parent = parent | |
| 644 | - | s.noJavaScriptAccess = noJavaScriptAccess | |
| 645 | - | } | |
| 646 | - | ||
| 647 | - | func (*popupOpenerBridgeWebViewStub) AddOpenerMessageCallback(func()) {} | |
| 648 | - | func (*popupOpenerBridgeWebViewStub) AddOpenerNavigationCallback(func(string)) { | |
| 649 | - | } | |
| 650 | - | func (*popupOpenerBridgeWebViewStub) HasActivePopupOpenerBridge() bool { return false } | |
| 651 | - | ||
| 652 | 688 | // stubFactory satisfies port.WebViewFactory without importing the mocks package. | |
| 653 | 689 | type stubFactory struct{} | |
| 654 | 690 | ||
M internal/ui/input/global_shortcuts.go +22 -1
| @@ -399,9 +399,14 @@ func (h *GlobalShortcutHandler) isActiveWindowShortcutHandler() bool { | |||
| 399 | 399 | } | |
| 400 | 400 | ||
| 401 | 401 | func (h *GlobalShortcutHandler) suppressRepeatedShortcut(action Action, now time.Time) bool { | |
| 402 | - | if h == nil || !isRepeatedGlobalShortcutSuppressed(action) { | |
| 402 | + | if h == nil { | |
| 403 | 403 | return false | |
| 404 | 404 | } | |
| 405 | + | if !isRepeatedGlobalShortcutSuppressed(action) { | |
| 406 | + | if _, ok := ParseFloatingProfileTarget(action); !ok { | |
| 407 | + | return false | |
| 408 | + | } | |
| 409 | + | } | |
| 405 | 410 | if h.lastDispatchAt == nil { | |
| 406 | 411 | // Detached/reloading handlers clear dispatch state; consume one-shot shortcuts | |
| 407 | 412 | // rather than letting a stale GTK callback repeat destructive actions. | |
| @@ -419,6 +424,22 @@ func isRepeatedGlobalShortcutSuppressed(action Action) bool { | |||
| 419 | 424 | case ActionGoBack, | |
| 420 | 425 | ActionGoForward, | |
| 421 | 426 | ActionZoomReset, | |
| 427 | + | ActionReload, | |
| 428 | + | ActionHardReload, | |
| 429 | + | ActionPrintPage, | |
| 430 | + | ActionOpenOmnibox, | |
| 431 | + | ActionOpenFind, | |
| 432 | + | ActionOpenDevTools, | |
| 433 | + | ActionToggleFullscreen, | |
| 434 | + | ActionToggleFloatingPane, | |
| 435 | + | ActionToggleHistorySystemView, | |
| 436 | + | ActionToggleFavoritesSystemView, | |
| 437 | + | ActionToggleConfigSystemView, | |
| 438 | + | ActionCopyURL, | |
| 439 | + | ActionConsumeOrExpelLeft, | |
| 440 | + | ActionConsumeOrExpelRight, | |
| 441 | + | ActionConsumeOrExpelUp, | |
| 442 | + | ActionConsumeOrExpelDown, | |
| 422 | 443 | ActionClosePane, | |
| 423 | 444 | ActionCloseTab, | |
| 424 | 445 | ActionQuit, | |
M internal/ui/input/global_shortcuts_test.go +92 -0
| @@ -44,6 +44,18 @@ func TestGlobalShortcutHandlerSuppressesRepeatedBrowserNavigationActions(t *test | |||
| 44 | 44 | } | |
| 45 | 45 | } | |
| 46 | 46 | ||
| 47 | + | func TestGlobalShortcutHandlerSuppressesRepeatedPrintPageAction(t *testing.T) { | |
| 48 | + | h := &GlobalShortcutHandler{lastDispatchAt: make(map[Action]time.Time)} | |
| 49 | + | now := time.Unix(100, 0) | |
| 50 | + | ||
| 51 | + | if h.suppressRepeatedShortcut(ActionPrintPage, now) { | |
| 52 | + | t.Fatal("first print-page dispatch was suppressed") | |
| 53 | + | } | |
| 54 | + | if !h.suppressRepeatedShortcut(ActionPrintPage, now.Add(time.Millisecond)) { | |
| 55 | + | t.Fatal("repeated print-page dispatch was not suppressed") | |
| 56 | + | } | |
| 57 | + | } | |
| 58 | + | ||
| 47 | 59 | func TestGlobalShortcutHandlerSuppressesRepeatedTabIndexSwitchActions(t *testing.T) { | |
| 48 | 60 | actions := []Action{ | |
| 49 | 61 | ActionSwitchTabIndex1, | |
| @@ -120,6 +132,86 @@ func TestGlobalShortcutHandlerReturnsInactiveForNilOrDetached(t *testing.T) { | |||
| 120 | 132 | } | |
| 121 | 133 | } | |
| 122 | 134 | ||
| 135 | + | func TestGlobalShortcutHandlerSuppressesRepeatedReloadAction(t *testing.T) { | |
| 136 | + | h := &GlobalShortcutHandler{lastDispatchAt: make(map[Action]time.Time)} | |
| 137 | + | now := time.Unix(100, 0) | |
| 138 | + | ||
| 139 | + | if h.suppressRepeatedShortcut(ActionReload, now) { | |
| 140 | + | t.Fatal("first reload dispatch was suppressed") | |
| 141 | + | } | |
| 142 | + | if !h.suppressRepeatedShortcut(ActionReload, now.Add(time.Millisecond)) { | |
| 143 | + | t.Fatal("repeated reload dispatch was not suppressed") | |
| 144 | + | } | |
| 145 | + | } | |
| 146 | + | ||
| 147 | + | func TestGlobalShortcutHandlerSuppressesRepeatedToggleFullscreenAction(t *testing.T) { | |
| 148 | + | h := &GlobalShortcutHandler{lastDispatchAt: make(map[Action]time.Time)} | |
| 149 | + | now := time.Unix(100, 0) | |
| 150 | + | ||
| 151 | + | if h.suppressRepeatedShortcut(ActionToggleFullscreen, now) { | |
| 152 | + | t.Fatal("first toggle-fullscreen dispatch was suppressed") | |
| 153 | + | } | |
| 154 | + | if !h.suppressRepeatedShortcut(ActionToggleFullscreen, now.Add(time.Millisecond)) { | |
| 155 | + | t.Fatal("repeated toggle-fullscreen dispatch was not suppressed") | |
| 156 | + | } | |
| 157 | + | } | |
| 158 | + | ||
| 159 | + | func TestGlobalShortcutHandlerSuppressesRepeatedToggleHistorySystemViewAction(t *testing.T) { | |
| 160 | + | h := &GlobalShortcutHandler{lastDispatchAt: make(map[Action]time.Time)} | |
| 161 | + | now := time.Unix(100, 0) | |
| 162 | + | ||
| 163 | + | if h.suppressRepeatedShortcut(ActionToggleHistorySystemView, now) { | |
| 164 | + | t.Fatal("first toggle-history-systemview dispatch was suppressed") | |
| 165 | + | } | |
| 166 | + | if !h.suppressRepeatedShortcut(ActionToggleHistorySystemView, now.Add(time.Millisecond)) { | |
| 167 | + | t.Fatal("repeated toggle-history-systemview dispatch was not suppressed") | |
| 168 | + | } | |
| 169 | + | } | |
| 170 | + | ||
| 171 | + | func TestGlobalShortcutHandlerSuppressesRepeatedAdditionalOneShotUIActions(t *testing.T) { | |
| 172 | + | actions := []Action{ | |
| 173 | + | ActionHardReload, | |
| 174 | + | ActionOpenOmnibox, | |
| 175 | + | ActionOpenFind, | |
| 176 | + | ActionOpenDevTools, | |
| 177 | + | ActionToggleFloatingPane, | |
| 178 | + | ActionToggleFavoritesSystemView, | |
| 179 | + | ActionToggleConfigSystemView, | |
| 180 | + | ActionCopyURL, | |
| 181 | + | ActionConsumeOrExpelLeft, | |
| 182 | + | ActionConsumeOrExpelRight, | |
| 183 | + | ActionConsumeOrExpelUp, | |
| 184 | + | ActionConsumeOrExpelDown, | |
| 185 | + | } | |
| 186 | + | ||
| 187 | + | for _, action := range actions { | |
| 188 | + | t.Run(string(action), func(t *testing.T) { | |
| 189 | + | h := &GlobalShortcutHandler{lastDispatchAt: make(map[Action]time.Time)} | |
| 190 | + | now := time.Unix(100, 0) | |
| 191 | + | ||
| 192 | + | if h.suppressRepeatedShortcut(action, now) { | |
| 193 | + | t.Fatalf("first %s dispatch was suppressed", action) | |
| 194 | + | } | |
| 195 | + | if !h.suppressRepeatedShortcut(action, now.Add(time.Millisecond)) { | |
| 196 | + | t.Fatalf("repeated %s dispatch was not suppressed", action) | |
| 197 | + | } | |
| 198 | + | }) | |
| 199 | + | } | |
| 200 | + | } | |
| 201 | + | ||
| 202 | + | func TestGlobalShortcutHandlerSuppressesRepeatedFloatingProfileAction(t *testing.T) { | |
| 203 | + | h := &GlobalShortcutHandler{lastDispatchAt: make(map[Action]time.Time)} | |
| 204 | + | now := time.Unix(100, 0) | |
| 205 | + | action := NewFloatingProfileAction("work", "https://example.com") | |
| 206 | + | ||
| 207 | + | if h.suppressRepeatedShortcut(action, now) { | |
| 208 | + | t.Fatal("first floating-profile dispatch was suppressed") | |
| 209 | + | } | |
| 210 | + | if !h.suppressRepeatedShortcut(action, now.Add(time.Millisecond)) { | |
| 211 | + | t.Fatal("repeated floating-profile dispatch was not suppressed") | |
| 212 | + | } | |
| 213 | + | } | |
| 214 | + | ||
| 123 | 215 | func TestGlobalShortcutHandlerSuppressesActionSwitchLastTab(t *testing.T) { | |
| 124 | 216 | if !isRepeatedGlobalShortcutSuppressed(ActionSwitchLastTab) { | |
| 125 | 217 | t.Fatal("ActionSwitchLastTab should be suppressed") | |
A internal/ui/native_popup_window.go +147 -0
| @@ -0,0 +1,147 @@ | |||
| 1 | + | package ui | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "fmt" | |
| 6 | + | "sync" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/dumber/internal/application/port" | |
| 9 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 10 | + | "github.com/bnema/dumber/internal/logging" | |
| 11 | + | "github.com/bnema/dumber/internal/ui/coordinator/content" | |
| 12 | + | "github.com/bnema/dumber/internal/ui/window" | |
| 13 | + | "github.com/bnema/puregotk/v4/gtk" | |
| 14 | + | ) | |
| 15 | + | ||
| 16 | + | type nativePopupWindow struct { | |
| 17 | + | popupID port.WebViewID | |
| 18 | + | parentPaneID entity.PaneID | |
| 19 | + | parentWindowID string | |
| 20 | + | popupWindow *window.PopupWindow | |
| 21 | + | webView port.WebView | |
| 22 | + | closeOnce sync.Once | |
| 23 | + | } | |
| 24 | + | ||
| 25 | + | func (a *App) ensureNativePopupWindows() { | |
| 26 | + | if a.nativePopupWindows == nil { | |
| 27 | + | a.nativePopupWindows = make(map[port.WebViewID]*nativePopupWindow) | |
| 28 | + | } | |
| 29 | + | } | |
| 30 | + | ||
| 31 | + | func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePopupInput) error { | |
| 32 | + | if a == nil || a.gtkApp == nil { | |
| 33 | + | return fmt.Errorf("gtk application not available for native popup") | |
| 34 | + | } | |
| 35 | + | if input.PopupWebView == nil { | |
| 36 | + | return fmt.Errorf("native popup webview is nil") | |
| 37 | + | } | |
| 38 | + | ||
| 39 | + | popupShell, err := window.NewPopup(ctx, a.gtkApp) | |
| 40 | + | if err != nil { | |
| 41 | + | return err | |
| 42 | + | } | |
| 43 | + | widget := a.contentCoord.WrapWidget(ctx, input.PopupWebView) | |
| 44 | + | if widget == nil || widget.GtkWidget() == nil { | |
| 45 | + | popupShell.Destroy() | |
| 46 | + | return fmt.Errorf("failed to wrap native popup webview widget") | |
| 47 | + | } | |
| 48 | + | popupShell.SetContent(widget.GtkWidget()) | |
| 49 | + | ||
| 50 | + | parentWindowID := "" | |
| 51 | + | if bw := a.browserWindowForAnyPane(input.ParentPaneID); bw != nil { | |
| 52 | + | parentWindowID = bw.id | |
| 53 | + | } | |
| 54 | + | ||
| 55 | + | popupID := input.PopupWebView.ID() | |
| 56 | + | state := &nativePopupWindow{ | |
| 57 | + | popupID: popupID, | |
| 58 | + | parentPaneID: input.ParentPaneID, | |
| 59 | + | parentWindowID: parentWindowID, | |
| 60 | + | popupWindow: popupShell, | |
| 61 | + | webView: input.PopupWebView, | |
| 62 | + | } | |
| 63 | + | popupShell.SetTitle("Dumber") | |
| 64 | + | ||
| 65 | + | a.ensureNativePopupWindows() | |
| 66 | + | a.nativePopupWindows[popupID] = state | |
| 67 | + | ||
| 68 | + | if popupShell.Window() != nil { | |
| 69 | + | closeRequestCb := func(_ gtk.Window) bool { | |
| 70 | + | a.releaseNativePopupWindow(popupID, false, false) | |
| 71 | + | return false | |
| 72 | + | } | |
| 73 | + | popupShell.Window().ConnectCloseRequest(&closeRequestCb) | |
| 74 | + | } | |
| 75 | + | ||
| 76 | + | if aborter, ok := input.PopupWebView.(port.NativePopupHostAbortCapable); ok { | |
| 77 | + | aborter.SetNativePopupHostAbort(func() { | |
| 78 | + | a.dispatchOnMainThread(func() { | |
| 79 | + | a.releaseNativePopupWindow(popupID, true, false) | |
| 80 | + | }) | |
| 81 | + | }) | |
| 82 | + | } | |
| 83 | + | if lifecycle, ok := input.PopupWebView.(port.PopupLifecycleCapable); ok { | |
| 84 | + | lifecycle.SetOnReadyToShow(func() { | |
| 85 | + | a.dispatchOnMainThread(func() { | |
| 86 | + | a.showNativePopupWindow(popupID) | |
| 87 | + | }) | |
| 88 | + | }) | |
| 89 | + | lifecycle.SetOnClose(func() { | |
| 90 | + | a.dispatchOnMainThread(func() { | |
| 91 | + | a.releaseNativePopupWindow(popupID, true, false) | |
| 92 | + | }) | |
| 93 | + | }) | |
| 94 | + | } else { | |
| 95 | + | popupShell.Show() | |
| 96 | + | } | |
| 97 | + | if oauthWV, ok := input.PopupWebView.(port.OAuthCallbackCapable); ok { | |
| 98 | + | oauthWV.AddCloseCallback(func() { | |
| 99 | + | a.dispatchOnMainThread(func() { | |
| 100 | + | a.releaseNativePopupWindow(popupID, true, false) | |
| 101 | + | }) | |
| 102 | + | }) | |
| 103 | + | } | |
| 104 | + | if a.contentCoord != nil && input.ObserveOAuthAutoClose { | |
| 105 | + | a.contentCoord.ObserveNativePopupAuth(ctx, input) | |
| 106 | + | } | |
| 107 | + | ||
| 108 | + | logging.FromContext(ctx).Info(). | |
| 109 | + | Uint64("popup_id", uint64(popupID)). | |
| 110 | + | Str("target_uri", input.TargetURI). | |
| 111 | + | Str("parent_pane_id", string(input.ParentPaneID)). | |
| 112 | + | Msg("native popup host created") | |
| 113 | + | return nil | |
| 114 | + | } | |
| 115 | + | ||
| 116 | + | func (a *App) showNativePopupWindow(popupID port.WebViewID) { | |
| 117 | + | if a == nil || a.nativePopupWindows == nil { | |
| 118 | + | return | |
| 119 | + | } | |
| 120 | + | if state := a.nativePopupWindows[popupID]; state != nil && state.popupWindow != nil { | |
| 121 | + | state.popupWindow.Show() | |
| 122 | + | } | |
| 123 | + | } | |
| 124 | + | ||
| 125 | + | func (a *App) releaseNativePopupWindow(popupID port.WebViewID, closeWindow, destroyWindow bool) { | |
| 126 | + | if a == nil || a.nativePopupWindows == nil { | |
| 127 | + | return | |
| 128 | + | } | |
| 129 | + | state := a.nativePopupWindows[popupID] | |
| 130 | + | if state == nil { | |
| 131 | + | return | |
| 132 | + | } | |
| 133 | + | state.closeOnce.Do(func() { | |
| 134 | + | delete(a.nativePopupWindows, popupID) | |
| 135 | + | if state.webView != nil && !state.webView.IsDestroyed() { | |
| 136 | + | state.webView.Destroy() | |
| 137 | + | } | |
| 138 | + | if state.popupWindow != nil { | |
| 139 | + | switch { | |
| 140 | + | case destroyWindow: | |
| 141 | + | state.popupWindow.Destroy() | |
| 142 | + | case closeWindow: | |
| 143 | + | state.popupWindow.Close() | |
| 144 | + | } | |
| 145 | + | } | |
| 146 | + | }) | |
| 147 | + | } | |
A internal/ui/native_popup_window_test.go +26 -0
| @@ -0,0 +1,26 @@ | |||
| 1 | + | package ui | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/dumber/internal/application/port" | |
| 7 | + | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | |
| 8 | + | "github.com/stretchr/testify/assert" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | func TestReleaseNativePopupWindow_DestroysWebViewAndRemovesState(t *testing.T) { | |
| 12 | + | wv := portmocks.NewMockWebView(t) | |
| 13 | + | wv.EXPECT().IsDestroyed().Return(false).Once() | |
| 14 | + | wv.EXPECT().Destroy().Once() | |
| 15 | + | ||
| 16 | + | app := &App{nativePopupWindows: map[port.WebViewID]*nativePopupWindow{ | |
| 17 | + | port.WebViewID(1): { | |
| 18 | + | popupID: port.WebViewID(1), | |
| 19 | + | webView: wv, | |
| 20 | + | }, | |
| 21 | + | }} | |
| 22 | + | ||
| 23 | + | app.releaseNativePopupWindow(port.WebViewID(1), false, false) | |
| 24 | + | _, ok := app.nativePopupWindows[port.WebViewID(1)] | |
| 25 | + | assert.False(t, ok) | |
| 26 | + | } | |
A internal/ui/window/popup_window.go +93 -0
| @@ -0,0 +1,93 @@ | |||
| 1 | + | package window | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/dumber/internal/logging" | |
| 7 | + | "github.com/bnema/puregotk/v4/gtk" | |
| 8 | + | "github.com/rs/zerolog" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | const ( | |
| 12 | + | popupWindowDefaultWidth = 900 | |
| 13 | + | popupWindowDefaultHeight = 800 | |
| 14 | + | ) | |
| 15 | + | ||
| 16 | + | // PopupWindow is a lightweight top-level GTK host for native-required popup flows. | |
| 17 | + | type PopupWindow struct { | |
| 18 | + | window *gtk.ApplicationWindow | |
| 19 | + | content *gtk.Box | |
| 20 | + | logger zerolog.Logger | |
| 21 | + | } | |
| 22 | + | ||
| 23 | + | func NewPopup(ctx context.Context, app *gtk.Application) (*PopupWindow, error) { | |
| 24 | + | log := logging.FromContext(ctx) | |
| 25 | + | pw := &PopupWindow{logger: log.With().Str("component", "popup-window").Logger()} | |
| 26 | + | ||
| 27 | + | pw.window = gtk.NewApplicationWindow(app) | |
| 28 | + | if pw.window == nil { | |
| 29 | + | return nil, ErrWindowCreationFailed | |
| 30 | + | } | |
| 31 | + | pw.window.SetDefaultSize(popupWindowDefaultWidth, popupWindowDefaultHeight) | |
| 32 | + | title := windowTitle | |
| 33 | + | pw.window.SetTitle(&title) | |
| 34 | + | ||
| 35 | + | pw.content = gtk.NewBox(gtk.OrientationVerticalValue, 0) | |
| 36 | + | if pw.content == nil { | |
| 37 | + | pw.window.Unref() | |
| 38 | + | return nil, ErrWidgetCreationFailed("popupWindow.content") | |
| 39 | + | } | |
| 40 | + | pw.content.SetHexpand(true) | |
| 41 | + | pw.content.SetVexpand(true) | |
| 42 | + | pw.content.SetVisible(true) | |
| 43 | + | pw.window.SetChild(&pw.content.Widget) | |
| 44 | + | return pw, nil | |
| 45 | + | } | |
| 46 | + | ||
| 47 | + | func (pw *PopupWindow) SetContent(widget *gtk.Widget) { | |
| 48 | + | if pw == nil || pw.content == nil { | |
| 49 | + | return | |
| 50 | + | } | |
| 51 | + | if widget == nil { | |
| 52 | + | return | |
| 53 | + | } | |
| 54 | + | widget.SetVisible(true) | |
| 55 | + | pw.content.Append(widget) | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | func (pw *PopupWindow) Show() { | |
| 59 | + | if pw == nil || pw.window == nil { | |
| 60 | + | return | |
| 61 | + | } | |
| 62 | + | pw.window.Present() | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | func (pw *PopupWindow) Close() { | |
| 66 | + | if pw == nil || pw.window == nil { | |
| 67 | + | return | |
| 68 | + | } | |
| 69 | + | pw.window.Close() | |
| 70 | + | } | |
| 71 | + | ||
| 72 | + | func (pw *PopupWindow) Destroy() { | |
| 73 | + | if pw == nil || pw.window == nil { | |
| 74 | + | return | |
| 75 | + | } | |
| 76 | + | pw.window.Destroy() | |
| 77 | + | pw.window = nil | |
| 78 | + | pw.content = nil | |
| 79 | + | } | |
| 80 | + | ||
| 81 | + | func (pw *PopupWindow) Window() *gtk.ApplicationWindow { | |
| 82 | + | if pw == nil { | |
| 83 | + | return nil | |
| 84 | + | } | |
| 85 | + | return pw.window | |
| 86 | + | } | |
| 87 | + | ||
| 88 | + | func (pw *PopupWindow) SetTitle(title string) { | |
| 89 | + | if pw == nil || pw.window == nil { | |
| 90 | + | return | |
| 91 | + | } | |
| 92 | + | pw.window.SetTitle(&title) | |
| 93 | + | } | |