From 6e8a954e5ff89b43c21cec294f22d5ec990b2f21 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Fri, 26 Jun 2026 14:24:24 +0600 Subject: [PATCH] [2026-06-26] eagle: work/tech-design/data-url-new-tab-frame-load-interrupted.md --- ...data-url-new-tab-frame-load-interrupted.md | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 work/tech-design/data-url-new-tab-frame-load-interrupted.md diff --git a/work/tech-design/data-url-new-tab-frame-load-interrupted.md b/work/tech-design/data-url-new-tab-frame-load-interrupted.md new file mode 100644 index 00000000..52c95ffb --- /dev/null +++ b/work/tech-design/data-url-new-tab-frame-load-interrupted.md @@ -0,0 +1,164 @@ +--- +title: 'Tech Design: data: URL New-Tab Race Condition and Popup Blocking' +created: '2026-06-26' +tags: + - tech-design + - apple + - macos + - navigation + - webkit + - data-url + - popup + - tab +status: draft +--- +**Author:** Apple team +**Project:** macOS browser — Tab navigation / Popup handling + +--- + +## Background & Requirements + +The DuckDuckGo macOS browser uses `WKWebView` for page rendering. All navigations flow through `DistributedNavigationDelegate` (BrowserServicesKit). Tabs are modelled by `Tab.swift` which owns a `@Published var content: TabContent` property that drives what is displayed in the address bar and tab strip. + +### WebKit security restriction: `data:` URLs in main-frame navigations + +WebKit deliberately blocks `data:` URLs from loading as a main-frame (top-level) document unless the load was initiated programmatically by the embedding app (`isRequestFromClientOrUserInput = true`). This is enforced in `DocumentLoader::disallowDataRequest()` (WebCore): + +```cpp +// WebCore/loader/DocumentLoader.cpp +bool DocumentLoader::disallowDataRequest() const { + if (!m_response.url().protocolIsData()) + return false; + // Allow if: not main frame, OR app-initiated, OR settings override + if (!frame()->isMainFrame() || allowsDataURLsForMainFrame() + || frame()->settings().allowTopNavigationToDataURLs()) + return false; + // Block: add console error and return true + return true; +} +``` + +Where `allowsDataURLsForMainFrame()` returns `m_isRequestFromClientOrUserInput`. For navigations originating from a web page (link click, `window.open()`, ``), this flag is `false`, so `data:` URL top-level navigations are always blocked. + +When blocked, `DocumentLoader::stopLoadingForPolicyChange()` calls `cancelMainResourceLoad(interruptedForPolicyChangeError())`, which produces a `WKError` with domain `WebKitErrorDomain`, code `WebKitErrorFrameLoadInterruptedByPolicyChange` — the "Frame load interrupted" error that surfaces in `webView(_:didFailProvisionalNavigation:withError:)` and then `Tab.navigation(_:didFailWith:)`. + +### Two paths for web-page-opened windows + +When a web page calls `window.open(dataURL)` or navigates a link with `target=_blank`, the browser must decide what to do: + +| Path | Mechanism | When used | +|------|-----------|----------| +| **Popup window** | `WKUIDelegate.webView(_:createWebViewWith:for:windowFeatures:)` returns a new `WKWebView` | `window.open()` with width/height set (or any dimensions in `WKWindowFeatures`) | +| **New tab** | Same delegate called; app returns `nil` and opens a new tab independently | `window.open()` / `target=_blank` without dimensions | + +In both cases WebKit ends up calling `continueLoadAfterNewWindowPolicy` in the web process, creating a new frame, and loading the `data:` URL into it — which triggers the `disallowDataRequest()` block and the `interruptedForPolicyChangeError` in both scenarios. + +--- + +## Problem Statement + +Two independent bugs: + +**Bug 1 — New tab: stale `data:` URL displayed in address bar after interrupted load.** +When a `data:` URL is opened in a new tab, `Tab.navigation(_:didFailWith:)` receives `isFrameLoadInterrupted = true` and previously returned early *without* synchronising `Tab.content` with `webView.url`. Because new-tab display is deferred by the tab-creation flow, the `webView.url` reset (`nil` after the interrupted load) propagated to `Tab.content` *after* the tab was first shown. This caused a race that left the address bar showing the `data:` URL even though the navigation had been cancelled. + +**Bug 2 — Popup: `data:` URLs could still be opened.** +For popup windows (with dimensions), `createChildWebView` returned a `WKWebView` to WebKit. The `data:` load was interrupted by WebKit and `interruptedForPolicyChangeError` was delivered, but because the popup WebView was displayed immediately on creation, `Tab.content` was already populated with the `data:` URL before the error arrived. The popup appeared to "open" (empty/blank page) and the address bar showed the `data:` URL. Additionally, `PopupHandlingTabExtension.createChildWebView` blocked `javascript:` URLs but not `data:` URLs. + +--- + +## Recommended Approach + +### Fix 1 — `Tab.navigation(_:didFailWith:)`: call `handleUrlDidChange()` on `isFrameLoadInterrupted` + +In `Tab.swift`, the `navigation(_:didFailWith:)` handler has an early-exit guard for errors that indicate the navigation was intentionally cancelled (user stop or policy interruption). Before the fix: + +```swift +guard !error.isNavigationCancelled, + !error.isFrameLoadInterrupted else { + return // ← early return, Tab.content not updated +} +``` + +After: + +```swift +guard !error.isNavigationCancelled, + !error.isFrameLoadInterrupted /* navigation cancelled by a Navigation Responder or Content Blocker */ else { + // Update tab content to the current URL to avoid race conditions with + // WebView.url publisher that may cause `reloadIfNeeded` to reload the same URL again. + handleUrlDidChange() + return +} +``` + +`handleUrlDidChange()` reads `webView.url` synchronously and sets `self.content` accordingly. Since `webView.url` is `nil` (or the previous page's URL) after an interrupted load, this immediately corrects `Tab.content` before any async publisher update can race. + +**Why this is safe for other callers of the guard:** +- `isNavigationCancelled` (user pressed Stop): `webView.url` already holds the previous committed URL. `handleUrlDidChange()` will either no-op (content already matches) or restore the correct content. +- `isFrameLoadInterrupted` (policy block, e.g. content blocker, `data:` URL): same reasoning — `webView.url` reflects the last committed URL, not the blocked candidate. + +### Fix 2 — `PopupHandlingTabExtension.createChildWebView`: block `data:` URLs + +Adding `data:` to the list of blocked navigational schemes alongside `javascript:`: + +```swift +// Before +guard navigationAction.request.url?.navigationalScheme != .javascript else { return nil } + +// After +guard ![.javascript, .data].contains(navigationAction.request.url?.navigationalScheme) else { return nil } +``` + +This prevents `createChildWebView` from returning a `WKWebView` to WebKit at all for `data:` URLs. WebKit interprets a `nil` return from `createWebViewWith` as "don't create a new window", so the navigation is silently dropped before a new tab or popup is ever presented to the user. No `interruptedForPolicyChangeError` reaches `Tab.navigation(_:didFailWith:)` at all. + +### Implementation steps + +1. `macOS/DuckDuckGo/Tab/Model/Tab.swift` — in `navigation(_:didFailWith:)`, add `handleUrlDidChange()` call before the `return` in the `isFrameLoadInterrupted` guard branch. ✅ Done. +2. `macOS/DuckDuckGo/Tab/TabExtensions/PopupHandlingTabExtension.swift` — extend the navigational-scheme guard to include `.data`. ✅ Done. + +--- + +## Notes + +[1] The root `disallowDataRequest()` block in WebKit is intentional and correct — it prevents phishing via `data:` URL spoofing. We must not work around it. Our fixes address the *symptom* (incorrect `Tab.content` state and incomplete blocking) rather than trying to make `data:` loads succeed. + +[2] `handleUrlDidChange()` is already called from the `webView.publisher(for: \.url)` observation path (Tab.swift ~line 1215). Adding it inside `navigation(_:didFailWith:)` makes the same update happen eagerly, eliminating the race between the Combine publisher delivery and the `reloadIfNeeded` pass. + +[3] The popup-window race (Bug 2) exists because popup tabs are displayed synchronously at WebView-creation time (before any navigation completes), while new tabs are deferred until the navigation policy decision resolves. Fix 1 closes the race for both paths; Fix 2 eliminates the root cause for popup windows specifically. + +[4] The `javascript:` URL guard in `createChildWebView` predates this change. `data:` should always have been included alongside it — both schemes are opaque, both produce navigation interruptions when loaded as main-frame top-level documents, and neither is a meaningful URL to display in a new tab opened by web content. + +--- + +## Testing + +- **Manual regression (macOS):** + - Open a page that calls `window.open('data:text/html,

