From d4c3a304b581e7c0fdb81659714eb34191258fe0 Mon Sep 17 00:00:00 2001 From: PJ Date: Wed, 12 Aug 2026 18:32:45 +0530 Subject: [PATCH] Sign in on a phone with no console work, in the browser's own session Mobile OAuth reused the desktop client all along; what stopped it was the browser. Sending the user out to Safari or Chrome backgrounds the app, iOS suspends it, and the redirect carrying the code arrives at a socket nobody is accepting on. The consent page now opens in front of the app instead, in SFSafariViewController or a Chrome Custom Tab, so the loopback listener stays live and the existing `installed` client is enough. Verified against Google's real consent screen on a simulator and an emulator. A per-platform client is still supported and is now an upgrade rather than a prerequisite. On iOS it buys ASWebAuthenticationSession, which shares Safari's session so nobody is asked to sign in to Google twice. Android needs nothing: Custom Tabs share Chrome's cookies, measured rather than assumed. iOS session sharing could not be confirmed on the simulator and wants a real device. Never an app-owned WebView: Google blocks it, and rightly, since a webview the app controls can read the password typed into it. Cancelling is no longer reported as a failure. AuthEvent carries a `cancelled` flag, set by comparing against the constant every back-out path returns, and Google's `access_denied` on desktop counts too. Five frontend bugs found by driving the real UI, not by reading it: the details card slid under the tab bar leaving its buttons unhittable; the ghost click after a touch pressed a button in the card that tap had just opened, opening the editor by itself; the swipe that pages the day was dead over every read-only block; 84px of macOS traffic-light lane was reserved on platforms with no traffic lights; and the desktop header ignored the top safe area on an iPad. A first launch now says what to do next rather than showing an empty grid, and accounts are named as Google accounts throughout. --- docs/architecture.md | 43 +- docs/mobile.md | 130 +++++- src-tauri/Cargo.lock | 2 + src-tauri/Cargo.toml | 15 +- src-tauri/gen/android/app/build.gradle.kts | 3 + .../android/app/src/main/AndroidManifest.xml | 11 + .../studio/margin/calendar/MainActivity.kt | 65 ++- src-tauri/src/dto.rs | 4 + src-tauri/src/google/auth.rs | 195 ++++++-- src-tauri/src/google/browser.rs | 420 ++++++++++++++++++ src-tauri/src/google/mod.rs | 6 +- src-tauri/src/lib.rs | 33 +- src/App.tsx | 6 +- src/components/Accounts.tsx | 10 +- src/components/CalendarList.tsx | 2 +- src/components/EventDetails.tsx | 20 +- src/components/FirstRun.tsx | 38 ++ src/components/GridEvent.tsx | 35 ++ src/components/GridView.tsx | 11 +- src/components/Settings.tsx | 2 +- src/components/overlayShell.tsx | 2 +- src/dev/mockIpc.ts | 18 +- src/ipc.ts | 10 + src/keys/commands.ts | 4 +- src/main.tsx | 5 + src/store/useAccounts.ts | 26 +- src/styles/app.css | 50 ++- src/styles/details.css | 10 + tests/bugs.spec.ts | 249 +++++++++++ tests/overlays.spec.ts | 6 +- tests/phone.spec.ts | 2 +- 31 files changed, 1319 insertions(+), 114 deletions(-) create mode 100644 src-tauri/src/google/browser.rs create mode 100644 src/components/FirstRun.tsx create mode 100644 tests/bugs.spec.ts diff --git a/docs/architecture.md b/docs/architecture.md index 0b7ae69..e4338d3 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -11,27 +11,34 @@ locked to `ipc:` exactly as margin has it. ## Authentication -Lifted from `margin/src-tauri/src/gdrive.rs`, then split in two when mobile arrived. Both halves -build the same consent URL with PKCE S256 and a CSRF state parameter and open it in the system -browser through the opener plugin, never an in-app webview: Google blocks the embedded-webview -flow, and it deserves to be blocked, because a webview the app controls can read the password -typed into it. They differ only in how the answer comes back. +Lifted from `margin/src-tauri/src/gdrive.rs`. Every platform builds the same consent URL with PKCE +S256 and a CSRF state parameter, and every platform opens it in the system's browser, never in a +webview this app owns: Google blocks the embedded-webview flow, and it deserves to be blocked, +because a webview the app controls can read the password typed into it. -Desktop binds a listener on `127.0.0.1:0` and catches the redirect on the loopback socket. The -verifier lives on that listener's stack for the two minutes it is alive. +The default flow is the same one on all five platforms. Bind a listener on `127.0.0.1:0`, ask +Google to redirect there, and catch the code on the loopback socket. Google allows a Desktop client +any loopback port without registering it, and the token endpoint checks the client id, the secret +and the redirect rather than the operating system, so a phone can use the desktop client too. What +makes that safe to rely on is where the browser is: on mobile the consent page opens **in front of** +the app, in `SFSafariViewController` or a Chrome Custom Tab, so this process stays foreground and +its listener stays live. Sending the user out to Safari would suspend it and the redirect would +arrive at a socket nobody is accepting on. `google/browser.rs` is that surface, and dismissing it +once the code lands. -Mobile cannot do that. iOS will not keep a background listener alive dependably, and Google -rejects loopback redirects for Android and iOS client types outright, so the redirect is a custom -URI scheme the OS routes back to the app through `tauri-plugin-deep-link`. There is no listener to -hold the verifier, the browser is a separate app and this process may be backgrounded while the -user consents, so the verifier waits in `AuthState.pending` and `handle_redirect` picks it up -whenever the link lands. It is taken rather than read, so a replayed link cannot start a second -exchange. +The second flow runs where a platform has been given its own OAuth client, which is Google's stated +guidance and what to fall back on if they ever enforce it. Those clients are public, have no secret, +and redirect to a custom URI scheme rather than to loopback. The verifier has no listener stack to +live on, so it waits in `AuthState.pending` until the callback lands, and is taken rather than read +so a replayed link cannot start a second exchange. -That split forces one more difference. A desktop client is confidential and has a secret; an -Android or iOS client is public and has none, so `google-credentials.json` carries up to three -clients and the build embeds the one for the platform it is compiling for. Details and the -console steps are in [mobile.md](mobile.md). +Where the callback lands differs. Android opens the external browser and the OS routes the scheme +back through `tauri-plugin-deep-link`, which is also the arrival route on a cold start. iOS uses +`ASWebAuthenticationSession`, which reports the URL straight to a completion handler and is the only +iOS surface that shares Safari's cookies, so an account already signed in on the phone is offered by +name. That last point is the reason the choice is not merely academic on iOS, and +[mobile.md](mobile.md) argues it out. `load_credentials` decides once, by whether the block is in +the file, and everything downstream follows from that. The scope is `https://www.googleapis.com/auth/calendar` plus `openid email`. Calendar is a sensitive scope, so an unverified client shows the unverified-app interstitial and caps at 100 diff --git a/docs/mobile.md b/docs/mobile.md index 2d63440..51ba404 100644 --- a/docs/mobile.md +++ b/docs/mobile.md @@ -4,14 +4,72 @@ The same Rust core and the same React app, with a different shape of chrome and getting a token back from Google. Nothing is forked: the layout switches on a `data-phone` attribute and the OAuth flow switches on `cfg(mobile)`. -## Google will not let you reuse the desktop client +## Signing in, and what console work buys you -This is the part that cannot be automated away, so do it first or nothing will sign in. +Both platforms sign in out of the box with the `installed` client the desktop build already uses. +Android is finished at that point. iOS works, but signs the user in from scratch, and one OAuth +client fixes it. That asymmetry is the whole of this section. -Google issues OAuth clients per platform and refuses one in another's place. The desktop client -redirects to a loopback port; Google rejects loopback redirects for Android and iOS client types -outright, and iOS will not dependably keep a listener alive to catch one anyway. So a phone build -redirects to a custom URI scheme instead, and needs its own client to do it. +### The flow that needs nothing + +A phone uses the desktop client, secret and all, and catches Google's answer on the same loopback +listener the desktop flow binds. Two facts make that work: a Desktop client may redirect to loopback +on **any** port with nothing registered in advance, and Google's token endpoint validates the client +id, the secret and the redirect URI without any way of knowing which operating system is asking. +Client types are policy guidance rather than a protocol check. + +What used to make this impossible on a phone was not the protocol. It was that opening the consent +page in Safari or Chrome sends the user to another app, iOS suspends this process, and a suspended +process is not accepting on its socket, so the redirect carrying the code arrives at nobody. The +consent page now opens **in front of** the app instead, in the system's own browser component: +`SFSafariViewController` on iOS, a Chrome Custom Tab on Android. This app stays foreground and its +listener stays live. `src-tauri/src/google/browser.rs` is all of it. + +Still the system browser, note, and never a WebView this app owns. Google blocks that outright with +`disallowed_useragent`, and it deserves to be blocked, because a webview the app controls can read +the password typed into it. A Safari sheet or a Custom Tab is a different process with the browser's +own cookies and autofill and an address bar the app cannot forge. + +**Be clear about the trade.** Reusing a Desktop client from a phone is off Google's stated guidance, +which says to create an Android or iOS client per platform. It works because the protocol does not +check, not because Google blesses it. If Google ever starts enforcing the guidance the symptom will +be a rejected token exchange, and the fix is a per-platform client, which the code already supports +in full. + +### Why Android needs nothing else + +A Custom Tab is Chrome. It reads Chrome's cookie jar, so an account already signed in there is +offered by name and there is no password to type. Loopback and shared session at once, for nothing. + +### Why iOS wants an `ios` client + +`SFSafariViewController` has not shared cookies with Safari since iOS 11: every app gets its own +storage. So the no-setup iOS flow puts a real Safari view in front of the user with an empty cookie +jar behind it, and Google has no idea who they are. It signs in. It just asks for a full login every +time the token store is emptied. + +`ASWebAuthenticationSession` is the API Apple shipped for exactly this, and it is the only one that +shares Safari's session, which is why iOS puts up its own "wants to use google.com to sign in" +prompt before it opens. But it intercepts a custom scheme and nothing else, never an http loopback +redirect, so on iOS a shared session and loopback are mutually exclusive. Adding an `ios` block is +what buys the scheme, and with it the good version. + +So `auth.rs` runs three flows, not two: loopback everywhere by default, `ASWebAuthenticationSession` +plus the custom scheme when there is an `ios` client, and the external browser plus the deep link +when there is an `android` one. `load_credentials` decides once, by whether the block is in the +file. + +An iOS client is about a minute of work: it wants the bundle identifier and nothing else, no +fingerprint, no keystore. An Android client wants a SHA-1 per signing key and buys nothing, so +there is no reason to make one unless Google forces the issue. + +One thing observed rather than assumed, and worth knowing before anyone re-tests this on a +simulator: a cookie set in the simulator's Safari showed up in neither surface, including +`ASWebAuthenticationSession` after its sharing prompt was accepted. The prompt appears and the +plumbing is right, so this looks like the simulator not backing the shared jar rather than the API. +Judge the session sharing on a real device. + +## Creating the per-platform clients Both mobile clients are **public**: no client secret exists, and PKCE is the only thing between an intercepted authorization code and a token. That is why the verifier is not optional anywhere in @@ -34,7 +92,7 @@ In the Google Cloud console, on the same project that already has the Calendar A 2. Create an OAuth client of type **iOS**. It wants the bundle identifier, which is also `studio.margin.calendar`. -3. Put both client ids in `google-credentials.json` alongside the desktop one, matching +3. Put the client ids in `google-credentials.json` alongside the desktop one, matching `google-credentials.example.json`: ```json @@ -45,12 +103,17 @@ In the Google Cloud console, on the same project that already has the Calendar A } ``` - The file is gitignored and embedded at build time. A build with no `android` or `ios` block - compiles and then says so at runtime rather than failing to compile, which is the same bargain - the desktop client already had. + The file is gitignored and embedded at build time. Adding a block for one platform leaves the + other on loopback: the choice is per platform, not per file. + +A refresh token belongs to the client that obtained it, so adding or removing a block invalidates +whatever is already stored on that device. Disconnect and reconnect the account afterwards. ## The redirect schemes +These matter only for the flow above. The loopback flow redirects to `http://127.0.0.1:PORT` and +never touches a scheme. + Android redirects to `studio.margin.calendar:/oauth2redirect`. That is the package name, which is Google's documented form for an Android client, and being known at build time means it can sit in `AndroidManifest.xml` permanently rather than being pasted in per install. @@ -75,10 +138,47 @@ when the `mobile` array is empty it deletes the key outright. That one empty arr callback was dead on both platforms at first, so if a deep link ever stops arriving, look here before anywhere else. +The `studio.margin.calendar` scheme stays registered on both platforms whether or not anything +uses it, because the OS is told about a scheme at install time and cannot be told about one later. + If the console shows you something different from either default, put it in the client's block in `google-credentials.json` as `redirect_uri` and it wins over both. Whatever you put there still has to have its scheme registered above, or the OS has no reason to hand the link to this app. +## The consent browser, and taking it away again + +Neither surface closes itself once the redirect has landed, so `browser.rs` dismisses both. What is +on screen by then is the listener's own "you can close this" page, and leaving it up would look +like a sign-in that hung while the token exchange quietly succeeded behind it. + +On iOS that is a `dismissViewControllerAnimated:` on the main thread. `SFSafariViewController` has +no objc2 binding, since `objc2-safari-services` covers only the macOS extension API, so the class +is reached by name and the SafariServices framework is linked by hand to put it in the process at +all. The controller and its delegate are both retained for the length of the flow: UIKit holds a +delegate weakly, and a controller nothing retains deallocates mid-sign-in. + +On Android a Custom Tab belongs to Chrome and cannot be closed by the app that launched it. What +works instead is starting `MainActivity` with `FLAG_ACTIVITY_CLEAR_TOP | FLAG_ACTIVITY_SINGLE_TOP`, +which brings this app back to the front of the task the tab was launched into and pops the tab off +on the way. Same move AppAuth makes. + +Closing the browser by hand is the other exit, and it has to be noticed or the accounts panel waits +on a sign-in that will never arrive. iOS gets it directly from `safariViewControllerDidFinish:`. +Android has no such callback, so it reads a process resume while a consent attempt is in flight, +which means the tab is gone: the tab is in this app's own task, and dismissal is the only way out +of it. Either way `await_code` sees the flag on its next poll and gives up. + +`ASWebAuthenticationSession` needs none of that. It takes itself away when the callback scheme +matches and reports a cancel as error code 1, so the completion handler is the whole of it. What it +does need is to be retained: a session nothing holds deallocates and then never calls back, which +is the classic way to lose an afternoon to that API, and its presentation context provider is held +weakly so it goes the same way. Both are kept in `browser.rs` until the next attempt replaces them, +rather than being released inside the completion handler that the session itself owns. + +One thing the Rust side cannot do anything about: a cancel still reaches the accounts panel as a +failed connect, because `AuthEvent` has no third shape, so it reads as "Sign-in was cancelled." +under a toast rather than as nothing at all. + ## Toolchain iOS needs Xcode and the iOS Rust targets: @@ -109,8 +209,14 @@ pnpm tauri android dev # emulator or attached device pnpm tauri android build ``` -The generated projects are committed. Re-running `init` overwrites them, so check afterwards that -`MainActivity.kt` still publishes the window insets, since nothing else on Android does. +The generated projects are committed, and three files in the Android one are edited by hand. +`MainActivity.kt` publishes the window insets and launches the consent tab, `app/build.gradle.kts` +carries the `androidx.browser` dependency the tab needs, and `AndroidManifest.xml` carries a +`` element without which Android 11 and up hide every browser from +`CustomTabsClient.getPackageName` and the consent page quietly falls back to a separate browser +app. Re-running `android init` overwrites all three and nothing else supplies any of them, so check +for them afterwards: without the insets the bars overlap the page, and without the bridge tapping +Connect opens nothing at all. Both `dev` commands start Vite themselves and fail outright if port 1430 is already taken, which it will be if a browser dev server is still up. Kill it, or pass a different port through diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 2ff7ad2..9e8d4ba 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -2279,10 +2279,12 @@ name = "margin-calendar" version = "0.1.0" dependencies = [ "base64 0.22.1", + "block2", "chacha20poly1305", "chrono", "chrono-tz", "objc2", + "objc2-foundation", "objc2-ui-kit", "rand 0.8.7", "reqwest 0.12.28", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 9140e39..9174e64 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -44,9 +44,10 @@ chacha20poly1305 = "0.10" tauri-plugin-process = "2" tauri-plugin-updater = "2" -# One UIKit property that wry does not set for us; stop_uikit_shrinking_the_viewport in lib.rs says -# which one and why. These versions are the ones wry already resolves for its own iOS backend, so -# matching them keeps a single copy of objc2 in the build rather than a second incompatible one. +# Two things wry leaves to us: one UIKit property (stop_uikit_shrinking_the_viewport in lib.rs) and +# the SFSafariViewController that shows the consent page (google/browser.rs). These versions are the +# ones wry already resolves for its own iOS backend, so matching them keeps a single copy of objc2 in +# the build rather than a second incompatible one. [target.'cfg(target_os = "ios")'.dependencies] objc2 = "0.6" objc2-ui-kit = { version = "0.3", default-features = false, features = [ @@ -55,6 +56,14 @@ objc2-ui-kit = { version = "0.3", default-features = false, features = [ "UIView", "UIScrollView", ] } +objc2-foundation = { version = "0.3", default-features = false, features = [ + "std", + "NSString", + "NSURL", +] } +# Only for the null completion handler the two present/dismiss calls take. Passing a raw null +# pointer there would encode as an object rather than a block, which objc2 checks and rejects. +block2 = "0.6" [dev-dependencies] tempfile = "3" diff --git a/src-tauri/gen/android/app/build.gradle.kts b/src-tauri/gen/android/app/build.gradle.kts index 132aabe..f95ddbd 100644 --- a/src-tauri/gen/android/app/build.gradle.kts +++ b/src-tauri/gen/android/app/build.gradle.kts @@ -58,6 +58,9 @@ rust { } dependencies { + // Chrome Custom Tabs, for the OAuth consent page. Added by hand, so `tauri android init` will + // drop it again along with the bridge in MainActivity.kt that uses it. + implementation("androidx.browser:browser:1.8.0") implementation("androidx.webkit:webkit:1.14.0") implementation("androidx.appcompat:appcompat:1.7.1") implementation("androidx.activity:activity-ktx:1.10.1") diff --git a/src-tauri/gen/android/app/src/main/AndroidManifest.xml b/src-tauri/gen/android/app/src/main/AndroidManifest.xml index d4225f6..01efbaf 100644 --- a/src-tauri/gen/android/app/src/main/AndroidManifest.xml +++ b/src-tauri/gen/android/app/src/main/AndroidManifest.xml @@ -5,6 +5,17 @@ + + + + + + + // The cutout is folded in rather than trusted on its own: a landscape cutout down one side diff --git a/src-tauri/src/dto.rs b/src-tauri/src/dto.rs index ea3590d..bbbedaa 100644 --- a/src-tauri/src/dto.rs +++ b/src-tauri/src/dto.rs @@ -174,4 +174,8 @@ pub struct AuthEvent { pub error: Option, pub account_id: Option, pub email: Option, + /// The user closed the consent browser rather than anything going wrong. Still `ok: false`, + /// because no account arrived, but changing your mind is not a failure and must not be + /// reported as one. + pub cancelled: bool, } diff --git a/src-tauri/src/google/auth.rs b/src-tauri/src/google/auth.rs index c12a441..03fabbc 100644 --- a/src-tauri/src/google/auth.rs +++ b/src-tauri/src/google/auth.rs @@ -7,12 +7,9 @@ // a different account cannot write against stale remote ids. use std::collections::HashMap; -#[cfg(desktop)] use std::io::{Read, Write}; -#[cfg(desktop)] use std::net::{TcpListener, TcpStream}; use std::sync::LazyLock; -#[cfg(desktop)] use std::time::Instant; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -29,13 +26,20 @@ use crate::dto::{Account, AuthEvent}; use crate::google::secrets; pub const SCOPES: &str = "openid email https://www.googleapis.com/auth/calendar"; + +/// How long the listener waits for Google to come back. Two minutes is generous on a desktop, where +/// the browser is a window away and a keyboard is a keyboard. #[cfg(desktop)] pub const AUTH_TIMEOUT_SECS: u64 = 120; -/// How long a mobile consent round trip may take. Far longer than the desktop listener's two -/// minutes, because on a phone the browser is a separate app: signing in, a password manager and -/// a 2FA prompt in a third app can all happen between leaving and coming back, and this process -/// is doing nothing but holding a verifier in the meantime. +/// The same wait on a phone, which has to cover an email and a password typed on glass, a password +/// manager round trip, and very often a 2FA prompt in a third app. +#[cfg(mobile)] +pub const AUTH_TIMEOUT_SECS: u64 = 900; + +/// The same again for the deep link flow, where the wait is even less this app's to control: the +/// browser is a separate app there, so this process may be backgrounded for all of it while it does +/// nothing but hold a verifier. #[cfg(mobile)] pub const PENDING_TIMEOUT_SECS: u64 = 900; @@ -53,10 +57,14 @@ pub static HTTP: LazyLock = LazyLock::new(|| { .expect("could not build the HTTP client") }); -/// Google issues a different OAuth client per platform and will not accept one in another's place. -/// A desktop client is confidential and redirects to loopback; an Android or iOS client is public, -/// has no secret at all, and redirects to a custom URI scheme. So the credentials file carries up -/// to three clients and the build picks the one for the platform it is being compiled for. +/// Up to three OAuth clients, of which a build uses exactly one. +/// +/// `installed` is a Desktop client: confidential, so it has a secret, and allowed to redirect to +/// loopback on any port without registering it first. `android` and `ios` are public clients with +/// no secret at all, and they redirect to a custom URI scheme instead. +/// +/// A phone uses `installed` unless its own block is present, which is a deliberate choice and the +/// reason mobile sign-in needs no console work. `connect` says what the two flows cost. #[derive(Deserialize)] struct CredentialsFile { installed: Credentials, @@ -81,6 +89,12 @@ pub struct Credentials { /// Mobile only, and only when Google's console shows something other than the default below. #[serde(default)] pub redirect_uri: Option, + /// Set at load time rather than read from the file: true when this client came out of the + /// `android` or `ios` block. It is the only thing that decides which of the two mobile flows + /// runs, so it travels with the client that forces the choice rather than being worked out + /// again wherever the answer is needed. + #[serde(skip)] + pub platform_client: bool, } fn default_auth_uri() -> String { @@ -109,20 +123,27 @@ fn reversed_client_id(client_id: &str) -> String { const NOT_SET_UP: &str = "Google Calendar is not set up yet. Add a real OAuth client to google-credentials.json and rebuild."; +/// The one client this build signs in with. A missing platform block is not an error: it means the +/// desktop client and the loopback flow, which is what a phone gets until somebody decides +/// otherwise. Every caller has to agree on the answer, because a refresh token belongs to the +/// client that obtained it. pub fn load_credentials() -> Result { let parsed: CredentialsFile = serde_json::from_str(CREDENTIALS_JSON) .map_err(|e| format!("invalid google-credentials.json: {e}"))?; #[cfg(target_os = "android")] - let creds = parsed.android.ok_or( - "google-credentials.json has no \"android\" client. Create an OAuth client of type Android in the Google Cloud console and add it. See docs/mobile.md.", - )?; + let (mut creds, platform_client) = match parsed.android { + Some(creds) => (creds, true), + None => (parsed.installed, false), + }; #[cfg(target_os = "ios")] - let creds = parsed.ios.ok_or( - "google-credentials.json has no \"ios\" client. Create an OAuth client of type iOS in the Google Cloud console and add it. See docs/mobile.md.", - )?; + let (mut creds, platform_client) = match parsed.ios { + Some(creds) => (creds, true), + None => (parsed.installed, false), + }; #[cfg(not(any(target_os = "android", target_os = "ios")))] - let creds = parsed.installed; + let (mut creds, platform_client) = (parsed.installed, false); + creds.platform_client = platform_client; if creds.client_id.starts_with("YOUR_CLIENT_ID") || creds @@ -135,8 +156,9 @@ pub fn load_credentials() -> Result { Ok(creds) } -/// Where Google sends the browser back to. Desktop binds a loopback port per attempt, so it is -/// decided in `connect` rather than here and this is mobile only. +/// Where Google sends the browser back to on the custom scheme flow. The loopback flow binds a port +/// per attempt and works its own redirect out in `connect_by_loopback`, so this is only for the +/// platforms that have been given their own client. #[cfg(mobile)] pub fn redirect_uri(creds: &Credentials) -> String { if let Some(explicit) = &creds.redirect_uri { @@ -219,7 +241,6 @@ fn urlencode(s: &str) -> String { url::form_urlencoded::byte_serialize(s.as_bytes()).collect() } -#[cfg(desktop)] fn write_http_message(stream: &mut TcpStream, message: &str) { let body = format!( "Margin Calendar\ @@ -235,7 +256,6 @@ fn write_http_message(stream: &mut TcpStream, message: &str) { let _ = stream.flush(); } -#[cfg(desktop)] #[derive(Debug, PartialEq, Eq)] enum Redirect { Code(String), @@ -245,7 +265,6 @@ enum Redirect { Waiting, } -#[cfg(desktop)] fn request_path(request: &str) -> &str { request .lines() @@ -254,7 +273,6 @@ fn request_path(request: &str) -> &str { .unwrap_or("") } -#[cfg(desktop)] fn parse_redirect(path: &str, expected_state: &str) -> Redirect { if path == "/favicon.ico" { return Redirect::Waiting; @@ -284,7 +302,14 @@ fn parse_redirect(path: &str, expected_state: &str) -> Redirect { } } -#[cfg(desktop)] +/// What the frontend is told when the user backed out rather than finishing: closing the consent +/// browser on a phone, or pressing Cancel on Google's own screen anywhere. +/// +/// `emit_auth` compares against this exact string to set `AuthEvent.cancelled`, which is what stops +/// the frontend reporting a change of mind as a failure. So it is a constant on every platform, and +/// every path that means "the user chose not to" must return this rather than wording its own. +const CANCELLED: &str = "Sign-in was cancelled."; + fn await_code( listener: TcpListener, expected_state: &str, @@ -295,6 +320,13 @@ fn await_code( if Instant::now() > deadline { return Err("Timed out waiting for Google authorization.".to_string()); } + // Closing the consent page is the mobile equivalent of closing the browser tab, and the + // one abandonment the OS actually tells us about. Polled here rather than interrupting the + // loop, so this stays the only place that decides an attempt is over. + #[cfg(mobile)] + if crate::google::browser::cancelled() { + return Err(CANCELLED.to_string()); + } match listener.accept() { Ok((mut stream, _)) => { stream.set_nonblocking(false).ok(); @@ -315,6 +347,12 @@ fn await_code( &mut stream, "Authorization was cancelled. You can close this tab.", ); + // `access_denied` is Google's word for the user pressing Cancel on the + // consent screen, which is the same decision as closing the browser and + // deserves the same quiet handling. Anything else really did go wrong. + if error == "access_denied" { + return Err(CANCELLED.to_string()); + } return Err(format!("Google authorization failed: {error}")); } Redirect::Mismatch => { @@ -491,9 +529,12 @@ fn auth_url(creds: &Credentials, redirect: &str, challenge: &str, csrf: &str) -> ) } -/// The system browser, never an in-app webview. Google blocks the embedded-webview flow outright, -/// and it deserves to be blocked: a webview the app controls can read what the user types into it. -fn open_in_browser(app: &tauri::AppHandle, url: &str) { +/// Hands the URL to whichever browser the OS considers the user's, in its own process. Never an +/// in-app webview: Google blocks the embedded-webview flow outright, and it deserves to be blocked, +/// because a webview the app controls can read what the user types into it. `browser.rs` covers the +/// mobile surfaces, which are the system's browser too and are only in front of the app rather than +/// inside it. +pub(super) fn open_in_browser(app: &tauri::AppHandle, url: &str) { use tauri_plugin_opener::OpenerExt; let _ = app.opener().open_url(url.to_string(), None::<&str>); } @@ -507,10 +548,14 @@ fn emit_auth(app: &tauri::AppHandle, outcome: Result<(String, String), String>) error: None, account_id: Some(account_id), email: Some(email), + cancelled: false, } } + // Compared against the constant the cancel paths raise, rather than matched on its text, + // so rewording it cannot quietly turn a cancel back into an error on screen. Err(error) => AuthEvent { ok: false, + cancelled: error == CANCELLED, error: Some(error), account_id: None, email: None, @@ -519,9 +564,34 @@ fn emit_auth(app: &tauri::AppHandle, outcome: Result<(String, String), String>) let _ = app.emit("auth", event); } -#[cfg(desktop)] +/// One consent request, on all five platforms. +/// +/// The loopback flow is the default everywhere, phones included. Google lets a Desktop client +/// redirect to loopback on any port with nothing registered in advance, and the token endpoint +/// checks the client id, the secret and the redirect and has no idea which OS asked. What used to +/// make this impossible on a phone was not the protocol but the browser: sending the user out to +/// Safari suspends this process, and a suspended process is not accepting on its socket. An in-app +/// browser does not leave, which is why `browser.rs` exists and why this branch is now the common +/// one. +/// +/// The custom scheme flow runs instead when the credentials file has a client for this platform. +/// That is Google's stated guidance, and the thing to reach for if they ever start enforcing it, +/// but on iOS it is worth having for its own sake: it is the only way to reach +/// `ASWebAuthenticationSession`, which is the only iOS browser that shares Safari's cookies. +/// docs/mobile.md has the trade in full. pub async fn connect(app: tauri::AppHandle) -> Result { let creds = load_credentials()?; + #[cfg(mobile)] + if creds.platform_client { + return connect_by_deep_link(app, creds).await; + } + connect_by_loopback(app, creds).await +} + +async fn connect_by_loopback( + app: tauri::AppHandle, + creds: Credentials, +) -> Result { let verifier = random_b64(64); let challenge = pkce_challenge(&verifier); let csrf = random_b64(24); @@ -531,7 +601,10 @@ pub async fn connect(app: tauri::AppHandle) -> Result { let redirect = format!("http://127.0.0.1:{port}"); let url = auth_url(&creds, &redirect, &challenge, &csrf); + #[cfg(desktop)] open_in_browser(&app, &url); + #[cfg(mobile)] + crate::google::browser::open(&app, &url); let app_bg = app.clone(); tauri::async_runtime::spawn(async move { @@ -542,13 +615,26 @@ pub async fn connect(app: tauri::AppHandle) -> Result { Ok(url) } -/// Mobile has no loopback listener to wait on, so `connect` ends the moment the browser opens and -/// the flow resumes in `handle_redirect` whenever the deep link arrives. What is stashed here is -/// the PKCE verifier and the CSRF state, which is the only thing tying the code that comes back to -/// the request that went out. +/// The custom scheme flow. What is stashed here is the PKCE verifier and the CSRF state, which is +/// the only thing tying the code that comes back to the request that went out, since there is no +/// listener holding either on its own stack. +/// +/// Where the answer comes back from differs by platform, and so does what it is worth. +/// +/// iOS hands the URL to `ASWebAuthenticationSession`, which reports the callback straight to a +/// completion handler. It is the only iOS surface that shares Safari's cookies, so an account +/// already signed in on the phone is offered by name rather than asking for a password again. That +/// is the reason this flow is worth the console visit on iOS, and `browser.rs` says the rest. +/// +/// Android opens the external browser and waits for the deep link, which is a genuinely separate +/// app: this process may be backgrounded, or killed outright, for the whole of it. Android needs +/// none of this, because a Custom Tab already shares Chrome's cookies and the loopback flow above +/// already works, so this is only ever reached when somebody has gone and made an Android client. #[cfg(mobile)] -pub async fn connect(app: tauri::AppHandle) -> Result { - let creds = load_credentials()?; +async fn connect_by_deep_link( + app: tauri::AppHandle, + creds: Credentials, +) -> Result { let verifier = random_b64(64); let challenge = pkce_challenge(&verifier); let csrf = random_b64(24); @@ -561,15 +647,40 @@ pub async fn connect(app: tauri::AppHandle) -> Result { *pending = Some(Pending { state: csrf, verifier, - redirect, + redirect: redirect.clone(), expires: now() + PENDING_TIMEOUT_SECS, }); } + #[cfg(target_os = "ios")] + crate::google::browser::authenticate(&app, &url, callback_scheme(&redirect)); + #[cfg(not(target_os = "ios"))] open_in_browser(&app, &url); Ok(url) } +/// `ASWebAuthenticationSession` matches on the scheme alone and wants it bare, so the path and the +/// colon that `redirect_uri` builds have to come back off. +#[cfg(target_os = "ios")] +fn callback_scheme(redirect: &str) -> &str { + redirect.split(':').next().unwrap_or(redirect) +} + +/// The consent surface ended with no callback URL at all: the user closed it, or it could not be +/// shown. Either way the pending verifier is spent, and the panel is waiting on an answer that is +/// never coming, so it is told. A cancel says so plainly rather than reporting a failure. +#[cfg(mobile)] +pub async fn abandon_pending(app: tauri::AppHandle, reason: Option) { + let auth = app.state::(); + let pending = { + let mut slot = auth.pending.lock().await; + slot.take() + }; + if pending.is_some() { + emit_auth(&app, Err(reason.unwrap_or_else(|| CANCELLED.to_string()))); + } +} + /// Every URL the OS hands the app on a registered scheme, including ones that have nothing to do /// with consent. A link that carries no `state` we are waiting for is ignored in silence rather /// than reported, because another feature may want that scheme later and a stray link is not an @@ -635,7 +746,6 @@ pub async fn handle_redirect(app: tauri::AppHandle, incoming: &url::Url) { emit_auth(&app, outcome); } -#[cfg(desktop)] async fn complete_auth( app: &tauri::AppHandle, listener: TcpListener, @@ -649,9 +759,16 @@ async fn complete_auth( await_code(listener, &csrf, deadline) }) .await - .map_err(|e| e.to_string())??; + .map_err(|e| e.to_string())?; - finish(app, &creds, &code, &redirect, &verifier).await + // Nothing takes the consent page down on its own once the listener has its answer, and what it + // is showing by then is the listener's own "you can close this" page. Taken away here rather + // than after the exchange, and whatever the outcome was, so the app comes back the moment the + // browser has nothing left to do. Desktop has no such surface and compiles this out. + #[cfg(mobile)] + crate::google::browser::close(app); + + finish(app, &creds, &code?, &redirect, &verifier).await } /// Code to stored account. Everything past the point where the two flows stop differing. diff --git a/src-tauri/src/google/browser.rs b/src-tauri/src/google/browser.rs new file mode 100644 index 0000000..904bf1b --- /dev/null +++ b/src-tauri/src/google/browser.rs @@ -0,0 +1,420 @@ +// Where the consent page is shown on a phone, and how it is taken away again. +// +// Desktop hands the URL to the system browser and forgets about it. The loopback listener is the +// only thing that has to survive, and a desktop process keeps running perfectly well while another +// window has focus. A phone does not work that way: sending the user out to Safari or Chrome +// backgrounds this process, iOS suspends it, and a suspended process accepts nothing on its socket, +// so the redirect carrying the authorization code arrives at nobody. +// +// So the consent page is put in front of the app instead, by the system's own browser component. +// Never a WebView this app owns: Google blocks that outright with `disallowed_useragent`, and it +// deserves to be blocked, because a webview the app controls can read what is typed into it. What +// this app gets in return for not owning it is to stay foreground, with its listener still +// accepting, for as long as consent takes. +// +// Three surfaces, because the platforms do not offer the same thing. +// +// Chrome Custom Tab, Android. Chrome itself, so it reads Chrome's cookie jar and an account +// already signed in there is offered by name. Nothing else is needed on Android. +// +// SFSafariViewController, iOS, when there is no `ios` OAuth client. Safari's engine, but with +// storage of this app's own since iOS 11, so the user signs in from scratch inside it. It works +// with no console setup at all, which is the only reason to accept that. +// +// ASWebAuthenticationSession, iOS, when there is one. The only iOS surface that shares Safari's +// session, and the one to prefer, but it intercepts a custom scheme and never an http loopback +// redirect, so it is reachable only where an `ios` client has bought a scheme. +// +// The first two are `open`/`close`: they know nothing about the answer, so the loopback listener +// stays the thing that decides an attempt is over, and the two flags below carry the one fact it +// cannot see for itself. `authenticate` is the third and reports its own outcome. + +use std::sync::atomic::{AtomicBool, Ordering}; + +/// Set while the consent page is up and the listener is still waiting on it. Only a dismissal +/// during that window means anything: the same signals fire when this app takes the browser down +/// itself, a moment after the code has already arrived. +static WAITING: AtomicBool = AtomicBool::new(false); + +/// Set when the user closed the browser without finishing. `await_code` polls it rather than being +/// interrupted, which costs up to one poll interval and keeps the listener loop the only thing that +/// decides when a consent attempt is over. +static DISMISSED: AtomicBool = AtomicBool::new(false); + +pub fn open(app: &tauri::AppHandle, url: &str) { + DISMISSED.store(false, Ordering::SeqCst); + WAITING.store(true, Ordering::SeqCst); + present(app, url); +} + +pub fn close(app: &tauri::AppHandle) { + WAITING.store(false, Ordering::SeqCst); + dismiss(app); +} + +pub fn cancelled() -> bool { + DISMISSED.load(Ordering::SeqCst) +} + +/// The user closed the consent page: the Done button on iOS, Back or a swipe on Android. +pub fn note_dismissed() { + if WAITING.load(Ordering::SeqCst) { + DISMISSED.store(true, Ordering::SeqCst); + } +} + +/// iOS presents the consent page inside this app, so this process never leaves the foreground and +/// there is nothing to hook. Android's Custom Tab is a Chrome activity on top of ours, so this app +/// really does go to the background and coming back is the signal: the tab is in this app's own +/// task, and the only way out of it is dismissal. `close` clears `WAITING` before it brings the +/// activity forward, so the resume this app asks for itself is not mistaken for the user's. +#[cfg(target_os = "android")] +pub fn note_resumed() { + note_dismissed(); +} + +/// The other iOS surface, and the one to prefer where a custom scheme exists to make it possible. +/// Nothing to do with the two flags above: it reports its own cancel, because it has a completion +/// handler to report it through and no listener to interrupt. +#[cfg(target_os = "ios")] +pub fn authenticate(app: &tauri::AppHandle, url: &str, scheme: &str) { + ios::authenticate(app, url, scheme); +} + +#[cfg(target_os = "ios")] +fn present(app: &tauri::AppHandle, url: &str) { + ios::present(app, url); +} + +#[cfg(target_os = "ios")] +fn dismiss(app: &tauri::AppHandle) { + ios::dismiss(app); +} + +#[cfg(target_os = "ios")] +mod ios { + use block2::{DynBlock, RcBlock}; + use objc2::rc::{Allocated, Retained}; + use objc2::runtime::{AnyClass, AnyObject, ClassBuilder, NSObject, Sel}; + use objc2::{msg_send, sel, ClassType}; + use objc2_foundation::{NSError, NSString, NSURL}; + use tauri::Manager; + + use std::cell::RefCell; + + // SFSafariViewController has no objc2 binding: objc2-safari-services covers the macOS extension + // API and nothing else. Linking the framework by hand is what puts the class in the process at + // all, after which it can be looked up by name. + #[link(name = "SafariServices", kind = "framework")] + extern "C" {} + + thread_local! { + /// The presented controller and the delegate it reports to, kept only so they can be found + /// again: UIKit holds a delegate weakly, and a controller nothing retains is a controller + /// that deallocates mid-flow. Both slots are main thread only, which is where every line in + /// this module runs. + static PRESENTED: RefCell>> = const { RefCell::new(None) }; + static DELEGATE: RefCell>> = const { RefCell::new(None) }; + } + + /// SFSafariViewController calls this when the user taps Done, and never when the dismissal came + /// from `dismiss` below. UIKit takes the controller off screen on its own here, so there is + /// nothing to do but let go of it and say what happened. + extern "C-unwind" fn did_finish(_this: &AnyObject, _cmd: Sel, _controller: *mut AnyObject) { + PRESENTED.with(|slot| slot.borrow_mut().take()); + DELEGATE.with(|slot| slot.borrow_mut().take()); + super::note_dismissed(); + } + + /// Registered once, lazily, because a class pair can only be registered under a given name + /// once per process. SFSafariViewControllerDelegate is checked with `respondsToSelector:` + /// rather than `conformsToProtocol:`, so declaring the one method is enough and there is no + /// protocol to adopt. + fn delegate_class() -> &'static AnyClass { + thread_local! { + static CLASS: &'static AnyClass = { + let mut builder = ClassBuilder::new(c"MarginConsentDelegate", NSObject::class()) + .expect("MarginConsentDelegate is registered once and by nobody else"); + unsafe { + builder.add_method( + sel!(safariViewControllerDidFinish:), + did_finish as extern "C-unwind" fn(_, _, _), + ); + } + builder.register() + }; + } + CLASS.with(|class| *class) + } + + pub fn present(app: &tauri::AppHandle, url: &str) { + let Some(window) = app.get_webview_window("main") else { + return crate::google::auth::open_in_browser(app, url); + }; + let inner_app = app.clone(); + let inner_url = url.to_string(); + let posted = window.with_webview(move |webview| { + // `with_webview` hands the WKWebView over on the main thread, which is both where UIKit + // needs this and where the two slots above live. Its window is the app's own, so the + // sheet comes up over the calendar rather than over whatever else UIKit would have + // picked. + let presented = unsafe { + let view = webview.inner().cast::(); + let ui_window: Option> = msg_send![view, window]; + let root: Option> = match &ui_window { + Some(ui_window) => msg_send![&**ui_window, rootViewController], + None => None, + }; + match root { + Some(root) => show(&root, &inner_url), + None => false, + } + }; + if !presented { + crate::google::auth::open_in_browser(&inner_app, &inner_url); + } + }); + if posted.is_err() { + crate::google::auth::open_in_browser(app, url); + } + } + + /// Everything that can be missing here is missing for the same reason: an iOS that does not + /// have the class, or a URL the OS will not parse. Both answer false and the caller falls back + /// to the external browser rather than leaving the user looking at nothing. + unsafe fn show(root: &AnyObject, url: &str) -> bool { + let Some(class) = AnyClass::get(c"SFSafariViewController") else { + return false; + }; + let Some(url) = NSURL::URLWithString(&NSString::from_str(url)) else { + return false; + }; + + let allocated: Allocated = msg_send![class, alloc]; + let controller: Option> = msg_send![allocated, initWithURL: &*url]; + let Some(controller) = controller else { + return false; + }; + + let delegate: Retained = msg_send![delegate_class(), new]; + let () = msg_send![&*controller, setDelegate: &*delegate]; + DELEGATE.with(|slot| *slot.borrow_mut() = Some(delegate)); + + let completion: Option<&DynBlock> = None; + let () = msg_send![root, presentViewController: &*controller, animated: true, completion: completion]; + PRESENTED.with(|slot| *slot.borrow_mut() = Some(controller)); + true + } + + pub fn dismiss(app: &tauri::AppHandle) { + let _ = app.run_on_main_thread(|| { + let Some(controller) = PRESENTED.with(|slot| slot.borrow_mut().take()) else { + return; + }; + DELEGATE.with(|slot| slot.borrow_mut().take()); + unsafe { + let completion: Option<&DynBlock> = None; + // Sent to the presented controller rather than the presenting one, which UIKit + // forwards. Letting go of the last reference afterwards is safe: the presenting + // controller holds it for the length of the animation. + let () = msg_send![&*controller, dismissViewControllerAnimated: true, completion: completion]; + } + }); + } + + // The second iOS surface, and the better one. See `authenticate` for what it buys. + #[link(name = "AuthenticationServices", kind = "framework")] + extern "C" {} + + /// `ASWebAuthenticationSessionErrorCodeCanceledLogin`. The user closed the sheet, which is not + /// a failure worth dressing up as one. + const CANCELED_LOGIN: isize = 1; + + thread_local! { + /// The session, the object that tells it which window to present over, and that window. + /// + /// A session nothing retains deallocates and then simply never calls back, which is the + /// classic way to lose an afternoon to this API, and the presentation context provider is + /// held weakly so it goes the same way. Replaced on the next attempt rather than cleared in + /// the completion handler, because the session owns the block that handler lives in and + /// releasing it from inside its own invocation is asking for a use after free. + static SESSION: RefCell>> = const { RefCell::new(None) }; + static ANCHOR: RefCell>> = const { RefCell::new(None) }; + static ANCHOR_WINDOW: RefCell>> = const { RefCell::new(None) }; + } + + /// Required from iOS 13 on: with no anchor the session refuses to start and reports + /// `presentationContextNotProvided` instead. Returned unretained, which is the convention for + /// a getter like this one, and safe because ANCHOR_WINDOW holds it for the flow. + extern "C-unwind" fn presentation_anchor( + _this: &AnyObject, + _cmd: Sel, + _session: *mut AnyObject, + ) -> *mut AnyObject { + ANCHOR_WINDOW.with(|slot| match slot.borrow().as_ref() { + Some(window) => Retained::as_ptr(window).cast_mut(), + None => std::ptr::null_mut(), + }) + } + + fn anchor_class() -> &'static AnyClass { + thread_local! { + static CLASS: &'static AnyClass = { + let mut builder = ClassBuilder::new(c"MarginConsentAnchor", NSObject::class()) + .expect("MarginConsentAnchor is registered once and by nobody else"); + unsafe { + builder.add_method( + sel!(presentationAnchorForWebAuthenticationSession:), + presentation_anchor as extern "C-unwind" fn(_, _, _) -> _, + ); + } + builder.register() + }; + } + CLASS.with(|class| *class) + } + + /// The consent page in an `ASWebAuthenticationSession`, which is the same Safari view underneath + /// but with Safari's own cookies rather than a jar of this app's alone. That is the whole point + /// of it: an account already signed in on this phone is offered by name instead of asking for a + /// password again. It is also why iOS puts up its own "wants to use google.com to sign in" + /// prompt first, since sharing the session is something the user gets to refuse. + /// + /// The price is that it only ever intercepts a custom scheme, never an http loopback redirect, + /// so this path exists only where a scheme exists: an `ios` block in the credentials file. + pub fn authenticate(app: &tauri::AppHandle, url: &str, scheme: &str) { + let Some(window) = app.get_webview_window("main") else { + return crate::google::auth::open_in_browser(app, url); + }; + let inner_app = app.clone(); + let inner_url = url.to_string(); + let scheme = scheme.to_string(); + let posted = window.with_webview(move |webview| { + let started = unsafe { + let view = webview.inner().cast::(); + let ui_window: Option> = msg_send![view, window]; + match ui_window { + Some(ui_window) => start(&inner_app, ui_window, &inner_url, &scheme), + None => false, + } + }; + if !started { + crate::google::auth::open_in_browser(&inner_app, &inner_url); + } + }); + if posted.is_err() { + crate::google::auth::open_in_browser(app, url); + } + } + + unsafe fn start( + app: &tauri::AppHandle, + ui_window: Retained, + url: &str, + scheme: &str, + ) -> bool { + let Some(class) = AnyClass::get(c"ASWebAuthenticationSession") else { + return false; + }; + let Some(url) = NSURL::URLWithString(&NSString::from_str(url)) else { + return false; + }; + let scheme = NSString::from_str(scheme); + + let handler_app = app.clone(); + let handler = RcBlock::new(move |callback: *mut NSURL, error: *mut NSError| { + finished(&handler_app, callback, error); + }); + + let allocated: Allocated = msg_send![class, alloc]; + let session: Option> = msg_send![ + allocated, + initWithURL: &*url, + callbackURLScheme: &*scheme, + completionHandler: &*handler, + ]; + let Some(session) = session else { + return false; + }; + + // False, not true: an ephemeral session is a fresh cookie jar, which throws away the one + // reason to be using this API at all. + let () = msg_send![&*session, setPrefersEphemeralWebBrowserSession: false]; + + let anchor: Retained = msg_send![anchor_class(), new]; + let () = msg_send![&*session, setPresentationContextProvider: &*anchor]; + ANCHOR_WINDOW.with(|slot| *slot.borrow_mut() = Some(ui_window)); + ANCHOR.with(|slot| *slot.borrow_mut() = Some(anchor)); + + let started: bool = msg_send![&*session, start]; + SESSION.with(|slot| *slot.borrow_mut() = Some(session)); + started + } + + /// One of three answers: the callback URL, a cancel, or a real failure. The URL goes to the same + /// `handle_redirect` the deep link uses, so the state check and the exchange stay in one place + /// and this knows nothing about either. + fn finished(app: &tauri::AppHandle, callback: *mut NSURL, error: *mut NSError) { + if !callback.is_null() { + let absolute = unsafe { + let string: Retained = msg_send![callback, absoluteString]; + string.to_string() + }; + if let Ok(parsed) = url::Url::parse(&absolute) { + let app = app.clone(); + tauri::async_runtime::spawn(async move { + crate::google::auth::handle_redirect(app, &parsed).await; + }); + } + return; + } + + let code = if error.is_null() { + CANCELED_LOGIN + } else { + unsafe { msg_send![error, code] } + }; + let reason = if code == CANCELED_LOGIN { + None + } else { + Some(format!("The sign-in sheet could not be shown (error {code}).")) + }; + let app = app.clone(); + tauri::async_runtime::spawn(async move { + crate::google::auth::abandon_pending(app, reason).await; + }); + } +} + +/// Android has no equivalent of `with_webview`, and reaching a Custom Tab from here would mean JNI +/// in Rust for the sake of one Intent. It goes through the JavaScript bridge in `MainActivity.kt` +/// instead, which is the same shape as the window insets bridge that is already there. +/// +/// That bridge is the one thing this depends on, and `tauri android init` regenerates the file it +/// lives in. Without it the script below is a no-op and nothing opens at all, which is why +/// docs/mobile.md says to check for it after running init. +#[cfg(target_os = "android")] +fn present(app: &tauri::AppHandle, url: &str) { + let literal = match serde_json::to_string(url) { + Ok(literal) => literal, + Err(_) => return crate::google::auth::open_in_browser(app, url), + }; + if eval(app, &format!("window.__androidAuthTab?.open({literal})")).is_err() { + crate::google::auth::open_in_browser(app, url); + } +} + +#[cfg(target_os = "android")] +fn dismiss(app: &tauri::AppHandle) { + let _ = eval(app, "window.__androidAuthTab?.close()"); +} + +#[cfg(target_os = "android")] +fn eval(app: &tauri::AppHandle, script: &str) -> Result<(), String> { + use tauri::Manager; + + let window = app + .get_webview_window("main") + .ok_or("there is no window to run the bridge from")?; + window.eval(script).map_err(|e| e.to_string()) +} diff --git a/src-tauri/src/google/mod.rs b/src-tauri/src/google/mod.rs index ae19ad8..bc35d96 100644 --- a/src-tauri/src/google/mod.rs +++ b/src-tauri/src/google/mod.rs @@ -1,9 +1,13 @@ -// auth.rs OAuth, token refresh. Loopback on desktop, a deep link on mobile. +// auth.rs OAuth, token refresh. A loopback listener everywhere, a deep link when a phone has +// been given its own OAuth client. +// browser.rs the consent page in front of the app on a phone, and taking it away again // api.rs typed Google Calendar REST wrapper // secrets.rs refresh tokens, encrypted on disk, same on every platform pub mod api; pub mod auth; +#[cfg(mobile)] +pub mod browser; pub mod secrets; use crate::dto::Account; diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 76922a6..9abbdbc 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -29,7 +29,7 @@ fn build_menu(handle: &tauri::AppHandle) -> tauri::Result let sync_now = MenuItemBuilder::with_id("sync-now", "Sync Now") .accelerator("CmdOrCtrl+R") .build(handle)?; - let accounts = MenuItemBuilder::with_id("accounts", "Accounts…").build(handle)?; + let accounts = MenuItemBuilder::with_id("accounts", "Google Accounts…").build(handle)?; let check_updates = MenuItemBuilder::with_id("check-updates", "Check for Updates…").build(handle)?; let settings = MenuItemBuilder::with_id("settings", "Settings…") @@ -136,8 +136,10 @@ fn build_menu(handle: &tauri::AppHandle) -> tauri::Result Ok(menu) } -/// Google's answer to a consent request comes back as a link into this app rather than to a -/// loopback port, because a phone has no loopback port to give it. +/// The deep link half of mobile auth: Google's answer arrives as a link into this app rather than +/// on a loopback socket. Only the flow that runs when a phone has its own OAuth client uses it, but +/// it stays registered on both platforms either way, because the OS has to be told about a scheme +/// at install time and cannot be told about one later. /// /// Both arrival routes are covered. `on_open_url` catches the link when the app was already /// running, which is the usual case since it is what opened the browser a moment ago; @@ -196,6 +198,27 @@ fn stop_uikit_shrinking_the_viewport(window: &tauri::WebviewWindow) { }); } +/// A Chrome Custom Tab is Chrome's activity sitting on top of ours, in our own task, so this +/// process really is backgrounded for the length of a consent round trip and coming back means the +/// tab has gone. That is the only notice Android gives that the user backed out of signing in, and +/// without it the accounts panel waits on an answer that is never coming. `note_resumed` works out +/// whether the tab went because the user closed it or because this app took it down a moment after +/// the code arrived. +/// +/// Not `RunEvent::Resumed`, which sounds right and is not: tauri raises that one on every poll of +/// the event loop. tao's real mobile resume arrives here, per window. +/// +/// iOS shows the consent page inside the app, never leaves the foreground, and gets the same +/// question answered by a delegate in `google/browser.rs` instead. +#[cfg(target_os = "android")] +fn watch_for_the_consent_tab_closing(window: &tauri::WebviewWindow) { + window.on_window_event(|event| { + if matches!(event, tauri::WindowEvent::Resumed) { + google::browser::note_resumed(); + } + }); +} + #[cfg_attr(mobile, tauri::mobile_entry_point)] pub fn run() { // generate_context! first, so the updater plugin registers only when the merged config @@ -230,6 +253,10 @@ pub fn run() { if let Some(window) = handle.get_webview_window("main") { stop_uikit_shrinking_the_viewport(&window); } + #[cfg(target_os = "android")] + if let Some(window) = handle.get_webview_window("main") { + watch_for_the_consent_tab_closing(&window); + } Ok(()) }); diff --git a/src/App.tsx b/src/App.tsx index 88959ac..55976d8 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -6,6 +6,7 @@ import { Header } from "./components/Header"; import { PhoneMenu, PhoneTabBar, PhoneTopBar } from "./components/PhoneBar"; import { GridView } from "./components/GridView"; import { AgendaView } from "./components/AgendaView"; +import { FirstRun } from "./components/FirstRun"; import { MiniMonthOverlay } from "./components/MiniMonth"; import { CalendarList } from "./components/CalendarList"; import { EventEditor } from "./components/EventEditor"; @@ -51,7 +52,9 @@ function App() { if (!isTauri) return; const menu = listen("menu-action", (event) => handleMenuAction(event.payload)); const auth = listen("auth", (event) => { - void useAccounts.getState().handleAuthEvent(event.payload.ok, event.payload.error); + void useAccounts + .getState() + .handleAuthEvent(event.payload.ok, event.payload.error, event.payload.cancelled); }); // A pass that fails in the background used to set the error and say nothing, so a calendar // that never arrived looked like a calendar you do not have. Surface it once per distinct @@ -113,6 +116,7 @@ function App() {
+
{phone ? : null} diff --git a/src/components/Accounts.tsx b/src/components/Accounts.tsx index 3931990..dcf5b3d 100644 --- a/src/components/Accounts.tsx +++ b/src/components/Accounts.tsx @@ -55,7 +55,7 @@ export function Accounts() { return ( void connect()} > - Connect an account + Connect a Google account ) } @@ -78,8 +78,8 @@ export function Accounts() { body={

The token is revoked and every calendar, event and pending write stored on this - computer for that account is deleted. Nothing changes in Google Calendar itself, and - you can connect the account again afterwards. + computer for that Google account is deleted. Nothing changes in Google Calendar + itself, and you can connect it again afterwards.

} confirmLabel="Disconnect" @@ -127,7 +127,7 @@ export function Accounts() { {accounts.length === 0 && !connecting ? (

- No account is connected, so there is nothing to show on the grid yet. + Connect a Google account to see its calendars here. Nothing syncs until you do.

) : ( accounts.map((account) => ( diff --git a/src/components/CalendarList.tsx b/src/components/CalendarList.tsx index e9a3382..6fc9804 100644 --- a/src/components/CalendarList.tsx +++ b/src/components/CalendarList.tsx @@ -74,7 +74,7 @@ export function CalendarList() {

No calendars here yet.

) : ( diff --git a/src/components/EventDetails.tsx b/src/components/EventDetails.tsx index 1773a25..1144f55 100644 --- a/src/components/EventDetails.tsx +++ b/src/components/EventDetails.tsx @@ -48,18 +48,26 @@ const TEXT = "M4 6h16M4 12h12M4 18h9"; const USERS = "M17 21v-2a4 4 0 0 0-4-4H6a4 4 0 0 0-4 4v2M12.5 7.5a3.5 3.5 0 1 1-7 0 3.5 3.5 0 0 1 7 0M22 21v-2a4 4 0 0 0-3-3.87M16 3.13a4 4 0 0 1 0 7.75"; -/** Clear of the window edges, and clear of the one row of chrome that is always resident. */ +/** Clear of the window edges, and clear of the chrome that is always resident. */ const EDGE = 8; +/** + * What the card is allowed to cover, measured off the chrome rather than named in tokens. + * + * A desktop has one row of it at the top. A phone has two, the second along the bottom, and both + * of them pad themselves out of the way of a notch and a home indicator, so their rectangles are + * the only thing that knows how tall they really are. Reading `--titlebar-h` instead meant the + * card was placed against the window: in landscape it came down over the tab bar, and since the + * bars paint above it, Edit, Delete and Close ended up behind the tab bar and unhittable. + */ function viewBounds(): Bounds { - const bar = Number.parseFloat( - getComputedStyle(document.documentElement).getPropertyValue("--titlebar-h"), - ); + const top = document.querySelector(".titlebar, .phonebar")?.getBoundingClientRect().bottom ?? 0; + const bottom = document.querySelector(".tabbar")?.getBoundingClientRect().top ?? window.innerHeight; return { - top: (Number.isFinite(bar) ? bar : 0) + EDGE, + top: top + EDGE, left: EDGE, right: window.innerWidth - EDGE, - bottom: window.innerHeight - EDGE, + bottom: bottom - EDGE, }; } diff --git a/src/components/FirstRun.tsx b/src/components/FirstRun.tsx new file mode 100644 index 0000000..05bb751 --- /dev/null +++ b/src/components/FirstRun.tsx @@ -0,0 +1,38 @@ +// What a first launch says. With nothing connected the grid is a correct and completely empty +// calendar, which looks exactly like a calendar you have nothing in, so the one thing to do next +// went unsaid: the panel that does it is behind a key on the desktop and an overflow sheet on a +// phone, and neither is somewhere you look when you do not yet know it exists. +// +// It covers the grid rather than sitting beside it. There is nothing underneath worth reading, and +// a note floating over an empty axis reads as a thing that failed to load. + +import { runCommand } from "../keys/commands"; +import { useAccounts } from "../store/useAccounts"; + +export function FirstRun() { + const loaded = useAccounts((s) => s.loaded); + const accounts = useAccounts((s) => s.accounts); + + if (!loaded || accounts.length > 0) return null; + + return ( +
+
+

No Google account connected

+

+ Connect one and the calendars on it show up here. Nothing syncs until you do. +

+ +
+
+ ); +} + +export default FirstRun; diff --git a/src/components/GridEvent.tsx b/src/components/GridEvent.tsx index 8e163cd..162532c 100644 --- a/src/components/GridEvent.tsx +++ b/src/components/GridEvent.tsx @@ -31,6 +31,12 @@ import { openDetailsFor } from "./useDetails"; const CLICK_SLOP = 3; const TOUCH_CLICK_SLOP = 12; +/** + * How long the click a touch leaves behind is still worth waiting for. It lands a frame or so + * after the release here, and historically as much as 300ms behind it on a mobile browser. + */ +const GHOST_CLICK_MS = 400; + interface GridEventProps { item: Placed; top: number; @@ -42,6 +48,34 @@ interface GridEventProps { onPointerDown: (e: ReactPointerEvent, item: Placed, mode: DragMode) => void; } +/** + * Eats the click the browser sends after a touch, and only that one. + * + * Cancelling the pointerdown stops the mouse events that travel with it but never the click, and + * the click is hit tested wherever the finger is when it lands, which by then is the card this + * press has just opened. Left alone it presses whatever the card put under the finger: on a phone + * in landscape the card is the whole stage, so a tap on a meeting opened its editor or its + * conference link on its own. + */ +function swallowGhostClick(): void { + let timer = 0; + const done = () => { + window.clearTimeout(timer); + window.removeEventListener("click", eat, true); + window.removeEventListener("pointerdown", done, true); + }; + const eat = (e: MouseEvent) => { + e.preventDefault(); + e.stopPropagation(); + done(); + }; + window.addEventListener("click", eat, true); + // The first click after a release is the ghost, and a press that starts before it arrives means + // it is never coming. The timer is only the backstop for a browser that sends neither. + window.addEventListener("pointerdown", done, true); + timer = window.setTimeout(done, GHOST_CLICK_MS); +} + function edgeMode(target: EventTarget | null): DragMode { const el = target instanceof Element ? target.closest("[data-edge]") : null; const edge = el?.getAttribute("data-edge"); @@ -103,6 +137,7 @@ export const GridEvent = memo(function GridEvent({ if (Math.abs(event.clientX - downX) > slop) return; if (Math.abs(event.clientY - downY) > slop) return; open(element); + if (event.pointerType !== "mouse") swallowGhostClick(); }; window.addEventListener("pointerup", up, true); window.addEventListener("pointercancel", stop, true); diff --git a/src/components/GridView.tsx b/src/components/GridView.tsx index c5e2d2c..123fe1c 100644 --- a/src/components/GridView.tsx +++ b/src/components/GridView.tsx @@ -430,12 +430,19 @@ export function GridView({ defaultCalendarId }: GridViewProps) { if (e.button !== 0 || gesture.current) return; e.stopPropagation(); select(keyOf(item.instance)); - if (item.instance.readOnly || useGrid.getState().draft) return; const dayStart = startOfDay(item.startMs); const index = days.findIndex((d) => d === dayStart); if (index === -1) return; const { startMin, endMin } = dayMinutes(item, dayStart); - begin(e, mode, item, index, startMin, endMin); + // A block nothing can be done to is still something a swipe has to travel through. The press + // is stopped here rather than on the canvas, so without this the page turn was dead over + // every read-only event, which on a day with a couple of meetings marked busy is most of the + // column. It gets a gesture with no long press behind it: the only thing it can become is + // the swipe, and there is no item on it to commit a move to. + const inert = item.instance.readOnly || useGrid.getState().draft !== null; + if (inert && !isCoarse(e)) return; + begin(e, mode, inert ? null : item, index, startMin, endMin); + if (inert && gesture.current) clearPress(gesture.current); }, // `begin` closes over the current layout and days, which is what a fresh gesture wants. [days, layout, select], diff --git a/src/components/Settings.tsx b/src/components/Settings.tsx index 7da5a29..186524a 100644 --- a/src/components/Settings.tsx +++ b/src/components/Settings.tsx @@ -93,7 +93,7 @@ export function Settings() {
- Accounts + Google accounts {accounts.length === 0 ? "No Google account connected yet." diff --git a/src/components/overlayShell.tsx b/src/components/overlayShell.tsx index 5858a56..b56aa7c 100644 --- a/src/components/overlayShell.tsx +++ b/src/components/overlayShell.tsx @@ -30,7 +30,7 @@ const TITLES: Record = { calendars: "calendars", "mini-month": "the calendar", editor: "the event", - accounts: "accounts", + accounts: "Google accounts", settings: "settings", shortcuts: "shortcuts", menu: "the menu", diff --git a/src/dev/mockIpc.ts b/src/dev/mockIpc.ts index eb34b91..1aa1d2f 100644 --- a/src/dev/mockIpc.ts +++ b/src/dev/mockIpc.ts @@ -24,13 +24,26 @@ function apply(list: Instance[]): Instance[] { }); } +/** + * A first launch, which the fixture otherwise has no way to show: it is seeded with two connected + * accounts, so the one screen somebody new actually opens on was the one screen nobody could look + * at. With this set the three reads come back empty, the way they do before anything is connected. + */ +const firstRun = (): boolean => { + try { + return localStorage.getItem("margincal-dev-empty") === "1"; + } catch { + return false; + } +}; + export async function mockCall(command: string, args?: Record): Promise { const a = (args ?? {}) as Record; switch (command) { case "accounts_list": - return devAccounts as unknown as T; + return (firstRun() ? [] : devAccounts) as unknown as T; case "calendars_list": - return calendars as unknown as T; + return (firstRun() ? [] : calendars) as unknown as T; case "calendar_set_selected": { const id = a.calendarId as unknown as string; const selected = a.selected as unknown as boolean; @@ -39,6 +52,7 @@ export async function mockCall(command: string, args?: Record c.selected).map((c) => c.id)); diff --git a/src/ipc.ts b/src/ipc.ts index 560826f..c0ffe8c 100644 --- a/src/ipc.ts +++ b/src/ipc.ts @@ -24,6 +24,14 @@ const isMobileOs = */ export const isDesktop = isTauri && !isMobileOs; +/** + * The one window whose title bar has the traffic lights inside the page. `titleBarStyle: "Overlay"` + * in tauri.conf.json is a macOS-only setting, so on Linux, Windows and every mobile build the + * header has nothing to leave room for. + */ +export const isMacDesktop = + isDesktop && typeof navigator !== "undefined" && /mac/i.test(navigator.userAgent); + /** * True when there is a backend to answer a command: Tauri, or the dev fixture in a browser. * Data-loading actions gate on this. Anything touching a window API must gate on `isDesktop` @@ -168,6 +176,8 @@ export interface AuthEvent { error: string | null; accountId: string | null; email: string | null; + /** The consent browser was closed by hand. Not `ok`, but not a failure to report either. */ + cancelled: boolean; } /** Payload of `sync-progress`. `store-changed` carries a plain reason string. */ diff --git a/src/keys/commands.ts b/src/keys/commands.ts index f6e2d79..86c9a1c 100644 --- a/src/keys/commands.ts +++ b/src/keys/commands.ts @@ -176,7 +176,9 @@ const TABLE: Record> = { }, }, calendars: { label: "Calendars", palette: true, run: () => overlays().show("calendars") }, - accounts: { label: "Accounts", palette: true, run: () => overlays().show("accounts") }, + // Named for what it connects to rather than for the panel. "Accounts" on its own says nothing + // about whose, and this row is how somebody opening the app cold finds the thing to do first. + accounts: { label: "Google accounts", palette: true, run: () => overlays().show("accounts") }, settings: { label: "Settings", palette: true, run: () => overlays().show("settings") }, "toggle-theme": { label: "Toggle dark mode", palette: true, run: () => useTheme.getState().toggle() }, "check-updates": { diff --git a/src/main.tsx b/src/main.tsx index cc4deba..94d20ff 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -1,6 +1,7 @@ import React from "react"; import ReactDOM from "react-dom/client"; import App from "./App"; +import { isMacDesktop } from "./ipc"; import { trackSafeArea } from "./safeArea"; import "./styles/tokens.css"; import "./styles/fonts.css"; @@ -9,6 +10,10 @@ import "./styles/app.css"; // Before the first render, so the bars are the right height on the first paint rather than after. trackSafeArea(); +// The header's lane for the traffic lights, which only one platform draws over it. Written here +// rather than assumed by the stylesheet, for the same reason: the first paint is the right shape. +if (isMacDesktop) document.documentElement.setAttribute("data-traffic", ""); + ReactDOM.createRoot(document.getElementById("root") as HTMLElement).render( diff --git a/src/store/useAccounts.ts b/src/store/useAccounts.ts index b3ccf5e..da57c70 100644 --- a/src/store/useAccounts.ts +++ b/src/store/useAccounts.ts @@ -10,6 +10,12 @@ type Phase = "idle" | "connecting" | "working" | "error"; interface AccountsState { accounts: Account[]; calendars: Calendar[]; + /** + * True once a list has actually come back. An empty `accounts` before that is a page that has not + * asked yet, and telling the two apart is the difference between "connect one" and a first frame + * of it on every launch. + */ + loaded: boolean; phase: Phase; error: string | null; authUrl: string | null; @@ -17,7 +23,7 @@ interface AccountsState { refresh: () => Promise; connect: () => Promise; cancelConnect: () => void; - handleAuthEvent: (ok: boolean, error: string | null) => Promise; + handleAuthEvent: (ok: boolean, error: string | null, cancelled?: boolean) => Promise; openAuthUrl: () => void; copyAuthUrl: () => Promise; disconnect: (accountId: string) => Promise; @@ -27,6 +33,7 @@ interface AccountsState { export const useAccounts = create((set, get) => ({ accounts: [], calendars: [], + loaded: false, phase: "idle", error: null, authUrl: null, @@ -35,7 +42,7 @@ export const useAccounts = create((set, get) => ({ if (!live()) return; try { const [accounts, calendars] = await Promise.all([accountsList(), calendarsList()]); - set({ accounts, calendars }); + set({ accounts, calendars, loaded: true }); } catch (e) { set({ error: String(e) }); } @@ -50,7 +57,7 @@ export const useAccounts = create((set, get) => ({ .then((url) => set({ authUrl: url })) .catch((e) => { set({ phase: "error", error: String(e), resolveConnect: null }); - notify(`Could not connect: ${e}`); + notify(`Could not connect your Google account: ${e}`); resolve(false); }); }), @@ -59,17 +66,22 @@ export const useAccounts = create((set, get) => ({ set({ phase: "idle", authUrl: null, resolveConnect: null }); resolve?.(false); }, - handleAuthEvent: async (ok, error) => { + handleAuthEvent: async (ok, error, cancelled = false) => { if (get().phase !== "connecting") return; const resolve = get().resolveConnect; if (ok) { await get().refresh(); set({ phase: "idle", authUrl: null, error: null, resolveConnect: null }); notify("Connected to Google Calendar"); + } else if (cancelled) { + // Closing the consent browser is an answer, not a fault. Back to idle with nothing said: + // the user already knows what they did, and a red panel telling them about it reads as + // though shutting the sheet broke something. + set({ phase: "idle", authUrl: null, error: null, resolveConnect: null }); } else { const message = error ?? "authorization failed"; set({ phase: "error", authUrl: null, error: message, resolveConnect: null }); - notify(`Could not connect: ${message}`); + notify(`Could not connect your Google account: ${message}`); } resolve?.(ok); }, @@ -93,10 +105,10 @@ export const useAccounts = create((set, get) => ({ await accountDisconnect(accountId); await get().refresh(); set({ phase: "idle" }); - notify("Disconnected"); + notify("Google account disconnected"); } catch (e) { set({ phase: "error", error: String(e) }); - notify(`Could not disconnect: ${e}`); + notify(`Could not disconnect that Google account: ${e}`); } }, setSelected: async (calendarId, selected) => { diff --git a/src/styles/app.css b/src/styles/app.css index f15184f..4935ba2 100644 --- a/src/styles/app.css +++ b/src/styles/app.css @@ -103,15 +103,26 @@ select { flex: none; position: relative; z-index: 45; - height: var(--titlebar-h); + /* Out of the way of a status bar or a notch, the same way the phone bars do it. A desktop reads + both insets as zero; an iPad gets this header rather than those bars and does not. */ + height: calc(var(--titlebar-h) + var(--safe-top)); display: grid; grid-template-columns: 1fr auto 1fr; align-items: center; - padding: 0 14px 0 var(--traffic-pad); + padding: var(--safe-top) 14px 0; background: var(--shell); border-bottom: 1px solid var(--line); } +/* The macOS traffic lights float over this row, so it opens a lane for them and costs no extra + height. macOS is the only platform with any: they come from `titleBarStyle: "Overlay"`, which is + a macOS-only window setting, and on Linux, Windows, an iPad or a browser the lane was 84px of + nothing, pushing the view switcher off centre and starving the date range of the room it needed + to say what day it is. */ +:root[data-traffic] .titlebar { + padding-left: var(--traffic-pad); +} + .titlebar .lead { display: flex; align-items: center; @@ -273,9 +284,44 @@ select { min-height: 0; display: flex; flex-direction: column; + /* The first-run note lays itself over whatever is here, so this is the box it covers. */ + position: relative; background: var(--paper); } +/* Above every part of the grid, which stacks up to 15, and under the overlays, which start at 20: + the panel this opens has to come up in front of it. */ +.first-run { + position: absolute; + inset: 0; + z-index: 19; + display: grid; + place-items: center; + padding: 24px; + background: var(--paper); +} + +.first-run-text { + display: flex; + flex-direction: column; + align-items: center; + gap: 10px; + max-width: 320px; + text-align: center; +} + +.first-run-title { + margin: 0; + font-family: var(--font-heading); + font-weight: 500; + font-size: 18px; +} + +.first-run-note { + margin: 0; + color: var(--ink-soft); +} + /* Overlays, margin's idiom: summoned by a key, dismissed with Escape, never resident. */ .overlay { position: fixed; diff --git a/src/styles/details.css b/src/styles/details.css index 3701459..4d87c2f 100644 --- a/src/styles/details.css +++ b/src/styles/details.css @@ -45,6 +45,16 @@ max-height: min(480px, calc(100vh - var(--titlebar-h) - 16px)); } +/* There is no title bar here. What the card has to fit inside is the window less both bars and the + insets they pad themselves with, which is the same box the placement measures. A cap any taller + than that does not merely scroll, it gets placed over the top bar: the card is laid out first and + pinned to its block second, so a card too tall for the gap has nowhere legal to go. */ +:root[data-phone] .details-card { + max-height: calc( + 100dvh - var(--phonebar-h) - var(--safe-top) - var(--tabbar-h) - var(--safe-bottom) - 16px + ); +} + /* The one scroll region. The footer is outside it, so however long the description runs, Edit, Delete and Close are where they were. */ .details-body { diff --git a/tests/bugs.spec.ts b/tests/bugs.spec.ts new file mode 100644 index 0000000..2f575b9 --- /dev/null +++ b/tests/bugs.spec.ts @@ -0,0 +1,249 @@ +// Defects found by driving the app at sizes and with pointers nothing else in this suite uses: +// a phone on its side, a tablet, and a window whose safe areas are not zero. +// +// Same rules as the rest of the suite. Nothing reaches into the app, every assertion is something +// the browser laid out, and each test failed before the fix that sits next to it. + +import { expect, test, type Page } from "@playwright/test"; +import { box, openApp, settle } from "./app"; + +interface Point { + x: number; + y: number; +} + +/** + * A finger, the same way touch.spec.ts makes one: through CDP, because those arrive as real touch + * events and only those carry `pointerType` `touch` into the handlers that read it. + */ +async function finger(page: Page) { + const session = await page.context().newCDPSession(page); + const send = (type: string, points: Point[]) => + session.send("Input.dispatchTouchEvent", { + type, + touchPoints: points.map((p) => ({ x: Math.round(p.x), y: Math.round(p.y) })), + }); + return { + down: (at: Point) => send("touchStart", [at]), + moveTo: (at: Point) => send("touchMove", [at]), + up: () => send("touchEnd", []), + }; +} + +async function swipe(page: Page, from: Point, dx: number): Promise { + const hand = await finger(page); + await hand.down(from); + for (let step = 1; step <= 6; step++) await hand.moveTo({ x: from.x + (dx * step) / 6, y: from.y }); + await hand.up(); + await settle(page); +} + +const dates = (page: Page) => page.locator(".grid-head-date").allTextContents(); + +/** + * The card's buttons that the chrome has taken over: a tap in the middle of one reaches a bar + * instead. Anything hidden by the card's own scrolling is the card's business and not this. + */ +function buriedButtons(page: Page) { + return page.evaluate(() => { + const card = document.querySelector(".details-card"); + if (!card) throw new Error("no details card is open"); + return [...card.querySelectorAll("button")] + .map((el) => { + const rect = el.getBoundingClientRect(); + const hit = document.elementFromPoint( + Math.round(rect.left + rect.width / 2), + Math.round(rect.top + rect.height / 2), + ); + const chrome = (hit as HTMLElement | null)?.closest(".tabbar, .phonebar"); + return chrome + ? `${(el.textContent ?? "").trim() || el.getAttribute("aria-label")} is behind the ${chrome.className}` + : null; + }) + .filter((entry): entry is string => entry !== null); + }); +} + +// A phone on its side. 568x320 is an iPhone SE in landscape, still narrow enough for the phone +// chrome, and the one shape where the two bars leave the least behind. +test.describe("a phone in landscape", () => { + test.use({ viewport: { width: 568, height: 320 }, hasTouch: true, isMobile: true }); + + // The card was placed against the window and painted under the bars, which put Edit, Delete and + // Close behind the tab bar: not merely covered, unhittable. + test("the details card stays clear of the tab bar", async ({ page }) => { + await openApp(page, { view: "day" }); + + await page.locator(".grid-event", { hasText: "Design review" }).first().tap(); + await expect(page.locator(".details-card[data-placed]")).toBeVisible(); + await settle(page); + + const card = await box(page.locator(".details-card")); + const tabs = await box(page.locator(".tabbar")); + const bar = await box(page.locator(".phonebar")); + expect(card.bottom).toBeLessThanOrEqual(tabs.top); + expect(card.top).toBeGreaterThanOrEqual(bar.bottom); + + expect(await buriedButtons(page)).toEqual([]); + }); + + // The card opens where the finger already is, and a touch is followed by a click the finger never + // asked for. Cancelling the pointerdown stops the mouse events either side of it and not that + // click, so it landed on whatever the card had just put under the thumb: tapping a meeting opened + // its editor, and one with a conference link would have opened a browser. + test("a tap opens the card and nothing else", async ({ page }) => { + await openApp(page, { view: "day" }); + + await page.locator(".grid-event", { hasText: "Design review" }).first().tap(); + await expect(page.locator(".details-card")).toBeVisible(); + await settle(page); + + await expect(page.getByRole("dialog", { name: "Edit event" })).toHaveCount(0); + await expect(page.locator(".details-card")).toBeVisible(); + }); +}); + +test.describe("a phone", () => { + test.use({ viewport: { width: 390, height: 844 }, hasTouch: true, isMobile: true }); + + // The swipe is the way a phone pages the day, and a block you cannot pick up used to swallow it: + // the block's own handler stopped the press reaching the canvas and then returned. On a day with + // a couple of meetings marked busy that is most of the column. + test("a swipe still pages the day when it starts on a read-only block", async ({ page }) => { + await openApp(page, { view: "day" }); + const before = await dates(page); + + const block = page.locator(".grid-event[data-readonly]").first(); + const rect = await box(block); + await swipe(page, { x: rect.left + rect.width / 2, y: rect.top + rect.height / 2 }, -160); + + expect(await dates(page)).not.toEqual(before); + // And it paged instead of picking the block up, rather than as well as. + await expect(page.locator(".grid-ghost")).toHaveCount(0); + await expect(page.locator(".quick-create")).toHaveCount(0); + }); + + // A read-only block is still a block: the swipe must not have cost it its tap. + test("tapping a read-only block still opens it", async ({ page }) => { + await openApp(page, { view: "day" }); + await page.locator(".grid-event[data-readonly]").first().tap(); + await expect(page.locator(".details-card")).toBeVisible(); + }); +}); + +// The desktop header, on the platforms that are not macOS. Both of these are wrong everywhere +// except the one window the header was drawn in, and an iPad is now one of the places it lands. +test.describe("the desktop header off macOS", () => { + // The 84px lane exists for the macOS traffic lights, which come from `titleBarStyle: "Overlay"`, + // a macOS-only setting. Everywhere else, Linux and Windows and an iPad and this browser, it was + // 84px of nothing that pushed the whole row off centre and starved the date range. + test("there is no lane for traffic lights that do not exist", async ({ page }) => { + await openApp(page); + + const lead = await box(page.locator(".titlebar .lead")); + const views = await box(page.locator(".view-switch")); + const width = page.viewportSize()!.width; + + expect(lead.left).toBeLessThanOrEqual(16); + // The view switcher is the middle column of the row, so off centre means the row is padded for + // something that is not there. + expect(Math.abs((views.left + views.right) / 2 - width / 2)).toBeLessThanOrEqual(2); + + // And the lane is still there for the one window that has traffic lights in it. `main.tsx` + // writes this attribute on a macOS desktop build and nowhere else. + await page.evaluate(() => document.documentElement.setAttribute("data-traffic", "")); + await settle(page); + expect((await box(page.locator(".titlebar .lead"))).left).toBeGreaterThan(64); + }); + + // `--safe-top` is what the phone bars pad themselves with, and it is not zero on an iPad, which + // gets this header rather than those bars. The header claimed the top of the screen regardless, + // which puts its controls under the status bar. + test("the header clears the safe area above it", async ({ page }) => { + await openApp(page); + const before = await box(page.locator(".titlebar")); + + // Written the same way the Android bridge writes it in src/safeArea.ts, which is the one thing + // that ever sets these at runtime. + await page.evaluate(() => document.documentElement.style.setProperty("--safe-top", "44px")); + await settle(page); + + const after = await box(page.locator(".titlebar")); + expect(after.height).toBeCloseTo(before.height + 44, 0); + const controls = await page.locator(".titlebar button").all(); + for (const control of controls) { + const rect = await box(control); + expect(rect.top).toBeGreaterThanOrEqual(44); + } + }); +}); + +// A launch with nothing connected. The fixture ships two accounts, so this state existed and was +// never looked at: an empty grid, which is exactly what a quiet week looks like, and no hint that +// the thing to do is behind a key on the desktop and an overflow sheet on a phone. +test.describe("a first launch", () => { + // Not `openApp`: it waits for an event to be on screen, and the whole point of this state is that + // there are none. The seed is otherwise the same. + const openEmpty = async (page: Page) => { + await page.addInitScript(() => { + localStorage.clear(); + localStorage.setItem("margincal-theme", "light"); + localStorage.setItem("margincal-view", "week"); + localStorage.setItem("margincal-dev-empty", "1"); + }); + await page.goto("/"); + }; + + test("the empty calendar says what to do next", async ({ page }) => { + await openEmpty(page); + + const note = page.locator(".first-run"); + await expect(note).toBeVisible(); + await expect(note).toContainText("Google account"); + + // And the way in works from here, rather than only naming the thing. + await note.getByRole("button", { name: "Connect a Google account" }).click(); + await expect(page.getByRole("dialog", { name: "Google accounts" })).toBeVisible(); + }); + + test("it is gone as soon as there is an account", async ({ page }) => { + await openApp(page); + await expect(page.locator(".first-run")).toHaveCount(0); + }); +}); + +// The phone is the tightest thing any of this copy has to fit in, and a button that wraps is the +// tell that a label got longer than the chrome it lives in. +test.describe("account copy on the smallest phone", () => { + test.use({ viewport: { width: 320, height: 568 }, hasTouch: true, isMobile: true }); + + test("no account control wraps or spills", async ({ page }) => { + await openApp(page, { view: "day" }); + await page.getByRole("button", { name: "Menu" }).click(); + await page.locator(".menu-item", { hasText: "Google accounts" }).click(); + + const panel = page.getByRole("dialog", { name: "Google accounts" }); + await expect(panel).toBeVisible(); + + // A `.panel-button` never wraps, so a label that outgrows the sheet does not get taller, it + // runs out of its row and is clipped by the panel. Judged on the laid-out box against the row + // it sits in rather than on the length of the string. + const spilled = await page.evaluate(() => + [...document.querySelectorAll(".panel button, .menu-item, .first-run button")] + .map((el) => { + const row = el.parentElement?.getBoundingClientRect(); + if (!row) return null; + const box = el.getBoundingClientRect(); + const past = Math.max(Math.round(box.right - row.right), Math.round(row.left - box.left)); + return past > 1 ? `${(el.textContent ?? "").trim()} runs ${past}px past its row` : null; + }) + .filter((entry): entry is string => entry !== null), + ); + expect(spilled).toEqual([]); + + const spill = await page.evaluate( + () => document.documentElement.scrollWidth - document.documentElement.clientWidth, + ); + expect(spill).toBeLessThanOrEqual(0); + }); +}); diff --git a/tests/overlays.spec.ts b/tests/overlays.spec.ts index d3a5006..b76ae90 100644 --- a/tests/overlays.spec.ts +++ b/tests/overlays.spec.ts @@ -17,12 +17,12 @@ const PANELS = [ { name: "Calendars", open: openCalendars, contains: /you@example\.com/ }, { name: "Settings", open: openSettings, contains: /Week view/ }, { - name: "Accounts", + name: "Google accounts", open: async (page: Page) => { await openSettings(page); await page.getByRole("button", { name: "Manage" }).click(); }, - contains: /Connect an account/, + contains: /Connect a Google account/, }, ]; @@ -71,7 +71,7 @@ test("Escape unwinds one layer at a time", async ({ page }) => { await openSettings(page); await page.getByRole("button", { name: "Manage" }).click(); - const accounts = page.getByRole("dialog", { name: "Accounts" }); + const accounts = page.getByRole("dialog", { name: "Google accounts" }); await expect(accounts).toBeVisible(); await page.getByRole("button", { name: "Disconnect" }).first().click(); diff --git a/tests/phone.spec.ts b/tests/phone.spec.ts index 8a18e25..4c07591 100644 --- a/tests/phone.spec.ts +++ b/tests/phone.spec.ts @@ -88,7 +88,7 @@ test("the tab bar switches the view", async ({ page }) => { const ROWS: { label: string; opens: string }[] = [ { label: "Calendars", opens: "Calendars" }, { label: "Settings", opens: "Settings" }, - { label: "Accounts", opens: "Accounts" }, + { label: "Google accounts", opens: "Google accounts" }, { label: "Keyboard shortcuts", opens: "Keyboard shortcuts" }, ];