fix(client): block Safari page-level pinch-zoom#3901
Conversation
iOS Safari has ignored the `user-scalable=no` viewport hint since iOS 10, so two-finger pinch still zooms the whole page and can softlock the in-game HUD. Intercept WebKit's non-standard `gesturestart`, `gesturechange` and `gestureend` events at `document` and call `preventDefault()` so the page stays put. The game's own pinch-to-zoom on the map canvas is driven by pointer events (InputHandler) and is unaffected; browsers that do not fire GestureEvent treat the listeners as a no-op. Resolves openfrontio#2330
|
|
WalkthroughThis PR introduces a Safari-specific pinch-zoom blocker to prevent iOS Safari's native gesture zooming, which can interfere with the HUD. A new utility function registers ChangesSafari Pinch-Zoom Prevention
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/client/utilities/DisableSafariPinchZoom.ts (1)
23-40: Clean implementation that correctly blocks Safari gesture zoom.The function properly:
- Captures the
blockhandler reference for both registration and cleanup- Returns a disposer that removes the exact same listeners
- Defaults to
documentscope (where Safari decides page zoom) while allowing custom targetsThe code will work correctly as-is. If you want to be explicit about the listener options (and avoid potential future console warnings), you could add
{ passive: false }when registering:Optional: Make non-passive intent explicit
const events = ["gesturestart", "gesturechange", "gestureend"] as const; for (const type of events) { - target.addEventListener(type, block); + target.addEventListener(type, block, { passive: false }); } return () => { for (const type of events) { - target.removeEventListener(type, block); + target.removeEventListener(type, block, { passive: false }); } };This is not required—GestureEvent defaults to non-passive—but makes the intent clearer.
🤖 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 `@src/client/utilities/DisableSafariPinchZoom.ts` around lines 23 - 40, Add explicit listener options to make non-passive intent clear: in installSafariPinchZoomBlocker capture an options object like const opts = { passive: false } and pass opts to target.addEventListener(type, block, opts) for each event, and use the same opts when calling target.removeEventListener(type, block, opts) in the returned disposer; this ensures the non-passive intent is explicit and removal matches the registered listeners (referencing installSafariPinchZoomBlocker, block, and events).
🤖 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 `@src/client/utilities/DisableSafariPinchZoom.ts`:
- Around line 23-40: Add explicit listener options to make non-passive intent
clear: in installSafariPinchZoomBlocker capture an options object like const
opts = { passive: false } and pass opts to target.addEventListener(type, block,
opts) for each event, and use the same opts when calling
target.removeEventListener(type, block, opts) in the returned disposer; this
ensures the non-passive intent is explicit and removal matches the registered
listeners (referencing installSafariPinchZoomBlocker, block, and events).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e1c1328a-1bbb-4994-bbe9-ef1f759a3921
📒 Files selected for processing (3)
src/client/Main.tssrc/client/utilities/DisableSafariPinchZoom.tstests/client/DisableSafariPinchZoom.test.ts
| return () => { | ||
| for (const type of events) { | ||
| target.removeEventListener(type, block); | ||
| } | ||
| }; |
There was a problem hiding this comment.
seems like this is never called? any reason to return it?
|
@vansszh I have a fix ready, looks like you missed my comment on the original issue. The fix looks solid at first glance so feel free to continue |
| return () => { | ||
| for (const type of events) { | ||
| target.removeEventListener(type, block); | ||
| } | ||
| }; |
github-project-automation
Bot
moved this from Triage
to Final Review
in OpenFront Release Management
github-project-automation
Bot
moved this from Final Review
to Complete
in OpenFront Release Management
4 participants
iOS Safari has ignored the
user-scalable=noviewport hint since iOS10, so two-finger pinch still zooms the whole page and can softlock the
in-game HUD. Intercept WebKit's non-standard
gesturestart,gesturechangeandgestureendevents atdocumentand callpreventDefault()so the page stays put. The game's own pinch-to-zoomon the map canvas is driven by pointer events (InputHandler) and is
unaffected; browsers that do not fire GestureEvent treat the listeners
as a no-op.
Resolves #2330
If this PR fixes an issue, link it below. If not, delete these two lines.
Resolves #(issue number)
Description:
Describe the PR.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
DISCORD_USERNAME