Hello

', '_blank')` (no dimensions). Verify no new tab appears and no stale `data:` URL is shown in any existing tab's address bar. + - Same test with a popup: `window.open('data:text/html,

Hello

', '', 'width=400,height=300')`. Verify no popup window is created. + - Open a page that calls `window.open('javascript:void(0)', '_blank')`. Verify unchanged — still blocked. + - Normal `window.open('https://example.com', '_blank')` continues to open in a new tab. + +- **Existing test coverage:** `PopupHandlingTabExtensionTests` should cover the `javascript:` scheme guard; extend to cover `data:` scheme. + +- **Tab content state test:** verify that after a navigation to a `data:` URL is interrupted, `Tab.content` reflects the previous committed URL (or `.none`) rather than the blocked `data:` URL. + +--- + +## Additional Considerations + +### Privacy +- No user data is collected or transmitted. Both changes are local, synchronous, and in-process. +- Blocking `data:` URLs from being opened in new windows is consistent with WebKit's own security model. + +### Security +- `data:` URLs opened by web content as top-level navigations are a known phishing vector (they can impersonate login pages without any domain). The existing WebKit block (`disallowDataRequest`) already prevents the load; these fixes ensure our app-level state machine does not misrepresent that a load succeeded. +- `javascript:` URLs are similarly dangerous as top-level navigations; they are already blocked. + +### Site Breakage +- Legitimate content that opens popup windows or new tabs with `data:` URLs as the primary content does not exist in practice. The change only affects URLs whose navigational scheme is `data:` — HTTP/HTTPS, `blob:`, and all other schemes are unaffected. +- The `handleUrlDidChange()` addition is a no-op when `webView.url` already matches `Tab.content`; it cannot cause a double-reload. + +### Operational +- No infrastructure, persistence, or network changes. Fix is entirely in the tab management layer.