-
-
Notifications
You must be signed in to change notification settings - Fork 617
Conversation
Any url from a browser bookmark should certainly be opened with a browser This means it will now support urls like chrome://settings as OpenUrl only passed http/https urls to the browser
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 1 file
📝 WalkthroughWalkthrough
ChangesBookmark result construction
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs`:
- Around line 118-122: The bookmark action handler in Main.cs can throw when
Context.API.OpenWebUrl receives a null, relative, or malformed bookmark.Url.
Update the Action callback for the bookmark item to wrap the OpenWebUrl call in
a try-catch, catch the URI parsing failure, and log the error instead of letting
the exception escape. Use the existing bookmark.Url and Context.API.OpenWebUrl
symbols to locate the handler and keep the failure path graceful.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
i️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 795f8b44-4716-4111-be3b-e1d7d2043264
📒 Files selected for processing (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -nP -C4 '\bpublic void OpenUrl\b' --type=cs
Repository: Flow-Launcher/Flow.Launcher
Length of output: 165
Guard against malformed bookmark URLs in action handler
Context.API.OpenWebUrl internally instantiates new Uri(url), which throws UriFormatException for null, relative, or malformed URLs. Since bookmark.Url is untrusted user data, the current action handler lacks protection, causing an uncaught exception if a bookmark is corrupt. Wrap the call in a try-catch to gracefully handle failures and log the error.
Recommended change
Action = _ =>
{
- Context.API.OpenWebUrl(bookmark.Url);
- return true;
+ try
+ {
+ Context.API.OpenWebUrl(bookmark.Url);
+ return true;
+ }
+ catch (Exception e)
+ {
+ // Log the malformed URL and return false to hide the exception from the user
+ Context.API.LogException("BrowserBookmark", $"Failed to open bookmark: {bookmark.Url}", e);
+ return false;
+ }
},📝 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.
🤖 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 `@Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs` around lines 118 - 122,
The bookmark action handler in Main.cs can throw when Context.API.OpenWebUrl
receives a null, relative, or malformed bookmark.Url. Update the Action callback
for the bookmark item to wrap the OpenWebUrl call in a try-catch, catch the URI
parsing failure, and log the error instead of letting the exception escape. Use
the existing bookmark.Url and Context.API.OpenWebUrl symbols to locate the
handler and keep the failure path graceful.
Uh oh!
There was an error while loading. Please reload this page.
Use OpenWebUrl instead of OpenUrl in Browser Bookmark plugin - all bookmarks coming from a browser should be opened in a browser. This means schemes like chrome:// will also be supported now in bookmarks.
Also extracted out the result creation to its own method as it was mostly duplicated code.
Summary by cubic
Bookmarks now always open in your default browser, including browser-specific URLs like chrome://. Also refactors bookmark result creation to a single helper for consistency.
Summary of changes
Release Note
Opening bookmarks now always uses your browser and supports special browser pages like Settings.
Written for commit 05880da. Summary will update on new commits.