Fix: Add fallback values to navigator.connection to prevent freezing on browsers that don't support it out of the box #4572
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAlias navigator.connection and use optional chaining with default fallbacks for effectiveType, downlink, and rtt; update showNetworkStatus to read from the alias and render the fallback/formatted values. Decision logic (offline/slow/online) is unchanged. Note: a likely typo uses an undefined Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant setup.js
participant UI
Note over Browser,setup.js #E8F1FF: read network info (safe)
Browser->>setup.js: navigator.connection
setup.js->>setup.js: const connection = navigator.connection\nuse connection?.effectiveType || "Unknown"\nuse connection?.downlink || "Unknown"\nuse connection?.rtt || "Unknown"
alt Data present
setup.js->>UI: update network-type, network-downlink, network-rtt (formatted)
else Missing data
setup.js->>UI: update fields with "Unknown" (or formatted "Unknown" strings)
end
Note right of setup.js #FFF4E5: decision logic (offline/slow/online) unchanged
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/js/tabs/setup.js (2)
500-503: Don’t default missing downlink/rtt to 0 — it misclassifies “unknown” as “slow”.Setting numeric fallbacks to 0 makes the slow/online decision treat unsupported/unknown values as slow, while the UI shows “Unknown”. Use presence checks and avoid coercing to 0; also consider separating connection.type (for “none”) from effectiveType (for “2g/3g/4g”) to keep the logic correct.
Apply this diff to make the values presence-aware and add legacy prefixes:
- const connection = navigator.connection; - const type = connection?.effectiveType || "unknown"; - const downlink = connection?.downlink || 0; - const rtt = connection?.rtt || 0; + const connection = + navigator.connection || navigator.mozConnection || navigator.webkitConnection; + // effectiveType is used for "slow-2g/2g/3g/4g" classification + const effectiveType = connection?.effectiveType ?? "unknown"; + // connection.type is used for "none"/wifi/cellular/etc. + const connType = connection?.type ?? "unknown"; + const downlink = typeof connection?.downlink === "number" ? connection.downlink : undefined; + const rtt = typeof connection?.rtt === "number" ? connection.rtt : undefined;Then adjust the decision logic to use the new vars (outside the changed hunk, shown here for clarity):
if (!networkStatus || !navigator.onLine || connType === "none") { statusText = i18n.getMessage("initialSetupNetworkInfoStatusOffline"); } else if ( effectiveType === "slow-2g" || effectiveType === "2g" || (downlink !== undefined && downlink < 0.115) || (rtt !== undefined && rtt > 1000) ) { statusText = i18n.getMessage("initialSetupNetworkInfoStatusSlow"); } else { statusText = i18n.getMessage("initialSetupNetworkInfoStatusOnline"); }This preserves “unknown” as unknown (not slow), and keeps “offline” detection tied to connection.type === "none" and navigator.onLine.
515-517: Truthiness check hides valid 0 values; reuse computed vars and prefer i18n for “Unknown”.Using
connection?.downlink ? ... : "Unknown"treats 0 as unknown. Reuse the computed vars and useNumber.isFiniteto decide display. Also, keep casing consistent and consider localizing “Unknown”.Apply this diff:
- $(".network-type").text(connection?.effectiveType || "Unknown"); - $(".network-downlink").text(connection?.downlink ? `${connection.downlink} Mbps` : "Unknown"); - $(".network-rtt").text(connection?.rtt ? `${connection.rtt} ms` : "Unknown"); + // If an i18n key exists (e.g., "initialSetupNetworkInfoUnknown"), prefer that over the literal "Unknown". + const unknownLabel = "Unknown"; // or i18n.getMessage("initialSetupNetworkInfoUnknown") + $(".network-type").text(effectiveType !== "unknown" ? effectiveType : unknownLabel); + $(".network-downlink").text(Number.isFinite(downlink) ? `${downlink} Mbps` : unknownLabel); + $(".network-rtt").text(Number.isFinite(rtt) ? `${rtt} ms` : unknownLabel);Optional follow-up: If you keep
typeas the name, consider aligning the capitalization: either store"Unknown"or map"unknown"to the display label consistently.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/js/tabs/setup.js(2 hunks)
Co-authored-by: Mark Haslinghuis <[email protected]>
Co-authored-by: Mark Haslinghuis <[email protected]>
|
|
Preview URL: https://f67edc86.betaflight-configurator.pages.dev |
This should address usability issues with Brave and potentially other browsers that focus on user privacy by disabling these APIs
Summary by CodeRabbit