Fix reconnect on visibilitychange never running (window.hiddenProperty is always truthy) - #499
Open
TowyTowy wants to merge 1 commit into
Open
Conversation
`_onVisibilityChange()` guarded the reconnect with
`if (window.hiddenProperty) return;`. `window.hiddenProperty` is the
*name* of the visibility property (the string `'hidden'`, or a
vendor-prefixed variant), set by the polyfill in util.js. A non-empty
string is always truthy, so the guard always returns early and
`this._connect()` is dead code — the page never reconnects when it
becomes visible again.
Read the actual visibility state instead by indexing into `document`
with the property name, restoring the pre-polyfill behaviour
(`if (document.hidden) return;`) while keeping vendor-prefix support:
if (document[window.hiddenProperty]) return;
Now the guard returns early only while the page is hidden and lets
`_connect()` run when it becomes visible, which is the intended
recover-on-return path (notably for standalone PWAs whose socket was
severed while backgrounded).
Fixes schlagmichdoch#498
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
What & why
Fixes #498.
ServerConnection._onVisibilityChange()never reconnects the signaling socket when the page returns to the foreground:window.hiddenProperty(set inutil.js) is the name of the visibility property — the string'hidden'(or a vendor-prefixed variant), not the page's visibility state. A non-empty string is always truthy, so the guard always returns early andthis._connect()is dead code: the client never re-establishes the WebSocket onvisibilitychange.git log -Lshows this was introduced in2a837eb("add 'visbilitychange' event support for older browsers"), which changedif (document.hidden) return;toif (window.hiddenProperty) return;and dropped thedocument[...]indexing.Fix
Index
documentwith the property name, restoring the original pre-polyfill behaviour while keeping vendor-prefix support:The guard now returns early only while the page is actually hidden and lets
_connect()run when it becomes visible — the intended recover-on-return path. This most visibly affects standalone PWAs (e.g. iPadOS home-screen web apps) whose socket is severed while backgrounded, which is the symptom reported in #498.Verification
node --check public/scripts/network.jspasses.util.jspolyfill + both guards: hidden page →_connect()correctly skipped (before and after); visible page → skipped before the fix (the bug), called after (correct); vendor-prefixed (webkitHidden) visible page → correctly calls_connect()after the fix.hiddenProperty === nullpath (no visibility API) is unaffected:visibilityChangeEventis then alsonull, so the listener never fires.Out of scope (possible follow-up)
Issue #498 also notes that on iPadOS the resumed socket can be a "zombie" whose
readyStatestill readsOPEN, so_connect()'s_isConnected()early-return may skip reconnecting even after this fix, since the client has no self-initiated ping timeout. Robustly handling that changes cross-platform behaviour and can't be verified without the physical device, so I've left it out of this minimal, deterministic fix.Disclosure: this one-line fix was prepared with AI assistance (Claude); the root cause, the change, and the verification above were reviewed before submission.