Repository navigation
fix(popup): keep hand-off values, report departures, expose connectionId - #79
Merged
Merged
Conversation
This was referenced Oct 2, 2026
🚀 Deploying Preview to Cloudflare 🚀Preview URL: https://fix-popup-fixes.previews.lib.id, https://fix-popup-fixes-libid.grounded-systems.workers.dev (commit fe9c561)This URL reflects your latest Preview deploymentPreview Deployments by commit
|
…ture A started port cannot be stopped: clearing its handler before the Service Worker lookup dropped whatever arrived in that gap, and WebKit also drops values it has already taken from the channel when the port is transferred. The departing document now collects what the port still dispatches until the transfer and hands it to the worker as the port's backlog; the destination delivers it before the port's own queue, so a value or a close sent right after navigate reaches the destination. Also: - a messageerror fails the connection as decode-rejected instead of silently closing the carrier; - a connected navigate to the popup's own document is rejected rather than stranding the port with the worker; - every carrier the application may already hold reports departure before the popup gives it up: an unadmitted restored or handshaken carrier, a malformed claim answer (claims no longer leak a valid port through Promise.all), and closure before the worker takes the port; - on and send reject types routing would reject; - the application's ready rejects with connection-closed when it closes before any carrier; - one keeper reply deadline covers both registration waits. Assisted-by: Claude Opus 5.5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
- qualify every fallback as the fallback constructor or carrier or the isolation fallback; resolveIsolationFallback; native-anchor binding; - leaving, routes and depart replace controlsDone, registrations and the four departure builders; retire only means dropping a carrier locally; - opener-timeout is a diagnostic only, operations on an ended endpoint throw connection-closed, and the main entry stops exporting the navigation-carrier hooks, ConnectionVersion and the unused count; - docs match the code: ignored versus rejected handshakes, the unstarted echo, worker validation, departure senders, the unimplemented WebRTC carrier, pending handshakes, and the keeper API sketch; - the testing double follows the real connection's contract; tests move to their modules, carry the right tags, and assert what they claim. Assisted-by: Claude Opus 5.5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
client.new took the ceremony ID beside a connection that already has one. It checked the two matched only for connections client.connect made, through a WeakMap, so a mismatched ID on any other connection never completed the handshake instead of failing at once. Connections now expose their validated connectionId, and client.new uses it as the ceremony ID; the separate parameter and the WeakMap go. BREAKING CHANGE: client.new(conn, id, platformId, ...) becomes client.new(conn, platformId, ...). Assisted-by: Claude Opus 5.5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
Callers generated a UUID only to hand it to client.connect, which made freshness their job. client.connect now generates it, as the carried protocol (REQ-POPUP-ID-02), and a JavaScript caller cannot supply one. client.new keeps running the ceremony under the connection's ID, so an application can still connect a popup itself, show its own pages, and pass that connection in. BREAKING CHANGE: client.connect takes no connectionId; read it from the returned connection. Assisted-by: Claude Opus 5.5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
An anchor launch made every caller invent a window name, mirror it on the anchor, set the anchor's href, prevent its navigation only when the scripted window opened, and keep rel=noopener off unchecked. PopupWindow.fromAnchor(event, features?) does all of it: it names the popup, tries the scripted window first, and suppresses the anchor only when that opened. While a blocked popup awaits binding, a navigate during the activation points the anchor at its destination, so the anchor needs no target or href. An activation without an opener-keeping anchor is refused and navigates nowhere. PopupWindow.open(features?) names its window too. BREAKING CHANGE: PopupWindow.open(target, features?) becomes open(features?); anchor launches use fromAnchor. Assisted-by: Claude Opus 5.5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
The backlog was replayed from a local copy, so a delivered value that made the document leave handed on an empty backlog and dropped the rest, a queued close included. Values now leave the backlog one at a time. An explicit popup-side close() during a hand-off discarded only the selected carrier, which is null while the port is in transit, so the application never learned of the departure. depart() now reports it over the in-transit port as well. Signed-off-by: Wondertan <hlibwondertan@gmail.com> Assisted-by: Claude Opus 5.5
Link types are ASCII case-insensitive, but relList.contains is not, so rel="NoOpEnEr" passed the check and opened an openerless popup that could not connect. A noopener or noreferrer in features threw without suppressing the anchor, so the application page followed it. Every refusal now suppresses the navigation. The docs advise keeping a placeholder href, which keeps the anchor focusable and keyboard-activatable. Signed-off-by: Wondertan <hlibwondertan@gmail.com> Assisted-by: Claude Opus 5.5
The application page navigated to the anchor's own href, so a blocked activation connected whether or not navigate pointed the anchor. The destination now comes from the spec, and the static href carries no connection. The noopener refusal uses a mixed-case token, and the open helper drops its unused rel option. Signed-off-by: Wondertan <hlibwondertan@gmail.com> Assisted-by: Claude Opus 5.5
The transport spec now allows the hand-off backlog in a continuity record, a departure report while continuity preparation is pending, and pointing the activating anchor during pending native-anchor binding. The test plans and docs match: the keeper holds undelivered values, navigate points a blocked anchor, a messageerror fails the connection through decoding, and client.connect/new take the connection ID from the connection. Signed-off-by: Wondertan <hlibwondertan@gmail.com> Assisted-by: Claude Opus 5.5
Wondertan
force-pushed
the
fix/popup-fixes
branch
from
October 2, 2026 14:11
d4ae849 to
fe9c561
Compare
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
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.
Popup fixes from the last review passes, plus the double ceremony ID from xgreenx's review of #28. The expired kept port is tracked separately in #78.
Changes
Hand-offs keep their values (6cead13). A started port cannot be stopped, so values arriving while a document looked up the Service Worker were dropped, and WebKit also dropped values it had already taken when the port was transferred. The departing document now collects what the port still dispatches and hands it to the worker as a backlog, which the destination delivers first. Also:
messageerrorfails the connection asdecode-rejectedinstead of silently closing the carrier;navigateto the popup's own document is rejected instead of stranding the port;onandsendreject unroutable types; the application'sreadyrejects withconnection-closedwhen it closes before any carrier; one keeper reply deadline covers both registration waits.Naming, API surface, docs and tests (c90d073). Every "fallback" says which one; departure helpers are consolidated; the main entry stops exporting internal hooks; docs match the code, including the unimplemented WebRTC carrier; the testing double follows the real connection's contract.
connectionId(416039b, comment).client.newtook the ceremony ID beside a connection that already has one, and checked them only for connectionsclient.connectmade, so a mismatched ID elsewhere never completed the handshake. Connections now expose their validatedconnectionId, andclient.newuses it; the parameter and the WeakMap go.Breaking:
client.new(conn, id, platformId, …)becomesclient.new(conn, platformId, …).The client generates the connection ID (2df78f3). Callers generated a UUID only to hand it to
client.connect, which made freshness their job.client.connectnow generates it, as the carried protocol (REQ-POPUP-ID-02), and ignores one a JavaScript caller passes. An application can still connect a popup itself, for example to show its own pages before and after a ceremony, and pass that connection toclient.new.Breaking:
client.connect(popup, { connectionId, … })becomesclient.connect(popup, { … }); read the ID fromconnection.connectionId.Anchor launches (517b2ec). An anchor launch made every caller invent a window name, mirror it on the anchor, set the anchor's
href, prevent its navigation only when the scripted window opened, and keeprel=noopeneroff unchecked.PopupWindow.fromAnchor(event, features?)does all of it: it names the popup, tries the scripted window first, and suppresses the anchor only when that opened. While a blocked popup awaits binding, anavigateduring the activation points the anchor at its destination, so the anchor needs notarget; keep a placeholderhrefso it stays focusable. An activation without an opener-keeping anchor, or withnoopener/noreferrerin any letter case or infeatures, is refused and navigates nowhere.Breaking:
PopupWindow.open(target, features?)becomesopen(features?), which names its window too; anchor launches usefromAnchor.Review fixes (5073ae5, c2764a6, 166dfc6, fe9c561).
close()during a hand-off now reports departure over the port in transit; before, the application waited forever.rel="NoOpEnEr"passed the check (relList.containsis case-sensitive, link types are not) and opened an openerless popup;noopenerinfeaturesthrew without suppressing the anchor. Both are refused now.hrefdoesn't carry, so it provesnavigatepoints the anchor.specs/popup-transport.mdnow allows the behaviours this PR adds: the backlog in a continuity record (SP-POPUP-03, REQ-POPUP-CONT-06), departure while continuity is pending (REQ-POPUP-CONTROL-08), and pointing the activating anchor while binding is pending (REQ-POPUP-CONTROL-04/04A). Stale test-plan and doc lines match the code.Verification
Rebased onto
mainafter #74, #80 and #13;ceremony/docs/client.mdkeeps #80's origin rules beside theconnectionIdchange.typecheck,test(popup 153, ceremony 994),typecheck:e2e,lint,fmt:check,test:distribution(32 passed, 7 skipped without a native SWS).