Reuse existing tabs in bookmarks plugin #4189
Conversation
|
🥷 Code experts: Jack251970 Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: ✨ Comment |
|
This PR contains a TODO statement. Please check to see if they should be removed. |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
This TODO is a placeholder for possible future enhancement. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a "Reuse tabs" feature to BrowserBookmark: new TabsReservationService, TabsTracker, TabsCache, and TabsEventsDispatcher integrate with UI Automation and settings to detect, register, and activate existing browser tabs when opening bookmarks. Changes
Sequence DiagramsequenceDiagram
autonumber
participant User
participant Main as Main (Plugin)
participant TabsRes as TabsReservationService
participant Dispatcher as TabsEventsDispatcher
participant Tracker as TabsTracker
participant UIAuto as UIAutomation
participant Browser
User->>Main: Select bookmark (ReuseTabs enabled)
Main->>TabsRes: OpenUrlAndTrack(url)
TabsRes->>Tracker: RegisterToken / TryToResolveToken(token)
alt token resolved immediately
Tracker-->>TabsRes: AutomationElement (tab)
TabsRes->>UIAuto: Activate tab element
UIAuto->>Browser: Switch to tab
else token pending
TabsRes->>Dispatcher: Enqueue(url, token)
Dispatcher->>Tracker: Poll TryToResolveToken (retry loop)
Tracker-->>Dispatcher: AutomationElement when found
Dispatcher->>TabsRes: RegisterTab(url, trackingInfo, tab)
TabsRes->>UIAuto: Activate tab element
end
alt no matching tab found within timeout
TabsRes->>Browser: Open URL (new tab)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md (1)
28-31: Add language identifier to the code block.The fenced code block should specify a language for proper syntax highlighting.
🔎 Proposed fix
-``` +```shell dotnet tool install --global dotnet-project-licenses dotnet-project-licenses --input Flow.Launcher.Plugin.BrowserBookmark.csproj --json</details> </blockquote></details> <details> <summary>Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.cs (1)</summary><blockquote> `17-23`: **Consider renaming `Empty()` to `IsEmpty` property for idiomatic C#.** C# conventions typically use properties for state queries without side effects. <details> <summary>🔎 Proposed fix</summary> ```diff - public bool Empty() + public bool IsEmpty { - lock (sync) + get { - return _knownTabs.Count == 0; + lock (sync) + { + return _knownTabs.Count == 0; + } } }Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (1)
19-19: Consider makingTabsTrackerinternal for consistency.Other classes in the
Tabsnamespace (TabsCache,TabsWalker,TabsDebug) areinternal. SinceTabsTrackeris only used within the plugin, making itinternalwould maintain consistent encapsulation.🔎 Proposed fix
-public class TabsTracker : IDisposable +internal class TabsTracker : IDisposable
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Flow.Launcher.Plugin.BrowserBookmark.csprojPlugins/Flow.Launcher.Plugin.BrowserBookmark/Languages/en.xamlPlugins/Flow.Launcher.Plugin.BrowserBookmark/Languages/pl.xamlPlugins/Flow.Launcher.Plugin.BrowserBookmark/Main.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Models/Settings.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.jsonPlugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.mdPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.mdPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsDebug.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Views/SettingsControl.xamlPlugins/Flow.Launcher.Plugin.BrowserBookmark/Views/SettingsControl.xaml.cs
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-10-16T14:32:00.580Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3770
File: Flow.Launcher.Infrastructure/UserSettings/Settings.cs:186-205
Timestamp: 2025-10-16T14:32:00.580Z
Learning: In Flow.Launcher.Infrastructure/UserSettings/Settings.cs, the default hotkey assignments SelectNextPageHotkey = "PageUp" and SelectPrevPageHotkey = "PageDown" are intentionally designed this way and should not be flagged as reversed. This is the intended UX convention for Flow Launcher's page navigation.
Applied to files:
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Views/SettingsControl.xaml.cs
📚 Learning: 2025-09-05T11:56:27.267Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/HotkeyControlDialog.xaml:6-6
Timestamp: 2025-09-05T11:56:27.267Z
Learning: In Flow.Launcher's migration to iNKORE.UI.WPF.Modern UI framework, dialog resource keys like PopuBGColor, PopupButtonAreaBGColor, PopupButtonAreaBorderColor, and PopupTextColor are provided by the iNKORE.UI.WPF.Modern framework itself, not defined locally in the codebase theme files.
Applied to files:
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Languages/en.xamlPlugins/Flow.Launcher.Plugin.BrowserBookmark/Languages/pl.xaml
📚 Learning: 2025-09-04T11:52:29.096Z
Learnt from: jjw24
Repo: Flow-Launcher/Flow.Launcher PR: 3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.096Z
Learning: In Flow Launcher's PluginsManifest.cs, when dealing with version parsing for the MinimumAppVersion feature, maintainer jjw24 prefers to keep the solution simple rather than implementing comprehensive helper methods for SemVer parsing normalization.
Applied to files:
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.mdPlugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.jsonPlugins/Flow.Launcher.Plugin.BrowserBookmark/Flow.Launcher.Plugin.BrowserBookmark.csproj
📚 Learning: 2024-10-08T15:52:58.573Z
Learnt from: taooceros
Repo: Flow-Launcher/Flow.Launcher PR: 2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Applied to files:
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.mdPlugins/Flow.Launcher.Plugin.BrowserBookmark/Flow.Launcher.Plugin.BrowserBookmark.csproj
📚 Learning: 2025-09-13T06:04:26.977Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
Applied to files:
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Views/SettingsControl.xaml
📚 Learning: 2025-09-11T08:30:29.487Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml:0-0
Timestamp: 2025-09-11T08:30:29.487Z
Learning: In Flow.Launcher's SettingsPaneTheme.xaml theme editor panel, the inner ui:ScrollViewerEx within the Border (Grid.Column="1") is intentionally kept because its height cannot be expanded to the whole page due to layout constraints. This serves a different scrolling purpose than the outer ScrollViewerEx and is necessary for the theme editor panel functionality.
Applied to files:
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Views/SettingsControl.xaml
🧬 Code graph analysis (4)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Views/SettingsControl.xaml.cs (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (1)
ReloadAllBookmarks(209-215)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.cs (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs (1)
IEnumerable(24-45)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsDebug.cs (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (1)
Main(17-279)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs (4)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (1)
Main(17-279)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (1)
TabsTracker(19-163)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.cs (5)
TabsCache(10-51)Contains(44-50)Empty(17-23)Add(25-31)Add(33-42)Flow.Launcher.Infrastructure/Stopwatch.cs (1)
Stopwatch(8-59)
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md
[warning] 11-11:
Skia is not a recognized word. (unrecognized-spelling)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.json
[warning] 82-82:
Wies is not a recognized word. (unrecognized-spelling)
[warning] 78-78:
Skia is not a recognized word. (unrecognized-spelling)
[warning] 68-68:
Skia is not a recognized word. (unrecognized-spelling)
[warning] 68-68:
Skia is not a recognized word. (unrecognized-spelling)
🪛 LanguageTool
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.md
[grammar] ~10-~10: Use a hyphen to join words.
Context: ... place. I strongly believe in a higher level concept of **"just take me to THIS...
(QB_NEW_EN_HYPHEN)
[grammar] ~20-~20: Use a hyphen to join words.
Context: ...here all is slow even if you have a high end laptop - you won't believe it until ...
(QB_NEW_EN_HYPHEN)
[style] ~27-~27: Consider removing “of” to be more concise
Context: ...S place in milliseconds"** works almost all of the time so it bring so much value that it ...
(ALL_OF_THE)
🪛 markdownlint-cli2 (0.18.1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md
7-7: Bare URL used
(MD034, no-bare-urls)
8-8: Bare URL used
(MD034, no-bare-urls)
9-9: Bare URL used
(MD034, no-bare-urls)
10-10: Bare URL used
(MD034, no-bare-urls)
11-11: Bare URL used
(MD034, no-bare-urls)
12-12: Bare URL used
(MD034, no-bare-urls)
28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (10)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.json (1)
1-92: LGTM with a minor note on Repository metadata.The third-party notices are well-structured and comprehensive. The empty
TypeandCommitfields in some Repository objects (lines 12, 14) are acceptable, though for consistency you could populate them where available (e.g., BrowserTabs Repository.Type could be "git").Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md (1)
1-31: Nice documentation!The third-party notices documentation is thorough and helpful. Including the generation instructions is a great touch for future maintainability. The additional credits section provides useful context about the Browser Tabs plugin influence.
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Views/SettingsControl.xaml.cs (1)
51-59: LGTM!The
ReuseTabsproperty implementation correctly follows the same pattern as other bookmark-reloading settings (LoadChromeBookmark,LoadFirefoxBookmark,LoadEdgeBookmark). The background reload ensures the bookmark cache is refreshed when the setting changes.Plugins/Flow.Launcher.Plugin.BrowserBookmark/Models/Settings.cs (1)
16-16: LGTM!The
ReuseTabsproperty is well-implemented with a sensible default offalse, making it an opt-in experimental feature. This follows the same pattern as other boolean settings in the class.Plugins/Flow.Launcher.Plugin.BrowserBookmark/Languages/pl.xaml (1)
32-32: LGTM!The Polish localization entry is properly structured and follows the naming convention of other keys in the file. The experimental nature of the feature is clearly indicated.
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Languages/en.xaml (1)
34-34: LGTM!The English localization entry is clear and properly marks the feature as experimental, setting appropriate user expectations for this new functionality.
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs (2)
62-103: Blocking retry loop may delay UI Automation event processing.The retry loop can block for up to 4 seconds. While this runs on a UI Automation background thread, prolonged blocking could delay other focus events. Consider whether async processing or shorter timeouts would be beneficial for responsiveness.
74-84: Good defensive design with documented limitations.The first-run heuristic (taking the last tab) and the TODO comment appropriately acknowledge the inherent limitations. The approach is reasonable given browser API constraints.
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsDebug.cs (1)
1-73: Debug utility implementation looks reasonable.The recursive traversal with exception handling is appropriate for a debugging tool. The filtering options and detailed logging will be useful for diagnosing UI Automation tree issues.
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (1)
87-97: Consider queuing expected URLs for rapid bookmark openings.If a user opens multiple bookmarks quickly, the current implementation overwrites the previous expected URL. While the error logging is helpful, this could result in missed tab mappings. For the current "best effort" approach documented in README, this is acceptable.
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Flow.Launcher.Plugin.BrowserBookmark.csproj
Show resolved
Hide resolved
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Flow.Launcher.Plugin.BrowserBookmark.csproj
Show resolved
Hide resolved
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs
Outdated
Show resolved
Hide resolved
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs
Outdated
Show resolved
Hide resolved
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs
Outdated
Show resolved
Hide resolved
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Views/SettingsControl.xaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md (1)
28-32: Add language identifier to code fence.The code block should specify a language identifier for proper syntax highlighting.
🔎 Proposed fix
-``` +```bash cd Plugins\Flow.Launcher.Plugin.BrowserBookmark dotnet tool install --global dotnet-project-licenses dotnet-project-licenses --input Flow.Launcher.Plugin.BrowserBookmark.csproj --json ```
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.jsonPlugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-04T11:52:29.096Z
Learnt from: jjw24
Repo: Flow-Launcher/Flow.Launcher PR: 3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.096Z
Learning: In Flow Launcher's PluginsManifest.cs, when dealing with version parsing for the MinimumAppVersion feature, maintainer jjw24 prefers to keep the solution simple rather than implementing comprehensive helper methods for SemVer parsing normalization.
Applied to files:
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md
📚 Learning: 2024-10-08T15:52:58.573Z
Learnt from: taooceros
Repo: Flow-Launcher/Flow.Launcher PR: 2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Applied to files:
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.json
[warning] 93-93:
Wies is not a recognized word. (unrecognized-spelling)
[warning] 88-88:
Skia is not a recognized word. (unrecognized-spelling)
[warning] 78-78:
Skia is not a recognized word. (unrecognized-spelling)
[warning] 78-78:
Skia is not a recognized word. (unrecognized-spelling)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md
[warning] 11-11:
Skia is not a recognized word. (unrecognized-spelling)
🪛 markdownlint-cli2 (0.18.1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md
7-7: Bare URL used
(MD034, no-bare-urls)
8-8: Bare URL used
(MD034, no-bare-urls)
9-9: Bare URL used
(MD034, no-bare-urls)
10-10: Bare URL used
(MD034, no-bare-urls)
11-11: Bare URL used
(MD034, no-bare-urls)
12-12: Bare URL used
(MD034, no-bare-urls)
28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: gitStream.cm
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.json
Outdated
Show resolved
Hide resolved
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (1)
65-65: Consider adding error handling for TabsTracker initialization.The
Init()call subscribes to UI Automation events that could potentially fail. While consistent with the file's pattern (e.g.,LoadBookmarksIfEnabledis also unwrapped), wrapping it in try-catch would improve robustness.🔎 Suggested error handling
- _tabsTracker.Init(); + try + { + _tabsTracker.Init(); + } + catch (Exception e) + { + Context.API.LogException(ClassName, "Failed to initialize tab tracker.", e); + }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs
🧰 Additional context used
🧬 Code graph analysis (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (4)
TabsTracker(19-163)Init(71-79)OpenUrlAndTrack(32-40)Dispose(81-85)
🔇 Additional comments (2)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (2)
100-119: LGTM: TabsTracker integration in Query method is clean and consistent.The integration correctly:
- Wraps bookmark results with
InjectExistingTabsto map existing tabs when reuse is enabled- Replaces direct URL opening with
OpenUrlAndTrackto enable tab tracking- Applies changes consistently across both query paths (with and without search parameters)
- Passes
_settingsto respect the ReuseTabs configurationAlso applies to: 123-142
268-268: Excellent: Resource leak resolved.The TabsTracker is now properly disposed, addressing the previous review concern about leaking UI Automation event handlers. The null-conditional operator provides defensive safety, though the readonly field is always initialized.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.md (1)
27-27: Minor: Improve conciseness and fix verb tense.Line 27 has two refinements:
- Remove "of" for more concise phrasing: "works almost all the time" instead of "works almost all of the time"
- Fix verb tense: "worthwhile to accept" instead of "worthwhile to accepts"
🔎 Proposed fix
-Still **"just take me to THIS place in milliseconds"** works almost all of the time so it brings so much value that it is worthwhile to accepts the fact it fails sometimes. +Still **"just take me to THIS place in milliseconds"** works almost all the time so it brings so much value that it is worthwhile to accept the fact it fails sometimes.
🧹 Nitpick comments (2)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md (2)
7-12: Wrap license URLs in markdown link syntax.The bare URLs in the license table should be formatted as markdown links to comply with markdown style guidelines (MD034).
🔎 Proposed fix
| Reference | Version | License Type | License | |-----------------------------|---------|--------------|---------------------------------------| -| BrowserTabs | 0.2.0 | Apache-2.0 | https://licenses.nuget.org/Apache-2.0 | -| CommunityToolkit.Mvvm | 8.4.0 | MIT | https://licenses.nuget.org/MIT | -| Flow.Launcher.Localization | 0.0.6 | MIT | https://licenses.nuget.org/MIT | -| Microsoft.Data.Sqlite | 10.0.1 | MIT | https://licenses.nuget.org/MIT | -| SkiaSharp | 3.119.1 | MIT | https://licenses.nuget.org/MIT | -| Svg.Skia | 3.2.1 | MIT | https://licenses.nuget.org/MIT | +| BrowserTabs | 0.2.0 | Apache-2.0 | [Apache-2.0](https://licenses.nuget.org/Apache-2.0) | +| CommunityToolkit.Mvvm | 8.4.0 | MIT | [MIT](https://licenses.nuget.org/MIT) | +| Flow.Launcher.Localization | 0.0.6 | MIT | [MIT](https://licenses.nuget.org/MIT) | +| Microsoft.Data.Sqlite | 10.0.1 | MIT | [MIT](https://licenses.nuget.org/MIT) | +| SkiaSharp | 3.119.1 | MIT | [MIT](https://licenses.nuget.org/MIT) | +| Svg.Skia | 3.2.1 | MIT | [MIT](https://licenses.nuget.org/MIT) |
28-28: Specify language for fenced code block.The code block should declare a language identifier per markdown style guidelines (MD040). The commands appear to be shell/bash syntax.
🔎 Proposed fix
-``` +```bash cd Plugins\Flow.Launcher.Plugin.BrowserBookmark dotnet tool install --global dotnet-project-licenses dotnet-project-licenses --input Flow.Launcher.Plugin.BrowserBookmark.csproj --json</details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between ecfe8c302dd5b261c5e5c0dd2670b82910c9ed5a and 944e3d74a34fa073a0faf09e1e5c84229b65f837. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md` * `Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.md` </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (2)</summary> <details> <summary>📚 Learning: 2025-09-04T11:52:29.096Z</summary>Learnt from: jjw24
Repo: Flow-Launcher/Flow.Launcher PR: 3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.096Z
Learning: In Flow Launcher's PluginsManifest.cs, when dealing with version parsing for the MinimumAppVersion feature, maintainer jjw24 prefers to keep the solution simple rather than implementing comprehensive helper methods for SemVer parsing normalization.**Applied to files:** - `Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md` </details> <details> <summary>📚 Learning: 2024-10-08T15:52:58.573Z</summary>Learnt from: taooceros
Repo: Flow-Launcher/Flow.Launcher PR: 2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in theFlow.Launcher.csprojfile is dynamically updated during the CI/CD process.**Applied to files:** - `Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md` </details> </details><details> <summary>🪛 GitHub Check: Check Spelling</summary> <details> <summary>Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md</summary> [warning] 11-11: `Skia` is not a recognized word. (unrecognized-spelling) </details> </details> <details> <summary>🪛 LanguageTool</summary> <details> <summary>Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.md</summary> [style] ~27-~27: Consider removing “of” to be more concise Context: ...S place in milliseconds"** works almost all of the time so it brings so much value that it... (ALL_OF_THE) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.md</summary> 7-7: Bare URL used (MD034, no-bare-urls) --- 8-8: Bare URL used (MD034, no-bare-urls) --- 9-9: Bare URL used (MD034, no-bare-urls) --- 10-10: Bare URL used (MD034, no-bare-urls) --- 11-11: Bare URL used (MD034, no-bare-urls) --- 12-12: Bare URL used (MD034, no-bare-urls) --- 28-28: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)</summary> * GitHub Check: gitStream.cm </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.md (1)</summary><blockquote> `10-10`: **✅ Grammar and style corrections from previous review have been applied.** The following improvements from the prior review are now in place: - Line 10: "higher-level concept" (correctly hyphenated) - Line 20: "high-end laptop" (correctly hyphenated) - Line 27: "it brings so much value" (correct subject-verb agreement) Also applies to: 20-20, 27-27 </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs (1)
55-56: Add parameter validation for defensive coding.The public method
GetCurrentTabFromWindowlacks null checks formainWindowandprocessparameters, which could lead toNullReferenceExceptionif callers pass null values.🔎 Proposed fix
public BrowserTab GetCurrentTabFromWindow(AutomationElement mainWindow, Process process, CancellationToken cancellationToken) { + if (mainWindow == null) + throw new ArgumentNullException(nameof(mainWindow)); + if (process == null) + throw new ArgumentNullException(nameof(process)); + try {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.jsonPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs
🧰 Additional context used
🧬 Code graph analysis (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.cs (5)
TabsCache(10-51)Contains(44-50)Empty(17-23)Add(25-31)Add(33-42)
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.json
[warning] 93-93:
Wies is not a recognized word. (unrecognized-spelling)
[warning] 88-88:
Skia is not a recognized word. (unrecognized-spelling)
[warning] 78-78:
Skia is not a recognized word. (unrecognized-spelling)
[warning] 78-78:
Skia is not a recognized word. (unrecognized-spelling)
🔇 Additional comments (5)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/THIRD_PARTY_NOTICES.json (2)
1-104: Well-structured attribution file with previous issues addressed.The file is well-organized with complete metadata for all dependencies. Previous review comments regarding empty fields for BrowserTabs and Flow.Launcher.Localization have been properly addressed.
Note: The spelling warnings from static analysis for "Wiesław" (author name) and "Skia" (library name) are false positives and can be safely ignored.
4-4: All package versions documented in THIRD_PARTY_NOTICES.json match the versions specified in the .csproj file. No changes required.Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs (3)
19-19: Past issue resolved.The
ClassNamefield is now correctly set tonameof(TabsWalker). The copy-paste error flagged in the previous review has been fixed.
79-82: TODO comment is intentional per PR objectives.The TODO on line 82 regarding browser settings research is a placeholder for future enhancement, as noted in the PR objectives. This is expected and should remain.
55-116: No issues found. The single call toGetCurrentTabFromWindowinTabsTracker.cs(line 144) properly checks for null returns before using the value (line 145:if (currentTab != null)).
…out synchronization.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsFocusEventDispatcher.cs (1)
65-65: Consider passing cancellation token for better shutdown responsiveness.
CancellationToken.Noneis passed toGetCurrentTabFromWindow, which means the worker thread will complete the full 5-second retry timeout even during disposal. Consider passing_cts.Tokeninstead to allow faster shutdown.🔎 Proposed change
- var currentTab = _walker.GetCurrentTabFromWindow(rootElement, process, CancellationToken.None); + var currentTab = _walker.GetCurrentTabFromWindow(rootElement, process, _cts.Token);Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (1)
74-83: Consider protecting Init() with a lock for complete thread-safety.The
_initializedflag is checked and set without synchronization. WhileInit()is likely called only once during plugin initialization, adding a lock would prevent potential issues if called concurrently and is consistent with the thread-safety approach used elsewhere.🔎 Proposed change
public void Init() { + lock (_sync) + { if (_initialized) return; _focusHandlerDispatcher = new(_walker, this); _focusHandler = OnFocusChanged; Automation.AddAutomationFocusChangedEventHandler(_focusHandler); _initialized = true; + } }Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.md (1)
27-28: Minor style improvements for clarity.Two small refinements flagged by static analysis:
- Line 27: Add article before "user" for better grammar
- Line 28: Simplify "all of the time" to "all the time"
🔎 Proposed changes
-The most common case is while user opens several tabs one after the other quickly. +The most common case is when a user opens several tabs one after the other quickly.-Still **"just take me to THIS place in milliseconds"** works almost all of the time so it brings so much value that it is worthwhile to accept the fact it fails sometimes. +Still **"just take me to THIS place in milliseconds"** works almost all the time so it brings so much value that it is worthwhile to accept the fact it fails sometimes.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.mdPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsFocusEventDispatcher.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.cs
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
📚 Learning: 2025-11-06T12:57:11.574Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 4072
File: Plugins/Flow.Launcher.Plugin.Program/Main.cs:100-111
Timestamp: 2025-11-06T12:57:11.574Z
Learning: In Flow Launcher's Program plugin Main.cs QueryAsync method, the semaphore pattern where WaitAsync(token) is inside a try-catch that only catches OperationCanceledException, followed by an explicit Release() call after the try-catch block, is safe and doesn't cause semaphore leaks. When WaitAsync throws OperationCanceledException, the semaphore is not acquired, so no release is needed. When WaitAsync succeeds, the Release() call after the try-catch executes normally.
Applied to files:
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs
📚 Learning: 2025-10-16T10:48:30.573Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:252-275
Timestamp: 2025-10-16T10:48:30.573Z
Learning: In Flow Launcher's App.xaml.cs, the plugin initialization block (lines 252-275) that includes PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, _mainVM.QueryResults(), and API.SaveAppAllSettings() does not require additional Task.Run wrappers or Dispatcher.InvokeAsync calls according to maintainer Jack251970, as the threading model is already safe as designed.
Applied to files:
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsFocusEventDispatcher.cs
📚 Learning: 2025-07-06T12:21:37.947Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3572
File: Flow.Launcher/App.xaml.cs:214-216
Timestamp: 2025-07-06T12:21:37.947Z
Learning: In Flow Launcher, the UpdatePluginManifestAsync method in PluginsManifest.cs already has comprehensive internal try-catch handling that logs exceptions and returns false on failure rather than throwing, making external try-catch wrappers unnecessary.
Applied to files:
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsFocusEventDispatcher.cs
🧬 Code graph analysis (2)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs (4)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (1)
Main(17-280)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.cs (7)
TabsCache(12-96)Contains(59-62)Contains(64-74)Empty(31-37)Add(39-50)Add(51-57)RemoveAllNonExistentTabs(76-95)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (1)
BrowserTab(114-124)Flow.Launcher.Infrastructure/Stopwatch.cs (1)
Stopwatch(8-59)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsFocusEventDispatcher.cs (2)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs (1)
TabsWalker(18-138)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (3)
TabsTracker(19-281)RegisterTab(262-280)Dispose(85-104)
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs
[warning] 246-246:
wnd is not a recognized word. (unrecognized-spelling)
[warning] 95-95:
wnd is not a recognized word. (unrecognized-spelling)
🪛 LanguageTool
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.md
[grammar] ~27-~27: Ensure spelling is correct
Context: ... a wrong tab. The most common case is while user opens several tabs one after the o...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~28-~28: Consider removing “of” to be more concise
Context: ...S place in milliseconds"** works almost all of the time so it brings so much value that it...
(ALL_OF_THE)
🔇 Additional comments (4)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs (1)
1-138: LGTM - Previous issues resolved.All critical issues from previous reviews have been addressed:
- ClassName now correctly uses
nameof(TabsWalker)(line 20)- Thread.Sleep properly used instead of unawaited Task.Delay (lines 71, 103)
- "Rescanning" typo fixed (line 121)
- Tab cache cleanup on window/tab removal events implemented
The TODO comment at line 83 is acknowledged in the PR objectives as an intentional placeholder for future enhancement.
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsFocusEventDispatcher.cs (1)
1-89: LGTM - Well-structured dispatcher pattern.The implementation properly handles focus events on a background thread with:
- Correct disposal sequence (cancel, complete adding, join, dispose)
- Appropriate exception handling for process lifetime and general errors
- Named background thread for easier debugging
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (1)
1-281: LGTM - Previous thread-safety issues resolved.The implementation now properly synchronizes access to shared state:
UrlToBrowserTabaccess protected by_synclock (lines 116-123, 128-131, 264-271)_browserWindowsTrackedaccess protected by_synclock (lines 88-91, 219-229, 249-253, 276-279)- Process properly disposed with
usingdeclaration (line 157)_initializedflag correctly reset in Dispose (line 101)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.md (1)
1-42: LGTM - Clear documentation with previous grammar issues resolved.The README effectively explains the feature, its implementation, known limitations, and alternatives explored. Previous grammar corrections (higher-level, high-end, brings) have been applied.
Replaced object locks with a custom Lock class for clearer thread synchronization in TabsCache and TabsTracker. Enabled nullable reference types in TabsTracker and updated method signatures and variables to use nullable types where appropriate. Modernized collection initializations and refactored methods for improved readability. Cleaned up using directives. These changes enhance thread safety, code clarity, and nullability handling in browser tab management.
The null-conditional operator was removed from _tabsTracker.Dispose() in the Dispose() method, assuming _tabsTracker is always non-null when Dispose() is called.
Jack251970
left a comment
Jack251970
left a comment
There was a problem hiding this comment.
It looks good to me! But due to my limited understanding of browsers, I have no way to confirm whether there are no problems with them. So let's wait for jjw24's review (he is the author of Browser Tabs)
|
@jjw24 Since you are the author of Browser Tabs, you should know more about this. I wonder if you can help me review it? Thanks! |
Jack251970
removed
the
review in progress
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs (2)
35-41: Consider documenting the "bolt-tab" filter rationale.The filter skips tabs with
classNamecontaining "bolt-tab", which appears to target Azure DevOps technical tabs. A brief inline comment explaining which browser/application uses this className pattern would help future maintainers understand why this specific filter exists.🔎 Suggested documentation
// There are kind of technical tabs that should be ignored + // Azure DevOps uses "bolt-tab" CSS class for its internal tab components var className = tab.Current.ClassName; if (className.Contains("bolt-tab", StringComparison.OrdinalIgnoreCase))
133-139: Consider logging when tab does not exist.
CheckTabExistenceonly logs when the tab exists. For debugging purposes, it may be helpful to also log when a tab is not found, especially since this method is called to verify tab state.🔎 Proposed enhancement
internal void CheckTabExistence(string runtimeId, string reason = "") { if (_cache.Contains(runtimeId)) { Context.API.LogDebug(ClassName, $"Tab exists {reason}"); } + else + { + Context.API.LogDebug(ClassName, $"Tab not found {reason}"); + } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.cs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-06T12:57:11.574Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 4072
File: Plugins/Flow.Launcher.Plugin.Program/Main.cs:100-111
Timestamp: 2025-11-06T12:57:11.574Z
Learning: In Flow Launcher's Program plugin Main.cs QueryAsync method, the semaphore pattern where WaitAsync(token) is inside a try-catch that only catches OperationCanceledException, followed by an explicit Release() call after the try-catch block, is safe and doesn't cause semaphore leaks. When WaitAsync throws OperationCanceledException, the semaphore is not acquired, so no release is needed. When WaitAsync succeeds, the Release() call after the try-catch executes normally.
Applied to files:
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs
🧬 Code graph analysis (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (4)
TabsTracker(21-283)Init(76-85)OpenUrlAndTrack(36-44)Dispose(87-106)
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs
[warning] 97-97:
wnd is not a recognized word. (unrecognized-spelling)
🔇 Additional comments (10)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (3)
30-31: LGTM! Clean TabsTracker integration.The TabsTracker is properly instantiated and initialized during plugin startup. The lazy
Init()call after bookmark loading ensures the automation event handlers are registered only when needed.Also applies to: 64-65
100-119: Tab injection and tracking integrated correctly.Both query branches now properly wrap results with
InjectExistingTabsand useOpenUrlAndTrackfor URL actions. The pattern is consistent between the topResults and non-topResults paths.Also applies to: 123-141
266-270: Resource cleanup properly implemented.The
_tabsTracker.Dispose()call ensures UI Automation event handlers are unregistered when the plugin is disposed, preventing resource leaks. This addresses the previously identified issue.Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs (2)
17-22: LGTM! Well-structured class with clear retry configuration.The retry timeout (5s) and interval (250ms) are reasonable for waiting on browser UI automation responses. The
TabsCacheprovides proper tracking of known tabs.
58-119: Solid implementation with proper timeout and cancellation handling.The retry loop correctly uses
Thread.Sleepfor synchronous waiting (addressing prior feedback), handlesElementNotAvailableExceptiongracefully, and logs appropriately at each step. The cache-based approach to detect new tabs is a reasonable heuristic given browser limitations.The TODO on line 85 is acknowledged as a placeholder for future research.
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (5)
36-44: LGTM! Clean URL tracking implementation.The method correctly checks the
ReuseTabssetting before enqueuing the URL, and always proceeds to open the URL regardless of the setting. The logging is appropriately placed.
46-74: Thread-safe tab injection with proper fallback handling.The
GetExistingTabhelper uses locking for thread-safe dictionary access (addressing prior feedback). The fallback toOpenUrlAndTrackwhen tab activation fails is a good resilience pattern.
76-106: Init/Dispose properly manage lifecycle.Initialization guards against double-init, and Dispose correctly:
- Unsubscribes from structure changed events for all tracked windows
- Removes the focus changed handler
- Resets
_initializedflag- Disposes the dispatcher
This addresses previous review feedback about the
_initializedflag reset.
248-262: Regarding static analysis hint:wndabbreviation.The static analysis flagged
wndas an unrecognized word. This is a conventional abbreviation for "window" in Windows API contexts and is acceptable here. You may optionally add it to a project dictionary to suppress future warnings.
21-34: No issues found. TheLockclass on line 29 is compatible with the project's target framework. The BrowserBookmark plugin targetsnet9.0-windows, which fully supportsSystem.Threading.Lock(available in .NET 9+).
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs
Outdated
Show resolved
Hide resolved
|
Hey everyone, especially first responders, thanks!, @VictoriousRaptor, @Jack251970 , @jjw24, EDITED: I couldn't stand bugs and fixed them with recent commit. Unfortunately code changes are quite huge. Apologize for this!!
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (1)
213-232: Event handler registration order could cause race conditions.The
StructureChangedEventHandleris registered (line 220) before adding the element to_browserWindowsTracked(line 227), and theWindowClosedEventhandler is registered after (line 229). IfDisableTracking()runs between lines 220 and 227, the structure handler won't be removed since the element isn't tracked yet.This was flagged in past reviews but the window-close handler registration was moved after adding to the dictionary. The structure-changed handler is still registered first.
🧹 Nitpick comments (2)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsEventsDispatcher.cs (1)
24-34: Unused constructor parametertracker.The
TabsTracker trackerparameter is accepted but never used. This appears to be dead code.🔎 Proposed fix
- public TabsEventsDispatcher(TabsTracker tracker, TabsReservationService reservationService) + public TabsEventsDispatcher(TabsReservationService reservationService) { _reservationService = reservationService;Also update the call site in
TabsTracker.csline 292.Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsReservationService.cs (1)
30-35: Static_tabsTrackerfield in non-static class creates coupling issues.The
_tabsTrackerisstaticbut assigned in the instance constructor. If multipleTabsReservationServiceinstances were created, the last one would overwrite the tracker. While currently used as a singleton inMain.cs, this pattern is error-prone.🔎 Proposed fix - make instance field
- private static TabsTracker _tabsTracker; + private readonly TabsTracker _tabsTracker; public TabsReservationService() { _tabsTracker = new TabsTracker(this); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsEventsDispatcher.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsReservationService.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Views/SettingsControl.xaml.cs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-16T14:32:00.580Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3770
File: Flow.Launcher.Infrastructure/UserSettings/Settings.cs:186-205
Timestamp: 2025-10-16T14:32:00.580Z
Learning: In Flow.Launcher.Infrastructure/UserSettings/Settings.cs, the default hotkey assignments SelectNextPageHotkey = "PageUp" and SelectPrevPageHotkey = "PageDown" are intentionally designed this way and should not be flagged as reversed. This is the intended UX convention for Flow Launcher's page navigation.
Applied to files:
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Views/SettingsControl.xaml.cs
📚 Learning: 2025-10-16T10:48:30.573Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:252-275
Timestamp: 2025-10-16T10:48:30.573Z
Learning: In Flow Launcher's App.xaml.cs, the plugin initialization block (lines 252-275) that includes PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, _mainVM.QueryResults(), and API.SaveAppAllSettings() does not require additional Task.Run wrappers or Dispatcher.InvokeAsync calls according to maintainer Jack251970, as the threading model is already safe as designed.
Applied to files:
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsEventsDispatcher.cs
🧬 Code graph analysis (2)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Views/SettingsControl.xaml.cs (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (2)
Main(17-285)SetReuseTabs(67-71)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.cs (3)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (1)
Main(17-285)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsReservationService.cs (1)
AutomationElement(111-141)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (1)
AutomationElement(300-316)
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.cs
[warning] 128-128:
tabto is not a recognized word. (unrecognized-spelling)
🔇 Additional comments (12)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Views/SettingsControl.xaml.cs (1)
51-58: LGTM - ReuseTabs property follows correct delegation pattern.The property correctly delegates to
Main.SetReuseTabs(value)which handles both enabling/disabling tracking and persisting the setting. This avoids the unnecessary bookmark reload that other settings trigger.Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (3)
30-31: Static field initialization happens beforeInit()setsContext.The
TabsReservationService(and consequentlyTabsTracker) is instantiated when the class loads, beforeInit()sets theContext. If any constructor or early initialization code attempts to useContext.APIfor logging, it will throw aNullReferenceException.Currently this appears safe because
TabsTrackerconstructor only stores references, but this is fragile.Consider lazy initialization or moving instantiation into
Init():🔎 Suggested approach
- private static readonly TabsReservationService _tabsReservationService = new(); + private static TabsReservationService _tabsReservationService; public void Init(PluginInitContext context) { Context = context; + _tabsReservationService = new TabsReservationService(); // ... rest of Init
105-124: Query integration with tab reuse looks correct.The pattern of wrapping results with
InjectExistingTabsand usingOpenUrlAndTrackfor actions properly integrates the tab reuse feature while preserving fallback behavior when the feature is disabled.
67-71:SetReuseTabscorrectly synchronizes tracking state with settings.The method properly enables/disables tracking via the service and persists the preference to settings.
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsEventsDispatcher.cs (2)
57-79: Tab resolution retry logic is reasonable for experimental feature.The 10-second timeout with 250ms intervals provides adequate time for browser tab creation. The error log on timeout is appropriate since this indicates unexpected behavior.
81-88: Dispose pattern is correct.Proper shutdown sequence: cancel token, complete queue, join worker thread, dispose resources.
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.cs (1)
87-135: UpdateTabs cache reconciliation logic is well-designed.The age-based removal strategy correctly handles UIA's unreliability by not immediately removing elements that temporarily disappear. The logic for adding, removing, and reviving tabs is sound.
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsReservationService.cs (1)
65-93:InjectExistingTabscorrectly handles tab activation failures.The fallback to
OpenUrlAndTrackwhenActivateTab()fails provides resilient behavior, and the cleanup of stale mappings prevents repeated failures.Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (4)
270-297:OnStructureChangedevent handler processes invalidations correctly.The handler properly enqueues structure invalidations and dispatches URL-token pairs for tab resolution. The lazy initialization of
_eventsDispatcherwithin the lock is thread-safe.
333-355:DisableTrackingproperly cleans up all resources.The method correctly disposes the events dispatcher, removes all automation event handlers, clears tracked windows, and resets state. The lock ensures thread-safe cleanup.
177-199: Process disposal is now handled correctly withusing.The
using var process = TryProcess(processId)ensures the Process object is disposed after use, addressing the previous review concern about handle leaks.
49-59:RuntimeIdToKeyexception handling is correct.The method now catches
ElementNotAvailableExceptionand returns null for stale elements, preventing propagation to callers.
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsReservationService.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsReservationService.cs (1)
30-34: Consider making_tabsTrackeran instance field.The static
_tabsTrackerfield prevents multiple independent instances ofTabsReservationServicefrom coexisting. While the current usage pattern (singleton in Main.cs) makes this acceptable, using an instance field would improve testability and design flexibility.🔎 Suggested refactor
- private static TabsTracker _tabsTracker; + private readonly TabsTracker _tabsTracker; public TabsReservationService() { _tabsTracker = new TabsTracker(this); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.mdPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsReservationService.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.md
🧰 Additional context used
🧬 Code graph analysis (2)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (4)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (4)
Main(17-285)Dispose(271-275)List(89-148)List(237-264)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsReservationService.cs (9)
TabsReservationService(19-203)TabsReservationService(32-35)AutomationElement(111-141)TokenForNewTab(40-43)TokenForNewTab(95-109)Dispose(186-189)UnregisterTabs(171-184)List(65-93)EnableTracking(191-202)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.cs (5)
AutomationElement(73-85)TabsCache(13-136)List(65-71)Invalidate(57-63)UpdateTabs(87-135)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsEventsDispatcher.cs (4)
TabsEventsDispatcher(12-89)TabsEventsDispatcher(24-34)Dispose(81-88)Enqueue(36-40)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsReservationService.cs (2)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (1)
Main(17-285)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (7)
TabsTracker(17-370)TabsTracker(46-49)AutomationElement(310-326)MakeSnapshot(109-121)TrackingInfo(90-98)IEnumerable(63-85)EnableTracking(328-341)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: gitStream.cm
🔇 Additional comments (17)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.cs (4)
21-21: Lock field naming is already consistent.The Copilot comment regarding lock field naming appears to be outdated. The code already uses
_syncwith the underscore prefix, which is consistent with the convention used in TabsTracker.
32-55: LGTM! Safe property access patterns.The helper methods provide appropriate exception handling for UI Automation element access.
TryNamereturns exception type names for debugging, andDestroyedcorrectly identifies stale elements.
57-85: LGTM! Thread-safe cache operations.The cache invalidation and retrieval methods correctly use synchronization and provide safe access to the cached elements. The collection expression syntax in
GetTabsis appropriate for creating a snapshot.
87-135: LGTM! Well-designed cache reconciliation with aging strategy.The
UpdateTabsmethod implements a robust aging-based removal strategy that handles UI Automation's unreliability. Tabs are only removed after aging out or when confirmed destroyed, and reappearing tabs have their age reset. The thread-safe implementation correctly maintains cache consistency.Note: The implementation assumes
AutomationElementinstances use reference equality, which is appropriate for UI Automation's per-snapshot behavior.Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsReservationService.cs (5)
55-63: LGTM! Correct sequence for tab tracking.The method correctly takes a snapshot before opening the URL, establishing the baseline for detecting the new tab. The token registration will be fulfilled when the tab appears.
65-93: LGTM! Proper tab activation with fallback.The method correctly injects existing tabs into results and handles activation failures by reopening the URL. The use of
ConcurrentDictionaryprovides appropriate thread-safety for the URL-to-tab mappings.
95-109: LGTM! Thread-safe token registration.The token registration correctly handles concurrent requests for the same index by incrementing the
RequestedStillcounter, ensuring each request gets resolved to a distinct tab.
111-141: LGTM! Robust token resolution logic.The method correctly resolves tokens to tabs using an incremental index strategy, ensuring multiple URLs opened simultaneously get mapped to distinct tabs. The
RequestedStillcounter andLastReturnedIndextracking prevent tab collision.
143-202: LGTM! Complete lifecycle management.These methods properly handle tab registration/unregistration with appropriate exception handling, thread-safety, and resource cleanup. The
EnableTrackingmethod correctly clears mappings when enabling tab reuse to start with a fresh state.Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (8)
17-49: LGTM! Well-structured concurrency design.The class uses separate locks (
_syncand_syncInvalidations) for different concerns, avoiding lock contention. The combination of locks and concurrent collections provides appropriate thread-safety for UI Automation event handling.
51-85: LGTM! Safe element enumeration with appropriate filtering.
RuntimeIdToKeycorrectly handles stale elements, andFindAllValidTabsproperly filters out technical UI tabs while using an efficient iterator pattern.
87-103: LGTM! Clean tracking context structure.The
TrackingInfoclass encapsulates all per-window state including event handlers and cache, and theDisposepattern correctly delegates toDisableTracking.
105-142: LGTM! Proper initialization and snapshot management.
MakeSnapshotcorrectly sequences window and tab discovery under lock, andEnsureHavingAllBrowsersWindowsuses a thread-safe one-time initialization pattern to set up global window monitoring.
144-213: LGTM! Proper window lifecycle management.The event handlers correctly manage browser window tracking with appropriate resource cleanup. The
using var processpattern ensures proper disposal, and all modifications to_browserWindowsTrackedare protected by locks.
215-243: LGTM! Thread-safe event handler registration.The subscription methods correctly use closures to capture window context and perform all registration steps (event handler attachment and dictionary updates) under the same lock held by the caller, preventing race conditions.
245-308: LGTM! Efficient cache invalidation and event handling.
EnsureHavingAllBrowsersTabsuses a correct pattern of copying invalidations under lock then processing outside the lock, minimizing lock contention.OnStructureChangedproperly handles different structure change types and lazily initializes the event dispatcher.
310-370: LGTM! Complete tracking lifecycle management.
TryGetTabefficiently searches across all tracked windows, andEnableTracking/DisableTrackingproperly manage the tracking state with comprehensive cleanup of event handlers, cached data, and background threads.
|
@andrzejmartyna Hello! First of all, thanks for your contribution! Currently, our team is working hard to bring the prerelease channel feature to Flow, and we may not have time to review your PR. Regarding your merge issue, my suggestion is to wait until the feature is basically complete before merging it, otherwise, our repository will have many issues related to this feature raised. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (1)
13-367: Solid implementation with proper concurrency control.The TabsTracker implementation demonstrates careful attention to thread-safety across UI Automation event handlers. The dual-lock design (_sync and _syncInvalidations) balances correctness with performance, and all past review concerns have been properly addressed.
Consider adding inline documentation for the two-lock strategy to help future maintainers understand why
_syncInvalidationsexists separately from_sync(specifically: to allow high-frequency structure-changed events to mark windows as invalid without blocking snapshot operations).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsReservationService.csPlugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs
🧰 Additional context used
🧬 Code graph analysis (2)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (4)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (4)
Main(17-285)Dispose(271-275)List(89-148)List(237-264)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsReservationService.cs (9)
TabsReservationService(19-203)TabsReservationService(32-35)AutomationElement(111-141)TokenForNewTab(40-43)TokenForNewTab(95-109)Dispose(186-189)UnregisterTabs(171-184)List(65-93)EnableTracking(191-202)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.cs (5)
AutomationElement(73-85)TabsCache(13-136)List(65-71)Invalidate(57-63)UpdateTabs(87-135)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsEventsDispatcher.cs (4)
TabsEventsDispatcher(12-89)TabsEventsDispatcher(24-34)Dispose(81-88)Enqueue(36-40)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsReservationService.cs (3)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (3)
Main(17-285)List(89-148)List(237-264)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (6)
TabsTracker(17-368)TabsTracker(46-49)AutomationElement(310-324)MakeSnapshot(109-121)TrackingInfo(90-98)EnableTracking(326-339)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsCache.cs (2)
AutomationElement(73-85)List(65-71)
🔇 Additional comments (7)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs (5)
1-85: LGTM! Clean structure and proper exception handling.The imports, constants, field declarations, and utility methods are well-organized. The
RuntimeIdToKeymethod correctly handlesElementNotAvailableException, andFindAllValidTabsappropriately filters out technical tabs.
87-142: LGTM! Proper initialization and synchronization.The
TrackingInfoclass provides good encapsulation of per-window state, andMakeSnapshotcorrectly synchronizes the initialization sequence with the_synclock.
144-243: LGTM! Event handler lifecycle properly managed.The event handler registration and unregistration logic is correctly synchronized with the
_synclock. Theusing varstatement on line 182 properly disposes theProcessobject, addressing the previous review concern.
245-308: LGTM! Cache invalidation and event handling well-synchronized.The two-lock design (
_syncand_syncInvalidations) effectively separates concerns and reduces contention. The lazy initialization of_eventsDispatcher(line 302) under the_synclock is correct, and the use ofConcurrentQueuefor_expectedUrlsensures thread-safe dequeuing.
310-367: LGTM! Resource cleanup and lifecycle management are thorough.
TryGetTab,EnableTracking, andDisableTrackingare all properly synchronized. The cleanup inDisableTrackingcorrectly removes event handlers, disposes resources, and resets state.Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsReservationService.cs (2)
55-141: LGTM! Token lifecycle and URL-to-tab mapping logic are sound.The methods correctly handle:
- Token registration and resolution with proper state management
- Concurrent requests via the
RequestedStillcounter- Fallback to reopening URLs when tab activation fails
- Cleanup of stale mappings
All operations are properly synchronized with the
_synclock.
143-202: LGTM! Registration, cleanup, and lifecycle management are properly implemented.The
RegisterTabmethod now correctly handlesElementNotAvailableException(lines 164-167), addressing the previous review concern. The synchronization throughout is correct, and resource cleanup is thorough.
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsReservationService.cs
Show resolved
Hide resolved
|
@andrzejmartyna Hi, sorry for pinging you again! I wonder if the function of this PR is closer to that of this plugin? https://github.com/Flow-Launcher/Flow.Launcher.Plugin.BrowserTabs And could you please further describe to me exactly what this PR has done? Did it directly manage all the tabs? What is its connection with bookmarks? Sorry for any inconvenience. |
Good point. TL;DR it is "take me to the bookmark no matter it is opened already or not".My PR is indeed kind of middle-ground between the two plugins.
So... with current plugins you can:
So finally... my PR builds a bridge between bookmarks and tabs but it is closer to bookmarks, IMO. BTW. If only browsers could expose URL in existing tabs in a secure way or if we could assign some metadata to tabs while opening it... There would be more possibilities. Cheers! |
Only tabs directly mapped with source URLs of bookmarks are important (to use "ActivateTab" on them). |
|
@jjw24 On whether this PR should belong more to bookmarks or tabs, my view is the same as andrzejmartyna's. If we must merge this PR in either Bookmark or Tab plugin, I think this Bookmark plugin is the better choice. |
|
@andrzejmartyna Thanks for your quick reply! Now I understand your PR better. |
|
@andrzejmartyna Thank you for the explanation with this PR. In the area of tab management, is it accurate to say that it manages tabs of the bookmarks opened via the plugin and not the other opened non-bookmark tabs? (Apologies if this question is already asked in a different way above- wanted to confirm my own understanding) Reading from the readme also, sounds like if user opens a bookmark and navigates away from it on the same tab, it will still activate that wrong tab, right? (Totally ok, just wanted to confirm out of curiosity) |
Exactly!
That's right. It might be confusing if the user goes to a totally different place. |
Now it works well so it is no longer experimental. Maybe let's keep it "false" by default for some time. It will make it harder to spread among users but IMO it's safer.
5 participants
Imagine you use tens or hundreds of bookmarks and tabs every day (I do, my coworkers do - we constantly switch between ADO repositories, ADO pipelines, Azure Portal tabs, AWS tabs, Confluence, JIRA, Lucid Chart, etc.).
--> Switching between them means several seconds of active searching (with some mental cost of it) every time.
--> This PR turns it into a few keystrokes using just automatic muscle memory.
Rationale behind this PR: https://github.com/Flow-Launcher/Flow.Launcher/blob/20f898ce2e036ce7a58badd34ff501ea293672f7/Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.md