Skip to content

Fix trackpad pinch zoom in Safari#4709

Open
landkirk wants to merge 1 commit into
openfrontio:mainfrom
landkirk:fix/4664-safari-trackpad-zoom-2
Open

Fix trackpad pinch zoom in Safari#4709
landkirk wants to merge 1 commit into
openfrontio:mainfrom
landkirk:fix/4664-safari-trackpad-zoom-2

Conversation

@landkirk

@landkirk landkirk commented Jul 24, 2026

Copy link
Copy Markdown

Resolves #4664

Submitting directly as a small bug fix, per the exception in CONTRIBUTING.md.

Problem

Zoom is completely dead on macOS Safari with a trackpad.

Chrome and Firefox synthesize a ctrl+wheel event for a trackpad pinch, which
InputHandler.onScroll already handles. WebKit fires its non-standard
gesturestart/gesturechange/gestureend events instead, so that branch never
ran in Safari.

Nothing consumed those events either — installSafariPinchZoomBlocker only calls
preventDefault() on them to stop the page zooming, on the assumption that the
game's own pinch runs on pointer events. That holds on a touchscreen, but a
trackpad pinch produces no second pointer. Net effect: page zoom blocked, game
zoom never triggered, pinch does nothing.

Fix

Handle the gesture events on the input overlay and convert them to a ZoomEvent.

GestureEvent.scale is cumulative from gesturestart, so the per-event ratio is
scale / lastGestureScale. onZoom divides the camera scale by
1 + delta / 600, so inverting that gives the delta which reproduces the pinch
ratio exactly — a 1.2× pinch zooms 1.2×, with no tuning factor.

That 600 was an unexplained literal in TransformHandler. Since the gesture
math has to invert it, it's now exported as ZOOM_DELTA_DIVISOR so retuning the
zoom sensitivity can't silently desync the two.

The document-level blocker in DisableSafariPinchZoom.ts stays as-is: it runs for
the page lifetime and also blocks page zoom on the lobby and menus, where no
InputHandler exists.

Testing

