From 6a780c74254cfa91f2d34fcb58ebca76133a4e47 Mon Sep 17 00:00:00 2001 From: Kayshen-X Date: Sat, 25 Jul 2026 23:51:27 +0800 Subject: [PATCH] fix(web): pre-open the sign-in popup in the click gesture and use popup wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opening the verification page from the async poll callback was popup- blocked (the 'click sign-in twice' bug); a placeholder window opened synchronously inside the press is navigated once the URL arrives, and closed on cancel/failure. The wasm login modal now says 'popup window' instead of 'your browser' — the web editor already runs in one. --- .github/workflows/nix-check.yml | 4 +- .github/workflows/rust-release.yml | 55 +++++++++++---- README.md | 8 ++- .../op-editor-ui/src/widgets/login_modal.rs | 11 ++- crates/op-host-web/src/web_auth_sync.rs | 67 ++++++++++++++++--- .../src/widget_host/account_press.rs | 17 +++++ crates/op-i18n/src/i18n/en.rs | 2 + crates/op-i18n/src/i18n/zh_cn.rs | 2 + 8 files changed, 136 insertions(+), 30 deletions(-) diff --git a/.github/workflows/nix-check.yml b/.github/workflows/nix-check.yml index 88e8d8944..e69954e71 100644 --- a/.github/workflows/nix-check.yml +++ b/.github/workflows/nix-check.yml @@ -38,10 +38,10 @@ jobs: submodules: recursive - name: Install Nix - uses: DeterminateSystems/nix-installer-action@main + uses: DeterminateSystems/nix-installer-action@ef8a148080ab6020fd15196c2084a2eea5ff2d25 # v22 - name: Enable Nix cache - uses: DeterminateSystems/magic-nix-cache-action@main + uses: DeterminateSystems/magic-nix-cache-action@908b263ff629f4cc17666315b7fd3ec127c6244d # v14 - name: Evaluate flake checks run: nix flake check --no-build --show-trace diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 01ca133e9..17702df9a 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -85,6 +85,7 @@ on: permissions: contents: write packages: write + pull-requests: write jobs: version: @@ -842,6 +843,8 @@ jobs: TAP_GITHUB_TOKEN: ${{ secrets.TAP_GITHUB_TOKEN }} steps: - uses: actions/checkout@v4 + with: + fetch-depth: 0 - uses: actions/download-artifact@v4 with: path: dist @@ -967,9 +970,8 @@ jobs: name: ${{ steps.body.outputs.title }} body_path: release-body.md files: release-files/* - - name: Update latest published Nix release manifest - env: - GH_TOKEN: ${{ github.token }} + - name: Prepare latest published Nix release manifest + id: nix-manifest shell: bash run: | set -euo pipefail @@ -983,18 +985,43 @@ jobs: --arg cliHash "$(sri_hash op-cli-linux-x86_64.tar.gz)" \ '{version: $version, desktopHash: $desktopHash, cliHash: $cliHash}')" - git fetch origin main - git switch --create update-release-manifest origin/main - printf '%s\n' "$manifest" > nix/release-manifest.json - git config user.name github-actions[bot] - git config user.email 41898282+github-actions[bot]@users.noreply.github.com - git add nix/release-manifest.json - if git diff --cached --quiet; then - echo "release manifest already current" - exit 0 + # Tags do not retain their source branch. Prefer the matching release + # branch when it contains the tagged commit, then fall back to the + # repository's default branch. + release_branch="v${version}" + default_branch="${{ github.event.repository.default_branch }}" + git fetch --prune origin \ + "+refs/heads/*:refs/remotes/origin/*" + if git show-ref --verify --quiet "refs/remotes/origin/${release_branch}" \ + && git merge-base --is-ancestor "$GITHUB_SHA" "origin/${release_branch}"; then + target_branch="$release_branch" + elif git merge-base --is-ancestor "$GITHUB_SHA" "origin/${default_branch}"; then + target_branch="$default_branch" + else + echo "::error::tagged commit is not contained in ${release_branch} or ${default_branch}" + exit 1 fi - git commit -m "chore(release): update Nix release manifest for v${version}" - git push origin HEAD:main + + git switch --force-create "$target_branch" "origin/${target_branch}" + printf '%s\n' "$manifest" > nix/release-manifest.json + echo "target-branch=$target_branch" >> "$GITHUB_OUTPUT" + + # The release branches are protected, so publish the generated hash + # update through a reviewable PR instead of trying to push as the + # GitHub Actions bot. Re-running a release updates the same PR branch. + - name: Open Nix release manifest update + uses: peter-evans/create-pull-request@5f6978faf089d4d20b00c7766989d076bb2fc7f1 # v8.1.1 + with: + base: ${{ steps.nix-manifest.outputs.target-branch }} + branch: automation/nix-release-manifest-v${{ env.OP_VERSION }} + delete-branch: true + add-paths: nix/release-manifest.json + commit-message: "chore(release): update Nix release manifest for v${{ env.OP_VERSION }}" + title: "chore(release): update Nix release manifest for v${{ env.OP_VERSION }}" + body: | + Updates the pinned Nix prebuilt release metadata after publishing v${{ env.OP_VERSION }}. + + This PR was generated by the Rust release workflow. package-managers: name: Update Homebrew tap and Scoop bucket diff --git a/README.md b/README.md index 52ac76037..5d8076695 100644 --- a/README.md +++ b/README.md @@ -165,9 +165,11 @@ nix build .#appimage # portable desktop AppImage The flake uses the pinned Rust toolchain from `rust-toolchain.toml` and is currently published for `x86_64-linux`. A Debian package is not produced by the flake yet; use the upstream release artifacts when a `.deb` is required. -The `prebuilt` outputs require the upstream GitHub release for the workspace -version to publish matching Linux archives; otherwise use the source-built -outputs above. +The `prebuilt` outputs use the release version and hashes pinned in +`nix/release-manifest.json`, independently of the workspace source version. +After a release is published, the release workflow opens a PR to refresh that +manifest. Until it is merged, the prebuilt outputs continue to use the prior +published release; the source-built outputs always build the checked-out source. **CLI (`op`):** diff --git a/crates/op-editor-ui/src/widgets/login_modal.rs b/crates/op-editor-ui/src/widgets/login_modal.rs index d4ffb4dfa..dc2d544b7 100644 --- a/crates/op-editor-ui/src/widgets/login_modal.rs +++ b/crates/op-editor-ui/src/widgets/login_modal.rs @@ -310,18 +310,25 @@ fn paint_status_note( ) { use op_editor_core::{LoginFlowError, LoginFlowStatus}; let status = status_rect(panel); + // The web build runs IN a browser: the approval happens in a popup + // window, so "waiting for your browser" would read as nonsense there. + let (waiting_key, approval_key) = if cfg!(target_arch = "wasm32") { + ("account.openingPopup", "account.approveInPopup") + } else { + ("account.waitingForBrowser", "account.waitingForApproval") + }; let (icon, text, background, border, color) = if let Some(flow) = flow_status { match flow { LoginFlowStatus::WaitingBrowser => ( Icon::Globe, - t(locale, "account.waitingForBrowser"), + t(locale, waiting_key), theme.row_selected_primary, theme.primary.with_alpha(0.32), theme.foreground, ), LoginFlowStatus::WaitingApproval => ( Icon::Globe, - t(locale, "account.waitingForApproval"), + t(locale, approval_key), theme.row_selected_primary, theme.primary.with_alpha(0.32), theme.foreground, diff --git a/crates/op-host-web/src/web_auth_sync.rs b/crates/op-host-web/src/web_auth_sync.rs index a27d2b5fa..bd628506d 100644 --- a/crates/op-host-web/src/web_auth_sync.rs +++ b/crates/op-host-web/src/web_auth_sync.rs @@ -132,7 +132,11 @@ fn drain_actions( } })), ); + if action == PendingAuthAction::CancelLogin { + close_login_popup_placeholder(); + } if !ok && action == PendingAuthAction::BeginLogin { + close_login_popup_placeholder(); let mut b = inner.borrow_mut(); let ui = &mut b.host_mut().editor_state_mut().editor_ui; ui.login_modal_status = Some(LoginFlowStatus::Failed(LoginFlowError::Unavailable)); @@ -194,7 +198,7 @@ fn apply_login_status( if let Some(url) = parsed["verification_uri"].as_str() { if !url.is_empty() { browser_opened.set(true); - open_popup(url); + navigate_login_popup(url); } } } @@ -217,6 +221,7 @@ fn apply_login_status( ui.login_modal_hover = None; } "error" => { + close_login_popup_placeholder(); ui.login_modal_status = Some(LoginFlowStatus::Failed( match parsed["code"].as_str().unwrap_or_default() { "denied" => LoginFlowError::Denied, @@ -227,7 +232,10 @@ fn apply_login_status( } // "idle" / "canceled": another tab or the daemon finished the // flow out from under us — reset the note, keep the modal. - _ => ui.login_modal_status = None, + _ => { + close_login_popup_placeholder(); + ui.login_modal_status = None; + } } if ui.login_modal_status != previous || !ui.login_modal_open { b.host_mut().mark_editor_state_dirty(); @@ -235,12 +243,53 @@ fn apply_login_status( } } -/// Open the verification page. A blocked popup falls back to a full -/// navigation in a new tab via an anchor click, and if even that is -/// unavailable the user can still see the URL-less waiting note (the -/// approval also works from any logged-in browser tab). -fn open_popup(url: &str) { - if let Some(window) = web_sys::window() { - let _ = window.open_with_url_and_target(url, "_blank"); +thread_local! { + /// Placeholder popup opened synchronously inside the SignIn click + /// (user-activation context — the only moment `window.open` is + /// reliably allowed). The poll callback later navigates it to the + /// verification page; opening from the async callback instead gets + /// popup-blocked, which is exactly the "have to click sign-in + /// twice" failure this design removes. + static PENDING_POPUP: RefCell> = const { RefCell::new(None) }; +} + +/// Open the placeholder popup. Must be called synchronously from a +/// pointer-event handler (the web sign-in press dispatcher). +pub(crate) fn open_login_popup_placeholder() { + let opened = web_sys::window() + .and_then(|window| { + window + .open_with_url_and_target("about:blank", "_blank") + .ok() + }) + .flatten(); + PENDING_POPUP.with(|slot| *slot.borrow_mut() = opened); +} + +/// Close a still-pending placeholder (flow canceled or failed before +/// the verification page was known). +pub(crate) fn close_login_popup_placeholder() { + PENDING_POPUP.with(|slot| { + if let Some(popup) = slot.borrow_mut().take() { + let _ = popup.close(); + } + }); +} + +/// Point the placeholder at the verification page; when the placeholder +/// is missing (blocked, or an older flow) fall back to a direct open — +/// it may be blocked outside a gesture, but the approval also works +/// from any logged-in browser tab, so the flow still completes. +fn navigate_login_popup(url: &str) { + let pending = PENDING_POPUP.with(|slot| slot.borrow_mut().take()); + match pending { + Some(popup) => { + let _ = popup.location().set_href(url); + } + None => { + if let Some(window) = web_sys::window() { + let _ = window.open_with_url_and_target(url, "_blank"); + } + } } } diff --git a/crates/op-host-web/src/widget_host/account_press.rs b/crates/op-host-web/src/widget_host/account_press.rs index 83c8eddd0..9e9e16f35 100644 --- a/crates/op-host-web/src/widget_host/account_press.rs +++ b/crates/op-host-web/src/widget_host/account_press.rs @@ -43,6 +43,23 @@ impl WidgetHost { self.editor_state.editor_ui.login_modal_stub_hint_shown = false; } LoginModalHit::SignIn => { + // A flow is already running — don't spawn another + // placeholder popup or re-begin; the poll owns the modal. + if matches!( + self.editor_state.editor_ui.login_modal_status, + Some( + LoginFlowStatus::WaitingBrowser + | LoginFlowStatus::WaitingApproval + | LoginFlowStatus::Exchanging + ) + ) { + self.mark_dirty(); + return; + } + // Open the placeholder popup NOW, inside the click's + // user-activation window — an async `window.open` later + // would be popup-blocked (the "click twice" bug). + crate::web_auth_sync::open_login_popup_placeholder(); // Optimistic status so the modal reacts on this frame; // the daemon poll refines it (or reports the failure). self.editor_state.editor_ui.login_modal_status = diff --git a/crates/op-i18n/src/i18n/en.rs b/crates/op-i18n/src/i18n/en.rs index 1ea197c51..e5947d693 100644 --- a/crates/op-i18n/src/i18n/en.rs +++ b/crates/op-i18n/src/i18n/en.rs @@ -587,6 +587,8 @@ pub fn lookup(key: &str) -> Option<&'static str> { "account.signInComingSoon" => "Browser sign-in is coming soon.", "account.waitingForBrowser" => "Waiting for your browser…", "account.waitingForApproval" => "Approve the sign-in in your browser…", + "account.openingPopup" => "Opening the sign-in window…", + "account.approveInPopup" => "Finish signing in in the popup window…", "account.signingIn" => "Signing you in…", "account.signInDenied" => "Sign-in was declined in the browser.", "account.signInExpired" => "The sign-in request expired. Try again.", diff --git a/crates/op-i18n/src/i18n/zh_cn.rs b/crates/op-i18n/src/i18n/zh_cn.rs index 7707be40d..c150be459 100644 --- a/crates/op-i18n/src/i18n/zh_cn.rs +++ b/crates/op-i18n/src/i18n/zh_cn.rs @@ -591,6 +591,8 @@ pub fn lookup(key: &str) -> Option<&'static str> { "account.signInComingSoon" => "浏览器登录即将上线。", "account.waitingForBrowser" => "正在等待浏览器…", "account.waitingForApproval" => "请在浏览器中确认登录…", + "account.openingPopup" => "正在打开登录窗口…", + "account.approveInPopup" => "请在弹出的窗口中完成登录确认…", "account.signingIn" => "正在登录…", "account.signInDenied" => "登录请求在浏览器中被拒绝。", "account.signInExpired" => "登录请求已过期，请重试。",