[2.x] fix(realtime): only force-reconnect on iOS visibilitychange#4662
Merged
Conversation
PR #4654 added a `visibilitychange` listener that forces a fresh Pusher instance and refreshes the visible discussion list whenever the tab has been hidden for >5s. That was needed on iOS Safari, where backgrounding the page silently drops the WebSocket without firing `close`. On every other platform the WebSocket survives tab backgrounding fine — but the visibilitychange handler still fired, causing an unnecessary `GET /api/discussions` request and full list re-render every time the user switched away and back. Gate the visibilitychange-triggered `forceReconnect()` on `isIOS()` so the workaround only runs on the platform that actually needs it. The `pageshow(persisted=true)` path stays unconditional — bfcache restoration only fires on browsers that bfcached the page, and the WebSocket was definitely torn down by then regardless of platform. `isIOS()` is broader than the existing `isSafariMobile()` core utility because all iOS browsers use WebKit and share the same backgrounding pathology — iOS Chrome (`CriOS`) and iOS Firefox (`FxiOS`) are excluded by `isSafariMobile()` but still need this workaround. Regression from #4654.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#4654 added a
visibilitychangelistener that constructs a fresh Pusher instance and refreshes the visible discussion list whenever the tab has been hidden for more than 5 seconds. That workaround was needed on iOS Safari, where backgrounding silently drops the WebSocket without firingclose. On every other platform the WebSocket survives tab backgrounding fine — but the listener still fired unconditionally, so on desktop Firefox/Chrome (and Android) every "switch to another tab, come back" cycle triggered an unnecessaryGET /api/discussionsrequest and a full IndexPage refresh.Fix
Gate the
visibilitychange-triggeredforceReconnect()onisIOS(). Thepageshow(persisted=true)path stays unconditional — bfcache restore only happens on browsers that bfcached the page, and at that point the WebSocket was definitely torn down regardless of platform.A new
isIOS()utility is added rather than reusing the existingisSafariMobile()core helper, becauseisSafariMobile()excludes iOS Chrome (CriOS) and iOS Firefox (FxiOS) — both of which use WebKit on iOS and share the same backgrounding pathology this workaround was written for.isIOS()also detects iPadOS 13+ via theMacIntel+maxTouchPoints > 1quirk (iPadOS reportsnavigator.platform === 'MacIntel'but desktop Macs havemaxTouchPoints === 0).Test plan
GET /api/discussionsrequest fires. Discussion list does not visibly refresh. (Was happening before this fix.)lives: 2kill-switch #4597.pageshow(persisted=true)path is unchanged.Regression from #4654.