Confirmed by hand on macOS:

  • Safari — trackpad pinch now zooms; page zoom is still suppressed.
  • Chrome — ctrl+wheel pinch and regular wheel zoom still work, no regression
    from the added listeners (Chrome doesn't fire GestureEvents).

Rebased on latest main; npm test passes (2221 + 235).

Added tests/client/InputHandlerGestureZoom.test.ts (12 tests): zoom direction
both ways, the ratio conversion checked against onZoom's formula,
cumulative-not-per-event scale, focal point, both guard paths, preventDefault,
and the pointer-lift transition below. jsdom has no GestureEvent, so the tests
fake one with a plain cancelable event — same approach as the existing
DisableSafariPinchZoom.test.ts.

Note for reviewers

iOS dispatches gesture events on top of the low-level per-touch events — Apple's
Safari Web Content Guide
describes GestureEvents as "also sent during a
multi-touch sequence", intended as an alternative to processing individual
touches. So onGestureChange bails out when two pointers are already tracked,
leaving that case to the existing pointer path rather than double-applying. It
still advances the cumulative scale before that guard, so if a pointer lifts
mid-gesture the next event measures from the last scale rather than re-applying
zoom the pointer path already handled.

Zoom was dead on macOS Safari with a trackpad. Chrome and Firefox
synthesize a ctrl+wheel event for a trackpad pinch, which
InputHandler.onScroll already handles; WebKit fires its non-standard
gesturestart/gesturechange/gestureend events instead, and nothing
converted those into a game zoom.

Handle the gesture events on the input overlay and convert them to a
ZoomEvent. GestureEvent.scale is cumulative from gesturestart, so the
per-event ratio is scale / lastGestureScale. onZoom divides the camera
scale by 1 + delta / DIVISOR, so inverting that reproduces the pinch
ratio exactly. That divisor is now exported as ZOOM_DELTA_DIVISOR so the
gesture math and the zoom sensitivity can't silently desync.

The scale is advanced before the multi-pointer guard: if a pointer lifts
mid-gesture, the next event must measure from the last scale, not from
gesturestart, or it re-applies zoom the pointer path already handled.
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Safari trackpad gestures are handled in InputHandler, converted into zoom events using a shared divisor, and applied by TransformHandler. Tests cover gesture lifecycle, scale conversion, pointer interaction, focal points, and default prevention.

Changes

Safari trackpad zoom

Layer / File(s) Summary
Gesture lifecycle and state
src/client/InputHandler.ts
Safari gesture events, cumulative scale tracking, default prevention, and blur/destroy cleanup are added.
Gesture zoom conversion
src/client/InputHandler.ts, src/client/TransformHandler.ts
Scale ratios produce ZoomEvent deltas, and both gesture handling and transform application use ZOOM_DELTA_DIVISOR.
Gesture zoom validation
tests/client/InputHandlerGestureZoom.test.ts
Tests cover scale mapping, lifecycle state, focal points, pointer pinch interaction, unchanged scale, and default prevention.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Safari
  participant InputHandler
  participant EventBus
  participant TransformHandler
  Safari->>InputHandler: Dispatch gesturestart
  Safari->>InputHandler: Dispatch gesturechange with scale and focal point
  InputHandler->>EventBus: Emit ZoomEvent
  EventBus->>TransformHandler: Deliver zoom delta
  TransformHandler->>TransformHandler: Apply zoom factor
  Safari->>InputHandler: Dispatch gestureend
Loading

Possibly related PRs

Suggested reviewers: evanpelle

Poem

Safari pinches, the canvas turns,
Scale becomes the zoom it earns.
Start and change, then gesture ends,
Pointer paths remain good friends.
A shared divisor keeps it bright.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy #4664 by converting Safari gesture events into zoom events on macOS trackpads.
Out of Scope Changes check ✅ Passed The added constant, handler updates, and tests are all aligned with the Safari pinch-zoom fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly summarizes the main change: fixing Safari trackpad pinch zoom.
Description check ✅ Passed The description matches the patch and explains the Safari pinch-zoom fix in relevant detail.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/client/InputHandlerGestureZoom.test.ts (1)

168-175: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test title contradicts its own setup.

The title says "which registers no pointers," but the test dispatches a pointerDown (line 169). Presumably this verifies that a single pointer stays below the 2-pointer multi-touch threshold so gesture zoom still fires — worth renaming to something like "still zooms when only one pointer is down (below the multi-touch threshold)" to avoid confusing future debugging.

✏️ Suggested title fix
-  it("still zooms for a trackpad pinch, which registers no pointers", () => {
+  it("still zooms when only one pointer is down (below multi-touch threshold)", () => {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/client/InputHandlerGestureZoom.test.ts` around lines 168 - 175, Rename
the test case containing dispatchPointerDown and the gesturestart/gesturechange
assertions to accurately describe that zoom still occurs with only one pointer
down, below the multi-touch threshold. Do not change the test setup or
expectations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/client/InputHandlerGestureZoom.test.ts`:
- Around line 168-175: Rename the test case containing dispatchPointerDown and
the gesturestart/gesturechange assertions to accurately describe that zoom still
occurs with only one pointer down, below the multi-touch threshold. Do not
change the test setup or expectations.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c1f85c52-b3fd-4fc9-858e-fe572a19fea9

📥 Commits

Reviewing files that changed from the base of the PR and between 1225324 and ff9adc3.

📒 Files selected for processing (3)
  • src/client/InputHandler.ts
  • src/client/TransformHandler.ts
  • tests/client/InputHandlerGestureZoom.test.ts

@landkirk
landkirk marked this pull request as ready for review July 24, 2026 21:22
@landkirk
landkirk requested a review from a team as a code owner July 24, 2026 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

Zoom doesn't work on Safari with trackpad

1 participant