Fix external links/buttons by using opener plugin#127
Conversation
📝 WalkthroughWalkthroughThe pull request introduces centralized external URL handling to resolve non-functioning external links. A new utility module provides two functions: Changes
Assessment against linked issues
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
web-app/src/components/AppFooter.tsxweb-app/src/components/setting-sidebar/setting-sidebar.tsxweb-app/src/lib/openExternalUrl.tsweb-app/src/lib/version.ts
| const openedWindow = window.open(url, '_blank', 'noopener,noreferrer') | ||
| if (openedWindow) { | ||
| openedWindow.opener = null | ||
| } |
There was a problem hiding this comment.
openedWindow.opener = null is unreachable in modern browsers; also consider acknowledging popup-blocked failures.
Two related points:
-
Per the MDN spec and the Firefox implementation, if you use the
noopenerfeature,window.open()will returnnulleven if the window got opened – not only can the new window not talk to you, but you can't talk to it either. Becausenoopeneris already in the features string on Line 9,openedWindowis alwaysnullin any modern browser, so theif (openedWindow)branch and theopener = nullassignment on Line 11 are dead code. -
nullis returned if the browser fails to open the new browsing context, for example because it was blocked by a browser popup blocker. In that case, the failure is silently swallowed. Consider logging or surfacing the event so it is at least observable.
♻️ Suggested cleanup
- const openedWindow = window.open(url, '_blank', 'noopener,noreferrer')
- if (openedWindow) {
- openedWindow.opener = null
- }
+ const opened = window.open(url, '_blank', 'noopener,noreferrer')
+ if (!opened) {
+ console.warn('openExternalUrl: window.open was blocked or failed for', url)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const openedWindow = window.open(url, '_blank', 'noopener,noreferrer') | |
| if (openedWindow) { | |
| openedWindow.opener = null | |
| } | |
| const opened = window.open(url, '_blank', 'noopener,noreferrer') | |
| if (!opened) { | |
| console.warn('openExternalUrl: window.open was blocked or failed for', url) | |
| } |
|
Thank you for this contribution @Avazbek22 !! |
Description
Fixes #125 by making external links/buttons open reliably in the desktop app.
What changed
@tauri-apps/plugin-opener(openUrl) when running in Tauri.window.openfor non-Tauri/web environments.PRIVACYtoPRIVACY.md.Why
In the desktop environment, plain anchor links (
target="_blank") may not consistently open in the system browser. Routing link opening through the opener plugin makes behavior consistent across platforms.Checklist
npm run lintbefore raising this PRnpm run formatbefore raising this PR