[2026-06-26] eagle: work/tech-design/data-url-new-tab-frame-load-interrupted.md
This commit is contained in:
Alexey Martemyanov
@@ -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()`, `<a target=_blank>`), 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,<h1>Hello</h1>', '_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,<h1>Hello</h1>', '', '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.
|
||||
Reference in New Issue
Block a user