fix(boot): sequence openCurrentPage after restoreSession to avoid duplicate windows#218
Merged
Merged
Conversation
…licate windows `restoreSession` and `openCurrentPage` were fired concurrently at the end of `init()`. `manager.open()`'s dedupe-by-baseId check happens before `createWindow()` awaits the lazy `window-system[.min].js` bundle, so two concurrent opens for the same window (e.g. portal intent + saved Dashboard) both pass the check and construct separate Window instances. The user sees two copies of the same window; the second one's iframe never finishes because the chromeless bridge only handshakes with one instance per id. Sequence them: openCurrentPage chains off the same promise restore runs on. The existing comment on case 2 already says "the page they asked for opens on top of the restored stack" — that's what sequencing actually delivers. If the page is in the restored stack, `manager.open()` finds and focuses it; otherwise it opens fresh on top.
✅ WordPress Plugin Check Report
📊 ReportAll checks passed! No errors or warnings found. 🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check |
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.
Summary
Reported by Roberto: in Incognito with a portal-intent URL (
?desktop_mode_portal=1&desktop_mode_portal_intent=1) and a saved session, two Dashboard windows opened; the second one's spinner never resolved.Root cause is a boot-time race in
src/desktop.ts:restoreSessionandopenCurrentPagewere fired concurrently (bothvoid …).manager.open()'s dedupe-by-baseId check (src/window-manager/index.ts:406) runs synchronously, beforecreateWindow()awaitsensureWindowSystemLoaded. So both calls pass the existing-check while the first window is still mid-await on the lazy bundle.id/baseId. The chromeless bridge handshakes by id and only finishes one of them; the other hangs on the spinner forever.Fix
Sequence them.
openCurrentPagechains off the same promiserestoreSessionresolves. If the page the user asked for is already in the restored stack,manager.open()finds and focuses it; otherwise a fresh window opens on top — exactly what the existing case-2 comment already promised ("the page they asked for opens on top of the restored stack").No new manager-level state. The change is one block in
init().Test plan
…/wp-admin/index.php?desktop_mode_portal=1&desktop_mode_portal_intent=1with Dashboard in the saved session → exactly one Dashboard window opens, spinner resolves.fromPortal=false) → window opens normally./desktop-mode/visit (fromPortal=true,fromPortalIntent=false, session exists) → session restores, no extra window.npm run lint,tsc --noEmit,npm run test:js(1214 tests) all pass.