fix(background): treat about:blank as an unsettled tab url in the attach probe - #1530
Merged
Merged
Conversation
…ach probe On Firefox a freshly created approval window's tab reports about:blank until the navigation commits (there is no pendingUrl there), so the attach-time ownership probe raced the popup's cold load, read the window as foreign and cancelled the request it was about to display — deleting the signing payload and leaving the page on its skeleton (WALLET-1439). Chrome was unaffected: it reports the same gap as url '' plus pendingUrl, which the guard already covered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
On Firefox the sign approval window opens but never renders the request — the page sits on its skeleton and the dapp waits forever (WALLET-1439). Chrome is unaffected.
The cause is the attach-time ownership probe in
attachWindowToRequest. Oncewindows.createresolves, the probe reads the new window's first tab and repairs the attach — i.e. cancels the request — when the tab's URL provably is not an extension page. In front of that sits a guard whose whole job is to let inconclusive shapes pass untouched, because repairing on one cancels an approval that is on screen. That guard only knew Chrome's unsettled shape: a navigating tab reported asurl: ''with the real target inpendingUrl.Firefox has no
pendingUrlat all, and reports a freshly created window's tab asabout:blankuntil the navigation commits. On a cold popup load that commit loses the race with the probe, soabout:blankfell through to!tabUrl.startsWith(runtime.getURL('')), the wallet's own window was read as foreign, and the request it was about to display was cancelled. Cancelling deletes the stored signing payload — so the popup already on screen had nothing left to render, which is the stuck skeleton in the report.What changed
about:blankjoinsnulland''as an unsettled tab URL in the probe's guard, and the comment there now enumerates the shapes per browser instead of describing Chrome's as if it were universal.{ tabs: [{ url: 'about:blank' }] }) asserting the probe does not cancel.This only widens the inconclusive set, not the accept set — the file's existing asymmetry (a provable verdict repairs, anything inconclusive does not) is what makes that safe. Detection is not weakened: a window that genuinely is not ours settles on a non-extension URL, and the foreign-window case that this arm exists for is still caught.
about:blankwas never a verdict in the first place; it was being treated as one.Verification
npm run ci-check— format:check, lint, tsc, knip and the full jest suite with coverage, all green (run by thepre-pushhook).agent-browserharness does not drive — the Chrome path this fix does not touch is the one it could exercise.Linked tickets
WALLET-1439
Checklist
Make sure this PR title follows semantic release conventions: https://semantic-release.gitbook.io/semantic-release/#commit-message-format
If the PR adds any new text to the UI, make sure they are localized — no UI text added
Include a screenshot or recording if implementing significant UI or user flow change — background-only, no UI change
When this PR affects architecture changes wait for review from Dmytro before merging