-
-
Notifications
You must be signed in to change notification settings - Fork 442
New browser bookmarks plugin #4003
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
A complete redesign focused on performance / low system impact / good separation of logic for easy maintenance. ## Architectural - all asynchronous, service-oriented design. - distinct services for loading bookmarks, handling favicons, monitoring files ## Performance - Faster Startup: bookmarks are loaded first and asynchronously for 'instant' search. The slower favicon processing is then completed in the background without blocking the UI. - Optimized Firefox: avoids the slow / high disk `places.sqlite` file copy in most instances, so much quicker loading of Firefox bookmarks. - Real-Time Updates: the hyperactive rescanning issue has been fixed. File monitoring for Chromium is now low-overhead with immediate update. The unstable monitoring (of `places.sqlite`) for Firefox has been removed and replaced with periodic check.
🥷 Code experts: jjw24 jjw24 has most 🧠 knowledge in the files. See details
Knowledge based on git-blame: ✨ Comment |
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
@dcog989 Why did you change the directory of the plugin from |
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.
Pull Request Overview
This PR replaces the existing BrowserBookmark
plugin with a completely rewritten BrowserBookmarks
plugin that improves performance, extensibility, and UX. The new plugin introduces automatic browser engine detection, real-time bookmark updates for Chromium browsers, and enhanced favicon handling with optional web fetching.
Key Changes:
- Complete plugin rewrite with async/await architecture and better separation of concerns
- Automatic browser engine detection eliminates manual configuration requirement
- Real-time file monitoring for Chromium browsers with periodic updates for Firefox
- Enhanced favicon support with local extraction and optional web fetching
Reviewed Changes
Copilot reviewed 39 out of 66 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/ |
New plugin implementation with improved architecture |
Plugins/Flow.Launcher.Plugin.BrowserBookmark/ |
Removed old plugin files |
Flow.Launcher.sln |
Updated solution to reference new plugin |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Flow.Launcher.Plugin.BrowserBookmarks.csproj
Outdated
Show resolved
Hide resolved
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Flow.Launcher.Plugin.BrowserBookmarks.csproj
Outdated
Show resolved
Hide resolved
{DB90F671-D861-46BB-93A3-F1304F5BA1C5}.Release|x64.Build.0 = Release|Any CPU | ||
{DB90F671-D861-46BB-93A3-F1304F5BA1C5}.Release|x86.ActiveCfg = Release|Any CPU | ||
{DB90F671-D861-46BB-93A3-F1304F5BA1C5}.Release|x86.Build.0 = Release|Any CPU | ||
{FF742965-9A80-41A5-B042-D6C7D3A21708}.Debug|Any CPU.ActiveCfg = Debug|Any CPU |
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.
Missing Build.0 configuration for Debug|Any CPU platform. This line should include '.Build.0 = Debug|Any CPU' similar to other configurations to ensure the project builds in Debug mode.
{FF742965-9A80-41A5-B042-D6C7D3A21708}.Debug|Any CPU.ActiveCfg = Debug|Any CPU | |
{FF742965-9A80-41A5-B042-D6C7D3A21708}.Debug|Any CPU.ActiveCfg = Debug|Any CPU | |
{FF742965-9A80-41A5-B042-D6C7D3A21708}.Debug|Any CPU.Build.0 = Debug|Any CPU |
Copilot uses AI. Check for mistakes.
It's a new plugin. All code is new apart from some reused XAML. Is this critical? |
A very noticeable problem is that users will lose their settings and caches of old Bookmark plugin since you change the namespace. And I am not sure if Crowdin can work here. Additionally, we need to check related Github Actions & CI/workflows. |
namespace Flow.Launcher.Plugin.BrowserBookmarks.Models; | ||
|
||
// A base class that implements INotifyPropertyChanged for view models. | ||
public abstract class BaseModel : INotifyPropertyChanged |
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.
Why we do not use this?
public class BaseModel : INotifyPropertyChanged |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
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. 📝 WalkthroughWalkthroughReplaces the old BrowserBookmark plugin with a new BrowserBookmarks plugin. Updates the solution to reference the new project. Removes all legacy loaders/UI/models and adds a new async loading pipeline, favicon services, watcher service, MVVM settings UI, localization updates, and a new plugin manifest. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant FL as Flow Launcher
participant Plugin as BrowserBookmarks.Main
participant BLS as BookmarkLoaderService
participant FWS as BookmarkWatcherService
participant FVS as FaviconService
FL->>Plugin: Init(context)
Plugin->>Plugin: Setup temp dir, load Settings
Plugin->>BLS: Construct(loaders from Settings)
Plugin->>FWS: Construct()
Plugin->>FVS: Construct()
FWS-->>Plugin: OnBookmarkFileChanged (event)
Plugin->>Plugin: ReloadDataAsync()
Plugin->>BLS: LoadBookmarksAsync()
BLS-->>Plugin: Bookmarks + Discovered files
Plugin->>FWS: UpdateWatchers(files)
Plugin->>FVS: ProcessBookmarkFavicons(bookmarks) [async]
FL->>Plugin: Query(q)
Plugin->>Plugin: Fuzzy match Name/URL
Plugin-->>FL: Results (open URL, copy URL ctx)
rect rgba(200,235,255,0.2)
note right of Plugin: Firefox periodic reload (if enabled)
Plugin->>BLS: Load Firefox-only
BLS-->>Plugin: Firefox bookmarks
Plugin->>Plugin: Merge & swap cache
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. 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: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/da.xaml (1)
1-34
: Incomplete Danish localization - Most strings remain in English.The Danish resource file contains mostly English strings instead of proper Danish translations. Only three strings on Lines 23-25 are translated to Danish ("Tilføj", "Rediger", "Slet"). All other strings should be properly localized.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_name">Browser Bookmarks</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_name">Browser Bogmærker</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_description">Search your browser bookmarks</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_description">Søg i dine browser bogmærker</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Indlæs favicons (kan tage tid under opstart)</system:String>This affects user experience for Danish users who will see mixed English/Danish text in the interface.
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Models/CustomBrowser.cs (1)
56-67
: Add localization for BrowserType.UnknownPlugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/en.xaml has no mapping for the EnumLocalizeValue "Unknown" — add the corresponding resource entry (follow the plugin's key naming convention) and sync it to Crowdin.
Flow.Launcher.sln (1)
5-20
: CI path mismatch — fix BrowserBookmark vs BrowserBookmarks and confirm plugin ID/migrations
- .github/workflows/default_plugins.yml references Plugins/Flow.Launcher.Plugin.BrowserBookmark/* and publishes Flow.Launcher.Plugin.BrowserBookmark, but the repo contains Plugins/Flow.Launcher.Plugin.BrowserBookmarks/plugin.json (ID: 0ECADE17459B49F587BF81DC3A125110). Update workflow paths or rename the folder to match.
- crowdin.yml uses Languages/en.xaml and Properties/Resources.resx (no plugin paths) — Crowdin config appears unaffected.
- If the rename intentionally changed the plugin identity, ensure plugin.json ID matches the previous ID or implement runtime migration for settings/caches (per Main.cs comment). Also update any Scripts/ or .github/actions references (e.g., .github/actions/spelling/expect.txt contains "browserbookmark").
🧹 Nitpick comments (81)
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/nb.xaml (7)
9-9
: Localize the English fallback ("copy failed") to Norwegian.Current value is English; inconsistent with the NB locale.
Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Kunne ikke kopiere nettadresse til utklippstavlen</system:String>
31-31
: Localize the favicons setting string.English text in NB file.
Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Last inn favikoner (kan være tidkrevende ved oppstart)</system:String>
16-16
: Fix Norwegian wording ("banen" → "sti").“Bane” is incorrect for filesystem path.
Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_setBrowserFromPath">Angi nettleser fra banen:</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_setBrowserFromPath">Angi nettleser fra sti:</system:String>
20-20
: Clarify intent: load bookmarks, not the browser.Wording suggests loading the browser itself.
Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_loadBrowserFrom">Last nettleser fra:</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_loadBrowserFrom">Last bokmerker fra:</system:String>
26-26
: Prefer “Bla gjennom” for browse action.More idiomatic button label.
Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_browseBrowserBookmark">Bla</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_browseBrowserBookmark">Bla gjennom</system:String>
30-30
: Norwegian possessive for “Brave's”.Use “Braves motor”.
Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">For eksempel: Brave's motor er Chromium; og standardplasseringen av bokmerker er: "%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData". For Firefox-motoren er bokmerkekatalogen brukerdatamappen som inneholder places.sqlite-filen.</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">For eksempel: Braves motor er Chromium; og standardplasseringen av bokmerker er: "%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData". For Firefox-motoren er bokmerkekatalogen brukerdatamappen som inneholder places.sqlite-filen.</system:String>
2-33
: Verify resource key prefix vs. plugin rename; confirm Crowdin path updates.Keys retain “browserbookmark” (singular) prefix while project is “BrowserBookmarks” (plural). Fine if code/XAML still reference these keys, but please verify to avoid missing strings. Also confirm Crowdin/source extraction jobs track the new plugin directory so NB (and other locales) continue to sync.
If helpful, I can provide a grep script to confirm key usage across the new project and check Crowdin config changes.
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/uk-UA.xaml (2)
31-31
: Update favicon setting copy to reflect background/parallel fetching.Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Завантажити піктограми (може зайняти багато часу під час запуску)</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Увімкнути значки сайтів; відсутні значки можуть завантажуватися у фоновому режимі</system:String>
9-9
: Ukrainian microcopy: use “скопіювати” (not “вставити”) and uppercase “URL”.Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Не вдалося вставити Url-адресу в буфер обміну</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Не вдалося скопіювати URL-адресу до буфера обміну</system:String>Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/sk.xaml (2)
31-31
: Favicon setting text implies slower startup; now favicons load async.Adjust wording to reflect background/async favicon loading and optional web fetch, not startup latency.
1-1
: Byte Order Mark present. Consider removing to reduce diff churn.The leading BOM before the XML declaration can cause noisy diffs across locales. Prefer UTF‑8 without BOM unless your tooling requires it.
If the repo standardizes on no‑BOM, apply “Save without BOM” to all XAML locale files in this plugin for consistency.
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/ko.xaml (2)
9-10
: Untranslated string and casing (URL) — localize for ko-KR.Line 9 is English and uses "url" instead of "URL". Recommend Korean translation consistent with other entries.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">클립보드에 URL을 복사하지 못했습니다</system:String>
13-16
: Several English strings left in Korean resource file — translate for consistency.Localize these labels to avoid mixed-language UI.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_openBookmarks">Open bookmarks in:</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_openBookmarks">북마크를 열 위치:</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_setBrowserFromPath">Set browser from path:</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_setBrowserFromPath">경로에서 브라우저 지정:</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_others">Others</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_others">기타</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">파비콘 로드(시작 시 시간이 오래 걸릴 수 있음)</system:String>Also applies to: 27-27, 31-31
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/cs.xaml (3)
9-9
: Untranslated string in Czech locale.
Failed to set url in clipboard
is English and “url” casing is inconsistent. Suggest Czech + URL casing.Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Nepodařilo se zkopírovat adresu URL do schránky</system:String>
31-31
: Untranslated + possibly outdated performance note.Localize and avoid implying startup blocking if work is now async/parallel.
Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Načítat favikony (první načtení může chvíli trvat)</system:String>
18-19
: Consistency: use “URL” casing in Czech strings.Keep “URL” uppercase for consistency with other entries.
Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Kopírovat URL</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Zkopírovat adresu URL záložky do schránky</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Kopírovat URL</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Zkopírovat adresu URL záložky do schránky</system:String>(If already consistent across locales, ignore.)
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/vi.xaml (4)
9-9
: Untranslated/error‑case string; fix capitalization and locale terms.Use Vietnamese and “URL” capitalization; prefer “bộ nhớ tạm” for clipboard.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Không thể sao chép URL vào bộ nhớ tạm</system:String>
18-19
: Copy URL strings: capitalize “URL” and localize “clipboard”.Keeps terminology consistent across vi locale.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Sao chép url</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Sao chép url của dấu trang vào clipboard</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Sao chép URL</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Sao chép URL của dấu trang vào bộ nhớ tạm</system:String>
15-15
: UI wording nit: drop “Thêm” for the option label.“Tab mới” reads more natural in settings.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_newTab">Thêm Tab mới</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_newTab">Tab mới</system:String>
31-31
: Untranslated setting; provide Vietnamese.Translate and keep “favicon” product term unlocalized.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Tải favicon (có thể tốn thời gian khi khởi động)</system:String>Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/es-419.xaml (1)
9-31
: Untranslated strings in es-419; propose Spanish localizations and minor copy fixes (URL casing).Several entries remain in English and “url” should be “URL”. Suggested changes below.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">No se pudo copiar la URL al portapapeles</system:String> @@ - <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Copiar url</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Copiar la url del marcador al portapapeles</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Copiar URL</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Copiar la URL del marcador al portapapeles</system:String> @@ - <system:String x:Key="flowlauncher_plugin_browserbookmark_browseBrowserBookmark">Browse</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_others">Others</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Browser Engine</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">If you are not using Chrome, Firefox or Edge, or you are using their portable version, you need to add bookmarks data directory and select correct browser engine to make this plugin work.</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">For example: Brave's engine is Chromium; and its default bookmarks data location is: "%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData". For Firefox engine, the bookmarks directory is the userdata folder contains the places.sqlite file.</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_browseBrowserBookmark">Examinar</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_others">Otros</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Motor del navegador</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">Si no usas Chrome, Firefox o Edge, o utilizas sus versiones portátiles, debes añadir la carpeta de datos de marcadores y seleccionar el motor de navegador correcto para que este complemento funcione.</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Por ejemplo: el motor de Brave es Chromium y su ubicación predeterminada de datos de marcadores es: "%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData". Para Firefox, la carpeta de marcadores es la de usuario que contiene el archivo places.sqlite.</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Cargar favicons (puede tardar durante el inicio)</system:String>Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/pl.xaml (2)
9-9
: English string left in Polish locale.Provide Polish translation.
Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Nie udało się skopiować adresu URL do schowka</system:String>
18-19
: Normalize “URL” capitalization in PL strings.Minor copy polish for consistency.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Kopiuj adres url</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Skopiuj adres url zakładki do schowka</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Kopiuj adres URL</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Skopiuj adres URL zakładki do schowka</system:String>Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/ru.xaml (5)
1-1
: Normalize UTF‑8 BOM usage across language filesThe leading BOM appears intentional. Please confirm all language XAMLs use the same encoding to avoid noisy diffs and potential tooling quirks.
9-9
: Untranslated string → provide RU translationReplace the English fallback with Russian.
Apply this diff:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Не удалось скопировать URL-адрес в буфер обмена</system:String>
31-31
: Untranslated string → provide RU translationLocalize the favicon setting.
Apply this diff:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Загружать значки сайтов (может замедлить запуск)</system:String>
17-17
: Wording: use imperative “Выбрать” instead of noun “Выбор”Improves UI tone consistency with other actions.
Apply this diff:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_choose">Выбор</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_choose">Выбрать</system:String>
22-22
: Clarify label to explicitly mention bookmarksMore specific and consistent with other strings.
Apply this diff:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_browserBookmarkDataDirectory">Путь к каталогу данных</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_browserBookmarkDataDirectory">Путь к каталогу данных закладок</system:String>Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/zh-cn.xaml (5)
9-9
: Fix trailing space and terminology (“剪切板” → “剪贴板”).There’s a trailing space and a common typo. Recommend consistent wording across related strings.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">无法复制 URL 到剪切板 </system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">无法将网址复制到剪贴板</system:String>
29-30
: Chinese spacing/wording polish in guide messages.Remove unnecessary spaces around punctuation and consider “便携版” over “绿色版” for zh-CN UI.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">如果你没有使用 Chrome、 Firefox 或 Edge,或者正在使用它们的绿色版, 那么你需要添加书签数据目录并选择正确的浏览器引擎才能使此插件正常工作。</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">如果你没有使用 Chrome、Firefox 或 Edge,或者正在使用它们的便携版,那么需要添加书签数据目录并选择正确的浏览器引擎才能使此插件正常工作。</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">例如:Brave 浏览器的引擎是 Chromium;其默认书签数据位置是 "%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData"。 对于 Firefox 引擎的浏览器,书签目录是 places.sqlite 文件所在的用户数据文件夹。</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">例如:Brave 浏览器的引擎是 Chromium;其默认书签数据位置是“%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData”。对于 Firefox 引擎的浏览器,书签目录是 places.sqlite 文件所在的用户数据文件夹。</system:String>
31-31
: “收藏夹图标”/paren style consistency (“网站图标(favicon)”).Prefer consistent terminology with other locales and full-width parentheses in zh-CN.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">载入收藏夹图标 (启动时可能十分耗时)</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">加载网站图标(favicon)(启动时可能较慢)</system:String>
1-3
: Optional: use x:String and drop mscorlib assembly hint.In modern WPF on .NET,
x:String
is sufficient and avoidsmscorlib
binding. Not required if consistent with the rest of the repo.-<ResourceDictionary xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:system="clr-namespace:System;assembly=mscorlib"> +<ResourceDictionary xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">And replace
system:String
withx:String
throughout this file.
5-33
: Crowdin and settings/cache migration check due to plugin rename.Since the directory/namespace changed, ensure:
- Crowdin path mapping is updated so zh-CN continues to sync.
- Plugin ID/Action keywords migration (if changed) won’t drop user settings/caches without a migration step.
If needed, I can draft a simple migration routine to copy old settings/caches from
Flow.Launcher.Plugin.BrowserBookmark
toFlow.Launcher.Plugin.BrowserBookmarks
.Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/de.xaml (5)
12-29
: Copy may be outdated vs. “automatic engine detection”.Strings reference manual engine selection; PR states engine detection is automatic. Align the guidance to avoid confusing users, or clarify that manual settings are only for portable/custom installs.
If manual selection is no longer exposed, I can propose updated German copy inline. Want me to draft it?
9-9
: German phrasing improvement for error message.Apply this diff:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">URL in Zwischenablage konnte nicht festgelegt werden</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">URL konnte nicht in die Zwischenablage kopiert werden</system:String>
22-22
: Grammar nit.Apply this diff:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_browserBookmarkDataDirectory">Pfad zu Datenverzeichnis</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_browserBookmarkDataDirectory">Pfad zum Datenverzeichnis</system:String>
32-32
: Label may not reflect new async favicon behavior.If favicons now load in background and not strictly on startup, rephrase to set expectations.
Apply this diff (if accurate):
- <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Favicons laden (kann während des Starts zeitaufwendig sein)</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Favicons laden (wird im Hintergrund verarbeitet; der erste Durchlauf kann länger dauern)</system:String>
2-2
: Crowdin/GitHub Actions path alignment.The plugin moved to Plugins/Flow.Launcher.Plugin.BrowserBookmarks/. Ensure l10n pipelines (Crowdin config, extraction/upload/download jobs) include this new path, otherwise translations won’t update.
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/he.xaml (3)
9-9
: Translate the remaining English strings for consistency (and RTL rendering).Two keys are still in English; translate to improve UX and avoid LTR/RTL mixing.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">העתקת הכתובת ללוח נכשלה</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">טעינת סמלי אתרים (עשויה להאט את ההפעלה)</system:String>Also applies to: 31-31
29-30
: Optional RTL polish for mixed-direction text (paths/env vars).Long Hebrew sentences with Latin paths can render awkwardly. Consider adding LRM around the path/env variable to stabilize direction.
Example:
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02"> לדוגמה: המנוע של Brave הוא Chromium, ומיקום ברירת המחדל של נתוני הסימניות שלו הוא: ‎%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData‎. עבור מנוע Firefox, ספריית הסימניות היא תיקיית המשתמש שמכילה את הקובץ ‎places.sqlite‎. </system:String>
16-17
: Remove stale "Set browser from path" / "Choose" localization keys or clarify as manual overrideNo XAML/C# references found for flowlauncher_plugin_browserbookmark_settings_setBrowserFromPath or flowlauncher_plugin_browserbookmark_settings_choose; engine auto‑detection is implemented (CustomBrowserSettingViewModel.DetectedEngineText uses engine_detection_* and CustomBrowserSetting.xaml displays it). Remove these keys from Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/*.xaml (all locales) or change the copy to explicitly indicate a manual/portable‑browser override (e.g., “Manual path for portable/advanced browsers”).
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/sr.xaml (3)
9-9
: Microcopy/grammar: capitalize “URL” and fix phrasing.Use consistent capitalization and clearer wording.
Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to copy URL to clipboard</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Copy url</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Copy URL</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Copy the bookmark's url to clipboard</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Copy the bookmark's URL to clipboard</system:String>Also applies to: 18-19
31-31
: Wording conflicts with async favicon pipeline.“time consuming during startup” suggests blocking startup; new plugin claims background/parallel favicon work. Adjust to set expectations.
Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (cached in background; initial caching may take time)</system:String>
20-30
: Guide messages: minor clarity and locale.These lengthy English guide strings in sr.xaml likely need translation; also consider breaking into shorter, translatable sentences in en.xaml first.
I can propose concise EN base strings to simplify translation if helpful.
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/es.xaml (1)
9-9
: Spanish typo: “portapeles” → “portapapeles”; also capitalize URL.Fix improves consistency with Line 19.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">No se ha podido establecer la url en el portapeles</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">No se ha podido establecer la URL en el portapapeles</system:String>Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/pt-pt.xaml (3)
5-6
: Confirm key prefix vs. new plugin ID (possible lookup mismatch).All keys still use the
flowlauncher_plugin_browserbookmark_*
prefix, while this PR renames the project to BrowserBookmarks. If the plugin ID or resource lookup prefix changed (e.g., tobrowserbookmarks
), these strings may not be resolved. Please confirm the plugin ID in plugin.json and how resources are looked up. If the old prefix is intentionally kept for continuity, ignore. Otherwise, consider adding temporary alias keys to ease migration.Option (non-breaking alias if moving to
browserbookmarks
prefix):@@ -</ResourceDictionary> + <!-- Transitional aliases if the resource lookup expects `browserbookmarks_*` --> + <system:String x:Key="flowlauncher_plugin_browserbookmarks_plugin_name"> + {StaticResource flowlauncher_plugin_browserbookmark_plugin_name} + </system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmarks_plugin_description"> + {StaticResource flowlauncher_plugin_browserbookmark_plugin_description} + </system:String> +</ResourceDictionary>
9-9
: Portuguese phrasing: “copiar” instead of “definir”.More natural and consistent with the title “Copiar URL”.
Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Falha ao definir o URL na área de transferência</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Falha ao copiar o URL para a área de transferência</system:String>
1-1
: BOM/encoding churn across language files.This file includes a BOM and appears to be a reformat‑only change. To reduce noisy diffs and Crowdin churn, normalize language files to UTF‑8 (consistently with/without BOM) and a single indentation style across all locales.
Would you like a script to scan the Languages folder and report encoding/BOM inconsistencies?
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/zh-tw.xaml (2)
5-6
: Key prefix still “browserbookmark” while project renamed; confirm consistency/migration.Keys retain the old “browserbookmark” prefix. If plugin ID/namespace changed to BrowserBookmarks, verify:
- Settings/localization loaders still resolve these keys.
- Any planned settings migration from the old plugin isn’t blocked by ID or path changes.
If the intent is seamless replacement, consider keeping the old PluginID or add a migration step that copies settings/cache from the previous path. I can draft a migration helper if needed.
1-3
: Encoding/BOM and Crowdin path check.
- File starts with BOM; acceptable for XAML, but ensure your Crowdin config and build tooling expect UTF‑8 with BOM or normalize consistently across language files.
- The directory path changed to “BrowserBookmarks”; confirm Crowdin mappings and GitHub Actions are updated so translations continue to sync.
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/ar.xaml (2)
9-9
: Untranslated string (“copy failed”).This should be localized for AR.
Apply:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">تعذّر نسخ الرابط إلى الحافظة</system:String>
31-31
: Untranslated string (“Load favicons”).Localize to AR.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">تحميل الأيقونات المفضلة (قد يستغرق وقتاً عند بدء التشغيل)</system:String>Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/fr.xaml (1)
9-9
: French casing/wording nits (“url” → “URL”).Minor copy polish.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Impossible de mettre l'url dans le presse-papiers</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Impossible de copier l’URL dans le presse-papiers</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Copier l'url</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Copier l’URL</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Copier l'Url du favori dans le presse-papier</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Copier l’URL du favori dans le presse‑papiers</system:String>Also applies to: 18-19
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/nl.xaml (1)
5-31
: Mixed EN/NL strings — inconsistent localization.Many entries remain English (name/description, main/guide texts). Either translate here or leave placeholders and rely on Crowdin.
If you want, I can propose NL translations for the unchanged keys.
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/pt-br.xaml (1)
9-9
: Untranslated EN strings (“copy failed”, “Load favicons”).Localize to PT‑BR.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Falha ao copiar a URL para a área de transferência</system:String> - <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Carregar favicons (pode demorar na inicialização)</system:String>Also applies to: 31-31
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/ja.xaml (1)
9-9
: Untranslated string (“copy failed”).Localize to JA.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">クリップボードにURLをコピーできませんでした</system:String>Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/BookmarkWatcherService.cs (1)
42-51
: Consider buffer overflow handling.Set InternalBufferSize and handle watcher.Error to recover from overflows on heavy writes.
var watcher = new FileSystemWatcher(directory) { Filter = fileName, - NotifyFilter = NotifyFilters.LastWrite | NotifyFilters.FileName | NotifyFilters.Size + NotifyFilter = NotifyFilters.LastWrite | NotifyFilters.FileName | NotifyFilters.Size, + InternalBufferSize = 64 * 1024 // 64KB to reduce overflow risk }; + watcher.Error += (_, __) => TriggerDebouncedReload();Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Models/Settings.cs (1)
14-48
: Guard setters to avoid redundant PropertyChanged churn.Early‑return when value unchanged. Reduces unnecessary reloads.
- set { _loadChromeBookmark = value; OnPropertyChanged(); } + set { if (_loadChromeBookmark == value) return; _loadChromeBookmark = value; OnPropertyChanged(); } @@ - set { _loadFirefoxBookmark = value; OnPropertyChanged(); } + set { if (_loadFirefoxBookmark == value) return; _loadFirefoxBookmark = value; OnPropertyChanged(); } @@ - set { _loadEdgeBookmark = value; OnPropertyChanged(); } + set { if (_loadEdgeBookmark == value) return; _loadEdgeBookmark = value; OnPropertyChanged(); } @@ - set { _loadChromiumBookmark = value; OnPropertyChanged(); } + set { if (_loadChromiumBookmark == value) return; _loadChromiumBookmark = value; OnPropertyChanged(); } @@ - set { _enableFavicons = value; OnPropertyChanged(); } + set { if (_enableFavicons == value) return; _enableFavicons = value; OnPropertyChanged(); } @@ - set { _fetchMissingFavicons = value; OnPropertyChanged(); } + set { if (_fetchMissingFavicons == value) return; _fetchMissingFavicons = value; OnPropertyChanged(); }Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/it.xaml (1)
1-34
: Inconsistent localization - Line 31 remains in English.While most strings are properly localized to Italian, the favicon loading setting remains in English: "Load favicons (can be time consuming during startup)". This should be translated to Italian for consistency.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Carica favicons (può richiedere tempo durante l'avvio)</system:String>Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/IBookmarkLoader.cs (1)
9-9
: Consider more descriptive property name.The
Name
property could be more specific about what it represents (e.g.,LoaderName
,BrowserName
, orSourceName
) to avoid confusion with bookmark names.- string Name { get; } + string LoaderName { get; }Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Flow.Launcher.Plugin.BrowserBookmarks.csproj (2)
86-99
: Duplicate copy of Languages\en.xaml (None + Content)en.xaml is copied twice (None Update and Content Include). Remove the explicit None entry; the Content wildcard already covers it.
<ItemGroup> - <None Update="plugin.json"> - <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory> - </None> - <None Update="Languages\en.xaml"> - <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory> - </None> + <None Update="plugin.json"> + <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory> + </None> <None Update="Images\bookmark.png"> <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory> </None> <None Update="Images\copylink.png"> <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory> </None> </ItemGroup>Also applies to: 100-105
41-61
: Optional: prune non‑Windows runtimes is redundant for netX‑windows TFMWith net9.0‑windows, NuGet should not bring non‑Windows assets. These RemoveDir targets add build time and complexity.
Consider dropping the non‑Windows entries or the entire target unless you’ve measured size wins.
Also applies to: 63-84
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/BrowserDetector.cs (1)
15-25
: Harden against IO exceptions and add Chromium “User Data” fallbackDirectory enumeration and file checks can throw; also consider when users select the browser root that contains “User Data”.
public static BrowserType DetectBrowserType(string dataDirectoryPath) { if (string.IsNullOrEmpty(dataDirectoryPath) || !Directory.Exists(dataDirectoryPath)) return BrowserType.Unknown; - // Check for Chromium-based browsers by looking for the 'Bookmarks' file. + // Check for Chromium-based browsers by looking for the 'Bookmarks' file. // This includes checking common profile subdirectories. - var profileDirectories = Directory.EnumerateDirectories(dataDirectoryPath, "Profile *").ToList(); - var defaultProfile = Path.Combine(dataDirectoryPath, "Default"); - if (Directory.Exists(defaultProfile)) - profileDirectories.Add(defaultProfile); - - // Also check the root directory itself, as some browsers use it directly. - profileDirectories.Add(dataDirectoryPath); + var profileDirectories = new List<string>(); + try + { + profileDirectories.AddRange(Directory.EnumerateDirectories(dataDirectoryPath, "Profile *")); + var defaultProfile = Path.Combine(dataDirectoryPath, "Default"); + if (Directory.Exists(defaultProfile)) + profileDirectories.Add(defaultProfile); + // Some configs store Bookmarks directly under the data dir. + profileDirectories.Add(dataDirectoryPath); + // Fallback: typical Chromium root contains a "User Data" dir. + var userData = Path.Combine(dataDirectoryPath, "User Data"); + if (Directory.Exists(userData)) + { + profileDirectories.Add(userData); + var userDataDefault = Path.Combine(userData, "Default"); + if (Directory.Exists(userDataDefault)) + profileDirectories.Add(userDataDefault); + profileDirectories.AddRange(Directory.EnumerateDirectories(userData, "Profile *")); + } + } + catch + { + return BrowserType.Unknown; + } if (profileDirectories.Any(p => File.Exists(Path.Combine(p, "Bookmarks")))) { return BrowserType.Chromium; } // Check for Firefox-based browsers by looking for 'places.sqlite'. // This leverages the existing FirefoxProfileFinder logic. - if (File.Exists(Path.Combine(dataDirectoryPath, "places.sqlite")) || !string.IsNullOrEmpty(FirefoxProfileFinder.GetPlacesPathFromProfileDir(dataDirectoryPath))) + try + { + if (File.Exists(Path.Combine(dataDirectoryPath, "places.sqlite")) || + !string.IsNullOrEmpty(FirefoxProfileFinder.GetPlacesPathFromProfileDir(dataDirectoryPath))) + { + return BrowserType.Firefox; + } + } + catch + { + return BrowserType.Unknown; + } - - { - return BrowserType.Firefox; - }Also applies to: 30-37
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Views/CustomBrowserSetting.xaml.cs (1)
26-36
: Guard Save with CanExecute and mark key events handledAvoid executing Save when invalid and stop event bubbling.
private void WindowKeyDown(object sender, KeyEventArgs e) { if (e.Key == System.Windows.Input.Key.Enter) { - _viewModel.SaveCommand.Execute(null); + if (_viewModel.SaveCommand.CanExecute(null)) + _viewModel.SaveCommand.Execute(null); + e.Handled = true; } else if (e.Key == System.Windows.Input.Key.Escape) { _viewModel.CancelCommand.Execute(null); + e.Handled = true; } }Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Models/CustomBrowser.cs (1)
9-12
: Initialize non‑nullable stringsPrevents null states if NRT is enabled and avoids null derefs in bindings.
- private string _name; - private string _dataDirectoryPath; + private string _name = string.Empty; + private string _dataDirectoryPath = string.Empty; private BrowserType _browserType = BrowserType.Unknown;Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Views/SettingsControl.xaml (1)
27-41
: Localize browser labels (Chrome/Chromium/Edge/Firefox)Hard‑coded strings bypass Crowdin. Use resource keys like other labels.
- <CheckBox - Margin="{StaticResource SettingPanelItemRightMargin}" - Content="Chrome" - IsChecked="{Binding Settings.LoadChromeBookmark}" /> + <CheckBox + Margin="{StaticResource SettingPanelItemRightMargin}" + Content="{DynamicResource flowlauncher_plugin_browserbookmark_browser_chrome}" + IsChecked="{Binding Settings.LoadChromeBookmark}" /> @@ - <CheckBox - Margin="{StaticResource SettingPanelItemRightMargin}" - Content="Chromium" - IsChecked="{Binding Settings.LoadChromiumBookmark}" /> + <CheckBox + Margin="{StaticResource SettingPanelItemRightMargin}" + Content="{DynamicResource flowlauncher_plugin_browserbookmark_browser_chromium}" + IsChecked="{Binding Settings.LoadChromiumBookmark}" /> @@ - <CheckBox - Margin="{StaticResource SettingPanelItemRightMargin}" - Content="Edge" - IsChecked="{Binding Settings.LoadEdgeBookmark}" /> + <CheckBox + Margin="{StaticResource SettingPanelItemRightMargin}" + Content="{DynamicResource flowlauncher_plugin_browserbookmark_browser_edge}" + IsChecked="{Binding Settings.LoadEdgeBookmark}" /> @@ - <CheckBox - Margin="{StaticResource SettingPanelItemRightMargin}" - Content="Firefox" - IsChecked="{Binding Settings.LoadFirefoxBookmark}" /> + <CheckBox + Margin="{StaticResource SettingPanelItemRightMargin}" + Content="{DynamicResource flowlauncher_plugin_browserbookmark_browser_firefox}" + IsChecked="{Binding Settings.LoadFirefoxBookmark}" />I can add the en.xaml entries if you want.
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Views/SettingsControl.xaml.cs (1)
33-39
: Use bound SelectedCustomBrowser consistentlyDelete uses the element’s SelectedItem while Edit uses the bound property. Use one pattern for clarity.
- if (CustomBrowsers.SelectedItem is CustomBrowser selectedCustomBrowser) - { - Settings.CustomBrowsers.Remove(selectedCustomBrowser); - } + if (SelectedCustomBrowser is not null) + Settings.CustomBrowsers.Remove(SelectedCustomBrowser);Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/FirefoxBookmarkLoader.cs (1)
39-83
: Consider adding defensive protection against very large bookmark collections.The current implementation loads all bookmarks into memory before yielding them. For users with extensive bookmark collections, this could cause memory pressure. Consider yielding bookmarks directly as they're read from the database instead of accumulating them first.
Apply this diff to stream bookmarks directly:
public async IAsyncEnumerable<Bookmark> GetBookmarksAsync([EnumeratorCancellation] CancellationToken cancellationToken = default) { if (string.IsNullOrEmpty(_placesPath) || !File.Exists(_placesPath)) yield break; - var bookmarks = new List<Bookmark>(); string? tempDbPath = null; try { // First, try to read directly from the source to avoid a slow file copy - await ReadBookmarksFromDb(_placesPath, bookmarks, cancellationToken); + await foreach (var bookmark in ReadBookmarksFromDbAsync(_placesPath, cancellationToken)) + { + yield return bookmark; + } } catch (SqliteException ex) when (ex.SqliteErrorCode is 5 or 6) // 5 is SQLITE_BUSY, 6 is SQLITE_LOCKED { // Fallback to copying the file if the database is locked (e.g., Firefox is open) try { tempDbPath = Path.Combine(_tempPath, $"ff_places_{Guid.NewGuid()}.sqlite"); File.Copy(_placesPath, tempDbPath, true); - await ReadBookmarksFromDb(tempDbPath, bookmarks, cancellationToken); + await foreach (var bookmark in ReadBookmarksFromDbAsync(tempDbPath, cancellationToken)) + { + yield return bookmark; + } } catch (Exception copyEx) { _logException(nameof(FirefoxBookmarkLoader), $"Failed to load {_browserName} bookmarks from fallback copy: {_placesPath}", copyEx); } } catch (Exception e) { _logException(nameof(FirefoxBookmarkLoader), $"Failed to load {_browserName} bookmarks: {_placesPath}", e); } finally { if (tempDbPath != null && File.Exists(tempDbPath)) { try { File.Delete(tempDbPath); } catch(Exception e) { _logException(nameof(FirefoxBookmarkLoader), $"Failed to delete temp db file {tempDbPath}", e); } } } - - foreach (var bookmark in bookmarks) - { - yield return bookmark; - } }And update the helper method:
-private async Task ReadBookmarksFromDb(string dbPath, ICollection<Bookmark> bookmarks, CancellationToken cancellationToken) +private async IAsyncEnumerable<Bookmark> ReadBookmarksFromDbAsync(string dbPath, [EnumeratorCancellation] CancellationToken cancellationToken = default) { var profilePath = Path.GetDirectoryName(dbPath) ?? string.Empty; var connectionString = $"Data Source={dbPath};Mode=ReadOnly;Pooling=false;"; await using var dbConnection = new SqliteConnection(connectionString); await dbConnection.OpenAsync(cancellationToken); await using var command = new SqliteCommand(QueryAllBookmarks, dbConnection); await using var reader = await command.ExecuteReaderAsync(cancellationToken); while (await reader.ReadAsync(cancellationToken)) { var title = reader["title"]?.ToString() ?? string.Empty; var url = reader["url"]?.ToString(); if (!string.IsNullOrEmpty(url)) { - bookmarks.Add(new Bookmark(title, url, _browserName, profilePath)); + yield return new Bookmark(title, url, _browserName, profilePath); } } }Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/LocalFaviconExtractor.cs (2)
62-90
: Consider implementing retry logic for locked database scenarios.When copying the database file fails due to it being locked or in use, consider implementing retry logic with exponential backoff. This would improve reliability, especially during browser updates or when antivirus software temporarily locks files.
Example retry implementation:
+private async Task<string?> CopyDatabaseWithRetryAsync(string sourcePath, string destPath, int maxRetries = 3, CancellationToken token = default) +{ + for (int i = 0; i < maxRetries; i++) + { + try + { + File.Copy(sourcePath, destPath, true); + return destPath; + } + catch (IOException) when (i < maxRetries - 1) + { + await Task.Delay(TimeSpan.FromMilliseconds(100 * Math.Pow(2, i)), token); + } + } + return null; +} try { - File.Copy(dbPath, tempDbPath, true); + var copiedPath = await CopyDatabaseWithRetryAsync(dbPath, tempDbPath, 3, token); + if (copiedPath == null) + { + _context.API.LogDebug(nameof(LocalFaviconExtractor), $"Failed to copy database after retries: {dbPath}"); + return null; + }
27-28
: Use BrowserType enum instead of string.Contains on Bookmark.SourceLocalFaviconExtractor.GetFaviconDataAsync currently checks bookmark.Source.Contains("Firefox") — brittle. Use the existing BrowserType (Models/CustomBrowser.cs): either add a BrowserType property to Bookmark and set it in the loaders, or resolve bookmark.Source to the matching CustomBrowser and use its BrowserType to choose Firefox vs Chromium DB/query.
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/BookmarkLoaderService.cs (1)
104-131
: Exception handling for FirefoxProfileFinder calls is redundant.The try-catch blocks around
FirefoxProfileFinder.GetFirefoxPlacesPath()
andGetFirefoxMsixPlacesPath()
are unnecessary since these methods already have internal exception handling and return null on failure. The current approach logs exceptions that will never be thrown.Apply this diff to remove redundant exception handling:
if (_settings.LoadFirefoxBookmark) { - string? placesPath = null; - try - { - placesPath = FirefoxProfileFinder.GetFirefoxPlacesPath(); - } - catch (Exception ex) - { - _context.API.LogException(nameof(BookmarkLoaderService), "Failed to find Firefox profile", ex); - } + var placesPath = FirefoxProfileFinder.GetFirefoxPlacesPath(); if (!string.IsNullOrEmpty(placesPath)) { yield return new FirefoxBookmarkLoader("Firefox", placesPath, _tempPath, logAction); } - string? msixPlacesPath = null; - try - { - msixPlacesPath = FirefoxProfileFinder.GetFirefoxMsixPlacesPath(); - } - catch (Exception ex) - { - _context.API.LogException(nameof(BookmarkLoaderService), "Failed to find Firefox MSIX package", ex); - } + var msixPlacesPath = FirefoxProfileFinder.GetFirefoxMsixPlacesPath(); if (!string.IsNullOrEmpty(msixPlacesPath)) { yield return new FirefoxBookmarkLoader("Firefox (Store)", msixPlacesPath, _tempPath, logAction); } }Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/FaviconService.cs (4)
248-257
: Avoid O(n²) StringBuilder.ToString() in the read loop.Converting the entire buffer to a string on each iteration scales poorly. Check only the tail segment.
Apply this diff:
- if (contentBuilder.ToString().Contains("</head>", StringComparison.OrdinalIgnoreCase)) + var tailStart = Math.Max(0, contentBuilder.Length - 64); + if (contentBuilder + .ToString(tailStart, contentBuilder.Length - tailStart) + .IndexOf("</head>", StringComparison.OrdinalIgnoreCase) >= 0) { break; }
348-353
: Preserve SVG aspect ratio when scaling to 32x32.Current independent X/Y scaling can distort icons. Use uniform scale and center.
Apply this diff:
- using var bitmap = new SKBitmap(32, 32); - using var canvas = new SKCanvas(bitmap); - canvas.Clear(SKColors.Transparent); - var scaleMatrix = SKMatrix.CreateScale(32 / svg.Picture.CullRect.Width, 32 / svg.Picture.CullRect.Height); - canvas.DrawPicture(svg.Picture, in scaleMatrix); + using var bitmap = new SKBitmap(32, 32); + using var canvas = new SKCanvas(bitmap); + canvas.Clear(SKColors.Transparent); + var rect = svg.Picture.CullRect; + var scale = Math.Min(32f / rect.Width, 32f / rect.Height); + var tx = (32f - rect.Width * scale) / 2f; + var ty = (32f - rect.Height * scale) / 2f; + var m = SKMatrix.CreateScale(scale, scale); + m = m.PostConcat(SKMatrix.CreateTranslation(tx, ty)); + canvas.DrawPicture(svg.Picture, in m);
364-387
: WPF imaging on background threads may require STA.IconBitmapDecoder/PngBitmapEncoder (WPF) can throw in MTA. Consider a non‑WPF ICO parser (or ImageSharp) to avoid apartment issues, or marshal this block to an STA.
Would you like a minimal ICO reader replacement using ImageSharp or a small ICO parser?
111-120
: Optional: prefer SHA‑256 for cache keys.SHA‑1 is fine for non‑security use, but SHA‑256 avoids collision debates at negligible cost.
Apply this diff:
- var hash = SHA1.HashData(Encoding.UTF8.GetBytes(url)); + var hash = SHA256.HashData(Encoding.UTF8.GetBytes(url));(Remember to add
using System.Security.Cryptography;
if not already present.)Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Main.cs (3)
73-75
: Use Volatile.Read to pair with the Volatile.Write.Ensures readers observe a fully‑published list version.
Apply this diff:
- var bookmarks = _bookmarks; // use a local copy + var bookmarks = Volatile.Read(ref _bookmarks); // read with a memory barrier
40-50
: Add a one‑time migration to preserve old plugin caches/settings after rename.Renaming the plugin directory/namespace risks users losing settings and favicon cache. Migrate the old cache on first run.
Apply this diff and add the helper below:
var tempPath = SetupTempDirectory(); _bookmarkLoader = new BookmarkLoaderService(Context, _settings, tempPath); _faviconService = new FaviconService(Context, _settings, tempPath); + TryMigrateOldCachesAndSettings();
Add this helper (outside the shown range):
// Place inside Main class private void TryMigrateOldCachesAndSettings() { try { // Adjust old IDs/paths if different; this assumes prior plugin folder name var oldCache = Path.Combine(Context.CurrentPluginMetadata.PluginDirectory, "..", "Flow.Launcher.Plugin.BrowserBookmark", "Cache", "FaviconCache"); var newCache = Path.Combine(Context.CurrentPluginMetadata.PluginCacheDirectoryPath, "FaviconCache"); if (Directory.Exists(oldCache) && !Directory.Exists(newCache)) { Directory.CreateDirectory(Path.GetDirectoryName(newCache)!); Directory.Move(oldCache, newCache); Context.API.LogInfo(nameof(Main), "Migrated old favicon cache."); } // If settings storage key changed, consider reading the old JSON and copying relevant fields. } catch (Exception ex) { Context.API.LogException(nameof(Main), "Cache/settings migration failed.", ex); } }
152-159
: Fire‑and‑forget timer loop: capture exceptions to avoid unobserved faults.Wrap the loop in a try/catch to ensure background exceptions are logged.
Apply this diff:
- _ = Task.Run(async () => - { - while (await _firefoxBookmarkTimer.WaitForNextTickAsync(_cancellationTokenSource.Token)) - { - await ReloadFirefoxBookmarksAsync(); - } - }, _cancellationTokenSource.Token); + _ = Task.Run(async () => + { + try + { + while (await _firefoxBookmarkTimer.WaitForNextTickAsync(_cancellationTokenSource.Token)) + await ReloadFirefoxBookmarksAsync(); + } + catch (OperationCanceledException) { } + catch (Exception ex) + { + Context.API.LogException(nameof(Main), "Firefox periodic reload loop crashed.", ex); + } + }, _cancellationTokenSource.Token);Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Views/CustomBrowserSetting.xaml (1)
14-14
: MVVM: remove code‑behind KeyDown and bind keys to commands.Avoid the WindowKeyDown handler; wire Enter/Escape to Save/Cancel.
Apply this diff:
- KeyDown="WindowKeyDown"
Add key bindings:
mc:Ignorable="d"> + <Window.InputBindings> + <KeyBinding Key="Enter" Command="{Binding SaveCommand}" /> + <KeyBinding Key="Escape" Command="{Binding CancelCommand}" /> + </Window.InputBindings>Flow.Launcher.sln (1)
72-73
: Project type GUID differs from other C# SDK projects.Most projects use {9A19103F-…} (SDK‑style). If this plugin is SDK‑style too, consider aligning for consistency.
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/en.xaml (1)
8-8
: Clarify description: favicons can come from local DBs too.Minor copy tweak to reflect behavior accurately.
Apply this diff:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_description">Search your browser bookmarks, retrieve favicons from the web.</system:String> + <system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_description">Search your browser bookmarks; show favicons (local cache + optional web fetch).</system:String>
<RootNamespace>Flow.Launcher.Plugin.BrowserBookmarks</RootNamespace> | ||
<AssemblyName>Flow.Launcher.Plugin.BrowserBookmarks</AssemblyName> |
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.
🧩 Analysis chain
Namespace/assembly rename will reset user settings and caches without a migration
Renaming the project/folder changes the plugin path; unless plugin.json keeps the same ID or you migrate on first run, users lose settings, custom browsers, and favicon cache.
- Prefer keeping the old plugin ID in the new plugin.json to preserve settings.
- Or ship a one‑time migration in Main/Init: if old settings exist and new are default, copy settings + favicon cache dir.
Example migration sketch (outside this file):
// On plugin init
var oldId = "BROWSERBOOKMARK"; // previous plugin ID
var newId = context.Metadata.ID; // current
if (!string.Equals(oldId, newId, StringComparison.OrdinalIgnoreCase))
{
var oldData = StorageHelper.GetPluginDataDirectory(oldId);
var newData = StorageHelper.GetPluginDataDirectory(newId);
if (Directory.Exists(oldData) && !Directory.Exists(newData))
{
Directory.CreateDirectory(newData);
foreach (var dir in new[] { "Cache", "Favicons" })
{
var src = Path.Combine(oldData, dir);
var dst = Path.Combine(newData, dir);
if (Directory.Exists(src) && !Directory.Exists(dst))
DirectoryCopy(src, dst, true);
}
// Migrate settings if applicable
SettingsMigration.TryMigrate(oldId, newId);
}
}
Run to verify ID alignment and Crowdin/workflow references:
Also applies to: 22-23, 32-33
🏁 Script executed:
#!/bin/bash
# plugin IDs and names
fd -a -t f "plugin.json" -E "bin" -E "obj" | while read f; do
echo "--- $f"
rg -nP '"(ID|Name)"\s*:\s*".*"' "$f"
done
# check workflows/crowdin paths mention old/new dirs
rg -nP 'Flow\.Launcher\.Plugin\.BrowserBookmark|Flow\.Launcher\.Plugin\.BrowserBookmarks' .github/ -S || true
rg -nP 'Plugins/Flow\.Launcher\.Plugin\.BrowserBookmark|Plugins/Flow\.Launcher\.Plugin\.BrowserBookmarks' crowdin* .github/ -S || true
Length of output: 2964
Preserve plugin ID and fix CI/workflows after rename
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/plugin.json contains ID "0ECADE17459B49F587BF81DC3A125110"; .github/workflows/default_plugins.yml still references 'Plugins/Flow.Launcher.Plugin.BrowserBookmark' and publishes Flow.Launcher.Plugin.BrowserBookmark.csproj (lines ~40,45–53). This mismatch will break CI and can reset user settings/caches if the plugin ID or assembly name changed.
- Keep the historical plugin ID in plugin.json (preferred) OR implement a one‑time migration on plugin init (Main/Init) to copy StorageHelper data directory (Cache, Favicons) and migrate settings from the old ID to the new ID.
- Update .github/workflows/default_plugins.yml (lines ~40,45–53) and any crowdin/workflow references to the new folder/project name, or revert the project/assembly rename so packaging/CI paths match.
🤖 Prompt for AI Agents
In
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Flow.Launcher.Plugin.BrowserBookmarks.csproj
around lines 9-10, the AssemblyName/RootNamespace rename has caused a mismatch
with plugin.json ID and .github/workflows/default_plugins.yml references (lines
~40,45–53), which will break CI and may reset user data; fix it by either
restoring the original plugin ID in plugin.json (preferred) or implementing a
one-time migration in the plugin Init (copy StorageHelper data directories —
Cache, Favicons — and migrate settings from the old ID to the new ID), and
update .github/workflows/default_plugins.yml and any crowdin/workflow references
to point to the new project/folder name (or revert the project/assembly rename)
so CI packaging paths and plugin ID remain consistent.
$(OutputPath)runtimes\win-arm; | ||
$(OutputPath)runtimes\win-arm64;"/> | ||
$(OutputPath)runtimes\win-arm64;" /> | ||
</Target> |
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.
Don’t delete win‑arm64 runtime assets — breaks native ARM64 on Windows
Removing win-arm64 will likely break Skia/Svg native loading on ARM64 Flow builds. Keep ARM64; prune only non‑Windows if needed.
Apply this diff to both targets:
- $(OutputPath)runtimes\win-arm;
- $(OutputPath)runtimes\win-arm64;" />
+ $(OutputPath)runtimes\win-arm;" />
- $(PublishDir)runtimes\win-arm;
- $(PublishDir)runtimes\win-arm64;" />
+ $(PublishDir)runtimes\win-arm;" />
Also applies to: 83-84
🤖 Prompt for AI Agents
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Flow.Launcher.Plugin.BrowserBookmarks.csproj
lines ~59-61 (and likewise lines ~83-84): the runtimes list removed "win-arm64"
which will break native ARM64 (Skia/Svg) on Windows; restore "win-arm64" into
the OutputPath runtimes entries for both targets (do not remove win-arm either),
and if pruning is required restrict removals to non-Windows runtimes only so
Windows ARM/ARM64 assets remain packaged.
<system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Jádro webového prohlížeče</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">Pokud nepoužíváte prohlížeč Chrome, Firefox nebo Edge nebo používáte přenosnou verzi prohlížeče Chrome, Firefox nebo Edge, musíte přidat složku záložek a vybrat správné jádro prohlížeče, aby tento doplněk fungoval.</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Například: prohlížeč Brave má jádro Chromium; výchozí umístění pro data záložek je: "%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData". V případě jádra Firefox je složkou záložek složka userdata, která obsahuje soubor places.sqlite.</system:String> |
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.
Text contradicts “automatic engine detection” claim.
Strings instruct users to select a browser engine, but PR states engine detection is automatic. This can mislead users.
Option A (prefer): Update copy to reflect auto‑detection and limit manual steps to portable/custom paths.
- <system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Jádro webového prohlížeče</system:String>
- <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">Pokud nepoužíváte prohlížeč Chrome, Firefox nebo Edge nebo používáte přenosnou verzi prohlížeče Chrome, Firefox nebo Edge, musíte přidat složku záložek a vybrat správné jádro prohlížeče, aby tento doplněk fungoval.</system:String>
+ <system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Jádro prohlížeče (detekováno automaticky)</system:String>
+ <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">Jádro prohlížeče se obvykle detekuje automaticky. U portable nebo nestandardních instalací přidejte složku záložek ručně.</system:String>
Option B: If engine selection is removed from UI, delete these strings and purge references.
📝 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.
<system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Jádro webového prohlížeče</system:String> | |
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">Pokud nepoužíváte prohlížeč Chrome, Firefox nebo Edge nebo používáte přenosnou verzi prohlížeče Chrome, Firefox nebo Edge, musíte přidat složku záložek a vybrat správné jádro prohlížeče, aby tento doplněk fungoval.</system:String> | |
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Například: prohlížeč Brave má jádro Chromium; výchozí umístění pro data záložek je: "%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData". V případě jádra Firefox je složkou záložek složka userdata, která obsahuje soubor places.sqlite.</system:String> | |
<system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Jádro prohlížeče (detekováno automaticky)</system:String> | |
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">Jádro prohlížeče se obvykle detekuje automaticky. U portable nebo nestandardních instalací přidejte složku záložek ručně.</system:String> | |
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Například: prohlížeč Brave má jádro Chromium; výchozí umístění pro data záložek je: "%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData". V případě jádra Firefox je složkou záložek složka userdata, která obsahuje soubor places.sqlite.</system:String> |
🤖 Prompt for AI Agents
In Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/cs.xaml around lines
28-30, the guide strings instruct users to select a browser engine which
contradicts the plugin's automatic engine detection; update the Czech copy to
state that the plugin detects the browser engine automatically and only requires
manual selection/path input for portable or non-standard installs (or, if the UI
no longer exposes engine selection, remove these strings and any code references
to them). Ensure the revised text mentions automatic detection first, gives a
brief note about portable/custom paths and example paths (Brave/Firefox) if
necessary, and keep keys unchanged so localization lookups remain valid.
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Zum Beispiel: Die Engine von Brave ist Chromium, und deren Standardspeicherort der Lesezeichen-Daten ist: | ||
%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData". Bei der Firefox-Engine ist das Lesezeichenverzeichnis der Ordner userdata, der die Datei places.sqlite enthält.</system:String> |
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.
Fix stray entity and incorrect/dated path; keep value on one line.
There’s a visible “"” artifact and the Brave path should be “User Data” (with space). Also, the line break inside the value is unnecessary.
Apply this diff:
- <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Zum Beispiel: Die Engine von Brave ist Chromium, und deren Standardspeicherort der Lesezeichen-Daten ist:
-%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData". Bei der Firefox-Engine ist das Lesezeichenverzeichnis der Ordner userdata, der die Datei places.sqlite enthält.</system:String>
+ <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Zum Beispiel: Brave basiert auf Chromium; der Standardspeicherort der Lesezeichendaten ist %LOCALAPPDATA%\BraveSoftware\Brave-Browser\User Data. Bei Firefox befindet sich das Lesezeichenverzeichnis im Profilordner, der die Datei places.sqlite enthält.</system:String>
📝 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.
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Zum Beispiel: Die Engine von Brave ist Chromium, und deren Standardspeicherort der Lesezeichen-Daten ist: | |
%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData". Bei der Firefox-Engine ist das Lesezeichenverzeichnis der Ordner userdata, der die Datei places.sqlite enthält.</system:String> | |
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Zum Beispiel: Brave basiert auf Chromium; der Standardspeicherort der Lesezeichendaten ist %LOCALAPPDATA%\BraveSoftware\Brave-Browser\User Data. Bei Firefox befindet sich das Lesezeichenverzeichnis im Profilordner, der die Datei places.sqlite enthält.</system:String> |
🤖 Prompt for AI Agents
In Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/de.xaml around lines
30 to 31, the string value contains a stray HTML entity ("), an incorrect
Brave bookmarks path and an unwanted newline; replace the value so it is a
single line, remove the " entity, and correct the Brave path to
"%LOCALAPPDATA%\BraveSoftware\Brave-Browser\User Data" (note the space) while
keeping the rest of the sentence intact.
<ResourceDictionary xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:system="clr-namespace:System;assembly=mscorlib"> | ||
|
||
<!-- Plugin Info --> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_name">Marcadores del navegador</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_description">Busca en los marcadores del navegador</system:String> | ||
|
||
<!-- Main --> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">No se ha podido establecer la url en el portapeles</system:String> | ||
|
||
<!-- Settings --> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_bookmarkDataSetting">Datos del marcador</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_settings_openBookmarks">Abrir marcadores en:</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_settings_newWindow">Nueva ventana</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_settings_newTab">Nueva pestaña</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_settings_setBrowserFromPath">Establecer navegador desde ruta:</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_settings_choose">Elegir</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Copiar url</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Copia la url del marcador al portapapeles</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_loadBrowserFrom">Cargar navegador desde:</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_browserName">Nombre del navegador</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_browserBookmarkDataDirectory">Ruta del directorio de datos</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_addBrowserBookmark">Añadir</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_editBrowserBookmark">Editar</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_removeBrowserBookmark">Eliminar</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_browseBrowserBookmark">Navegar</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_others">Otros</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Motor del navegador</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">Si no está utilizando Chrome, Firefox o Edge, o si está utilizando su versión portable, debe añadir el directorio de datos de los marcadores y seleccionar el motor del navegador correcto para que este complemento funcione.</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Por ejemplo: El motor de Brave es Chromium; y la ubicación por defecto de los datos de los marcadores es: "%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData". Para el motor de Firefox, el directorio de los marcadores es la carpeta de datos del usuario que contiene el archivo places.sqlite.</system:String> | ||
<system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Cargar favicons (puede llevar mucho tiempo durante el arranque)</system:String> | ||
|
||
</ResourceDictionary> |
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.
🧩 Analysis chain
Ensure parity with en.xaml (new strings likely missing).
Given the new features (favicon fetching, watcher options, auto-detection), Spanish likely lacks new keys added in English. Sync to avoid fallback.
Generate a missing-keys report:
I can help draft Spanish strings once you confirm the missing keys.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Extract and diff keys ignoring prefix variants
rg -nPo 'x:Key="\K[^"]+' -- Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/en.xaml | sort > /tmp/en.keys
rg -nPo 'x:Key="\K[^"]+' -- Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/es.xaml | sort > /tmp/es.keys
echo "Keys in EN but not in ES:"
comm -23 /tmp/en.keys /tmp/es.keys || true
Length of output: 1247
Add missing Spanish translation keys to es.xaml
EN keys present in Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/en.xaml but missing from Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/es.xaml — add Spanish translations for:
- flowlauncher_plugin_browserbookmark_plugin_name
- flowlauncher_plugin_browserbookmark_plugin_description
- flowlauncher_plugin_browserbookmark_copy_failed
- flowlauncher_plugin_browserbookmark_copyurl_title
- flowlauncher_plugin_browserbookmark_copyurl_subtitle
- flowlauncher_plugin_browserbookmark_loadBrowserFrom
- flowlauncher_plugin_browserbookmark_others
- flowlauncher_plugin_browserbookmark_addBrowserBookmark
- flowlauncher_plugin_browserbookmark_editBrowserBookmark
- flowlauncher_plugin_browserbookmark_removeBrowserBookmark
- flowlauncher_plugin_browserbookmark_enable_favicons
- flowlauncher_plugin_browserbookmark_fetch_missing_favicons
- flowlauncher_plugin_browserbookmark_bookmarkDataSetting
- flowlauncher_plugin_browserbookmark_browserName
- flowlauncher_plugin_browserbookmark_browserBookmarkDataDirectory
- flowlauncher_plugin_browserbookmark_browseBrowserBookmark
- flowlauncher_plugin_browserbookmark_guideMessage03
- flowlauncher_plugin_browserbookmark_engine_detection_invalid
- flowlauncher_plugin_browserbookmark_engine_detection_chromium
- flowlauncher_plugin_browserbookmark_engine_detection_firefox
- flowlauncher_plugin_browserbookmark_engine_detection_select_directory
I can draft the Spanish strings for these keys.
🛠️ Refactor suggestion
🧩 Analysis chain
Crowdin path and migration note (namespace change).
Since files moved to Plugin.BrowserBookmarks/…
, update Crowdin config/mapping to preserve translation memory and avoid resetting locale progress.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate Crowdin config and check paths referencing old BrowserBookmark location
fd -a 'crowdin*.yml' .
rg -n 'BrowserBookmark' -g 'crowdin*.yml' -n || true
rg -n 'BrowserBookmarks' -g 'crowdin*.yml' -n || true
Length of output: 30
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="/home/jailuser/git/crowdin.yml"
echo "FILE: $file"
rg -n --hidden -S 'BrowserBookmark|BrowserBookmarks|Plugin.BrowserBookmarks|Flow.Launcher.Plugin.BrowserBookmarks' "$file" || true
echo "---- START FILE CONTENT ----"
sed -n '1,240p' "$file" || true
echo "---- END FILE CONTENT ----"
Length of output: 311
Update Crowdin config to include plugin Language files (preserve translation memory)
crowdin.yml at repo root currently only maps Languages/en.xaml and Properties/Resources.resx; it does not match Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/*, so translations for moved plugin files will be lost.
- Edit crowdin.yml (repo root) to add a glob or explicit source for plugin language files, e.g.:
- source: "Plugins/**/Languages/en.xaml"
translation: "/%original_path%/%two_letters_code%.xaml"
or add an explicit entry for Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/en.xaml.
- source: "Plugins/**/Languages/en.xaml"
- Run a Crowdin import/dry-run to confirm existing translations map to the new paths.
🤖 Prompt for AI Agents
In Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/es.xaml lines 2-33,
the review notes Crowdin config at the repo root currently only maps
Languages/en.xaml and Properties/Resources.resx so plugin language files (like
this one) aren’t included and translations will be lost; fix by editing
crowdin.yml to add a source mapping for plugin language files (either a glob
such as Plugins/**/Languages/en.xaml with a translation pattern like
/%original_path%/%two_letters_code%.xaml or an explicit mapping for
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/en.xaml), commit the
updated crowdin.yml, then run a Crowdin import or dry-run to verify existing
translations map to the new paths.
private void TriggerDebouncedReload() | ||
{ | ||
// Reset the timer to fire after 2 seconds. | ||
// This prevents multiple reloads if a browser writes to the file several times in quick succession. | ||
lock (_lock) | ||
{ | ||
_debounceTimer.Change(2000, Timeout.Infinite); | ||
} | ||
} | ||
|
||
public void Dispose() | ||
{ | ||
_debounceTimer?.Dispose(); | ||
foreach (var watcher in _watchers) | ||
{ | ||
watcher.EnableRaisingEvents = false; | ||
watcher.Dispose(); | ||
} | ||
} |
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.
Timer dispose race can throw ObjectDisposedException.
TriggerDebouncedReload may call Change while Dispose is freeing the timer. Guard with a disposed flag and lock; also unsubscribe watcher events before dispose.
@@
- // Timer to debounce file change events
- private Timer _debounceTimer;
+ // Timer to debounce file change events
+ private Timer _debounceTimer;
private readonly object _lock = new();
+ private volatile bool _disposed;
@@
private void TriggerDebouncedReload()
{
// Reset the timer to fire after 2 seconds.
// This prevents multiple reloads if a browser writes to the file several times in quick succession.
lock (_lock)
{
- _debounceTimer.Change(2000, Timeout.Infinite);
+ if (_disposed) return;
+ _debounceTimer?.Change(2000, Timeout.Infinite);
}
}
@@
public void Dispose()
{
- _debounceTimer?.Dispose();
- foreach (var watcher in _watchers)
- {
- watcher.EnableRaisingEvents = false;
- watcher.Dispose();
- }
+ lock (_lock)
+ {
+ _disposed = true;
+ _debounceTimer?.Change(Timeout.Infinite, Timeout.Infinite);
+ _debounceTimer?.Dispose();
+ _debounceTimer = null;
+ }
+ foreach (var watcher in _watchers)
+ {
+ watcher.EnableRaisingEvents = false;
+ watcher.Changed -= OnFileChanged;
+ watcher.Created -= OnFileChanged;
+ watcher.Deleted -= OnFileChanged;
+ watcher.Renamed -= OnFileRenamed;
+ watcher.Dispose();
+ }
}
📝 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.
private void TriggerDebouncedReload() | |
{ | |
// Reset the timer to fire after 2 seconds. | |
// This prevents multiple reloads if a browser writes to the file several times in quick succession. | |
lock (_lock) | |
{ | |
_debounceTimer.Change(2000, Timeout.Infinite); | |
} | |
} | |
public void Dispose() | |
{ | |
_debounceTimer?.Dispose(); | |
foreach (var watcher in _watchers) | |
{ | |
watcher.EnableRaisingEvents = false; | |
watcher.Dispose(); | |
} | |
} | |
// Timer to debounce file change events | |
private Timer _debounceTimer; | |
private readonly object _lock = new(); | |
private volatile bool _disposed; | |
private void TriggerDebouncedReload() | |
{ | |
// Reset the timer to fire after 2 seconds. | |
// This prevents multiple reloads if a browser writes to the file several times in quick succession. | |
lock (_lock) | |
{ | |
if (_disposed) return; | |
_debounceTimer?.Change(2000, Timeout.Infinite); | |
} | |
} | |
public void Dispose() | |
{ | |
lock (_lock) | |
{ | |
_disposed = true; | |
_debounceTimer?.Change(Timeout.Infinite, Timeout.Infinite); | |
_debounceTimer?.Dispose(); | |
_debounceTimer = null; | |
} | |
foreach (var watcher in _watchers) | |
{ | |
watcher.EnableRaisingEvents = false; | |
watcher.Changed -= OnFileChanged; | |
watcher.Created -= OnFileChanged; | |
watcher.Deleted -= OnFileChanged; | |
watcher.Renamed -= OnFileRenamed; | |
watcher.Dispose(); | |
} | |
} |
🤖 Prompt for AI Agents
In
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/BookmarkWatcherService.cs
around lines 66-84, TriggerDebouncedReload can call _debounceTimer.Change while
Dispose is running and freeing the timer, causing ObjectDisposedException; fix
by adding a private bool _disposed flag, wrap accesses to _debounceTimer and
_disposed in the same lock used currently so TriggerDebouncedReload checks
_disposed before calling Change, and in Dispose acquire the lock, set _disposed
= true, stop/unsubscribe all watcher events (remove event handlers) before
setting EnableRaisingEvents = false and disposing them, then safely dispose
_debounceTimer and release the lock so no Change can race with disposal.
using System.Net.Http; | ||
using System.Security.Cryptography; |
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.
Enable HTTP decompression (fix gzipped HTML parsing).
Without AutomaticDecompression, many pages return gzipped/br content and HTML parsing will see binary. Enable it at handler construction.
Apply this diff:
using System.Net.Http;
+using System.Net;
...
- var handler = new HttpClientHandler { AllowAutoRedirect = true };
+ var handler = new HttpClientHandler
+ {
+ AllowAutoRedirect = true,
+ AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate | DecompressionMethods.Brotli
+ };
_httpClient = new HttpClient(handler);
Also applies to: 54-59
🤖 Prompt for AI Agents
In Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/FaviconService.cs
around lines 10-11 (and also apply same fix to lines 54-59), the HttpClient is
created without enabling AutomaticDecompression, causing gzipped/br responses to
be left compressed and parsed as binary; update the HttpClientHandler
construction to set AutomaticDecompression = DecompressionMethods.GZip |
DecompressionMethods.Deflate | DecompressionMethods.Brotli (or include Brotli if
available) and pass that handler into the HttpClient so responses are
automatically decompressed before HTML parsing.
private readonly string _faviconCacheDir; | ||
private readonly HttpClient _httpClient; | ||
private readonly LocalFaviconExtractor _localExtractor; | ||
|
||
private readonly ConcurrentDictionary<string, Task<string?>> _ongoingFetches = new(); | ||
|
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.
Decouple in‑flight fetch dedup from caller tokens; also make keys case‑insensitive and cancel on dispose.
A single caller cancellation currently cancels the shared in‑flight task for the same authority, causing spurious misses for other callers. Use a service‑level CTS and case‑insensitive keys; cancel outstanding work on Dispose.
Apply this diff:
private readonly Settings _settings;
private readonly string _faviconCacheDir;
private readonly HttpClient _httpClient;
private readonly LocalFaviconExtractor _localExtractor;
+ private readonly CancellationTokenSource _disposeCts = new();
- private readonly ConcurrentDictionary<string, Task<string?>> _ongoingFetches = new();
+ private readonly ConcurrentDictionary<string, Task<string?>> _ongoingFetches =
+ new(StringComparer.OrdinalIgnoreCase);
...
- private Task<string?> GetFaviconFromWebAsync(Uri url, CancellationToken token)
+ private Task<string?> GetFaviconFromWebAsync(Uri url, CancellationToken token)
{
if (url is null || (url.Scheme != "http" && url.Scheme != "https"))
{
return Task.FromResult<string?>(null);
}
var authority = url.GetLeftPart(UriPartial.Authority);
- return _ongoingFetches.GetOrAdd(authority, key => FetchAndCacheFaviconAsync(new Uri(key), token));
+ // Decouple the shared fetch task from per-caller tokens
+ return _ongoingFetches.GetOrAdd(authority,
+ key => FetchAndCacheFaviconAsync(new Uri(key), CancellationToken.None));
}
...
- using var overallCts = CancellationTokenSource.CreateLinkedTokenSource(token);
+ using var overallCts = CancellationTokenSource.CreateLinkedTokenSource(_disposeCts.Token);
overallCts.CancelAfter(TimeSpan.FromSeconds(10));
var linkedToken = overallCts.Token;
And dispose cancellation:
public void Dispose()
{
+ _disposeCts.Cancel();
_httpClient.Dispose();
GC.SuppressFinalize(this);
}
Also applies to: 101-109, 128-131, 421-425
🤖 Prompt for AI Agents
In Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/FaviconService.cs
around lines 24-29 (and also update occurrences at 101-109, 128-131, 421-425):
the current in‑flight fetch dictionary and task creation use caller-provided
CancellationTokens and case-sensitive keys which lets one caller cancel the
shared task and treats equivalent hosts as distinct; change the
ConcurrentDictionary to use a case‑insensitive string comparer, introduce a
service‑level CancellationTokenSource (_cts) used for all in‑flight operations,
and ensure when starting fetch tasks you only use the service token (or link
caller token for local cancellation without cancelling the shared task) so
individual caller cancellation does not cancel the shared fetch; when creating
linked tokens use CancellationTokenSource.CreateLinkedTokenSource(service token,
caller token) only for per‑caller waiters while the underlying fetch uses just
the service token, and on Dispose cancel the service _cts and clear/cancel
outstanding work so all in‑flight fetches stop promptly.
private async Task<FetchResult> FetchAndProcessUrlAsync(Uri faviconUri, CancellationToken token) | ||
{ | ||
var tempPath = Path.GetTempFileName(); | ||
try | ||
{ | ||
_context.API.LogDebug(nameof(FaviconService), $"Attempting to fetch favicon: {faviconUri}"); | ||
using var request = new HttpRequestMessage(HttpMethod.Get, faviconUri); | ||
var response = await _httpClient.SendAsync(request, HttpCompletionOption.ResponseContentRead, token); | ||
|
||
if (!response.IsSuccessStatusCode) | ||
{ | ||
_context.API.LogDebug(nameof(FaviconService), $"Fetch failed for {faviconUri} with status code {response.StatusCode}"); | ||
File.Delete(tempPath); | ||
return default; | ||
} |
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.
Guard against oversized downloads (DoS/memory).
Add a simple max size check before reading image bodies.
Apply this diff:
private async Task<FetchResult> FetchAndProcessUrlAsync(Uri faviconUri, CancellationToken token)
{
var tempPath = Path.GetTempFileName();
try
{
_context.API.LogDebug(nameof(FaviconService), $"Attempting to fetch favicon: {faviconUri}");
using var request = new HttpRequestMessage(HttpMethod.Get, faviconUri);
var response = await _httpClient.SendAsync(request, HttpCompletionOption.ResponseContentRead, token);
if (!response.IsSuccessStatusCode)
{
_context.API.LogDebug(nameof(FaviconService), $"Fetch failed for {faviconUri} with status code {response.StatusCode}");
File.Delete(tempPath);
return default;
}
+ // Hard cap (1 MB) to avoid pathological images
+ const long MaxImageBytes = 1_000_000;
+ if (response.Content.Headers.ContentLength is long len && len > MaxImageBytes)
+ {
+ _context.API.LogDebug(nameof(FaviconService), $"Skipping {faviconUri}: content too large ({len} bytes).");
+ return default;
+ }
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/FaviconService.cs
around lines 192-206, the fetch logic reads the response body without size
limits which can lead to DoS/memory issues; add a max allowed bytes constant
(e.g. 1-2MB) and enforce it before and while saving the response: first check
response.Content.Headers.ContentLength and fail early if it exceeds the limit,
then when copying the response stream to the temp file read and write in chunks
while tracking total bytes and abort/delete/return default if the running total
exceeds the limit or cancellation is requested; ensure temp file is deleted and
response/streams are disposed on early exit and that appropriate debug logging
is added for size-rejection cases.
// Try to find the default-release profile first, which is the most common case. | ||
var profileSectionMatch = Regex.Match(iniContent, @"\[Profile[^\]]+\]\s*Name=default-release[\s\S]+?Path=([^\r\n]+)[\s\S]+?Default=1", RegexOptions.IgnoreCase); | ||
|
||
// Fallback to any default profile. | ||
if (!profileSectionMatch.Success) | ||
{ | ||
profileSectionMatch = Regex.Match(iniContent, @"\[Profile[^\]]+\][\s\S]+?Path=([^\r\n]+)[\s\S]+?Default=1", RegexOptions.IgnoreCase); | ||
} | ||
|
||
// Fallback to the first available profile if no default is marked. | ||
if (!profileSectionMatch.Success) | ||
{ | ||
profileSectionMatch = Regex.Match(iniContent, @"\[Profile[^\]]+\][\s\S]+?Path=([^\r\n]+)"); | ||
} |
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.
🛠️ Refactor suggestion
🧩 Analysis chain
Regex patterns could miss valid profiles if Default=1
is on a separate line.
The current regex patterns assume that Path=
and Default=1
are in sequence without other profile sections in between. However, INI format allows these properties to appear on separate lines with other properties interspersed. This could cause valid default profiles to be missed.
Let me verify the typical structure of Firefox profiles.ini:
🏁 Script executed:
#!/bin/bash
# Check if any Firefox profiles.ini files exist and display their structure
fd -H "profiles.ini" | head -5 | while read -r file; do
echo "=== Content of $file ==="
cat "$file" | head -30
echo ""
done
Length of output: 0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file
echo "Searching for FirefoxProfileFinder.cs..."
file_path=$(fd -HI --hidden --no-ignore-vcs "FirefoxProfileFinder.cs" || true)
if [ -z "$file_path" ]; then
echo "FILE_NOT_FOUND"
exit 0
fi
echo "Found: $file_path"
echo "---- Full file (first 400 lines) ----"
sed -n '1,400p' "$file_path" || true
echo ""
echo "---- Context around lines 40-80 ----"
sed -n '40,80p' "$file_path" || true
echo ""
echo "---- Lines containing 'Regex' or 'Profile' ----"
rg -n --hidden --no-ignore-vcs "Regex|\\[Profile|profiles.ini|Path=|Default=" "$file_path" || true
Length of output: 5543
Restrict regex to a single [Profile] section and accept Default=1 before or after Path.
Current regexes in Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/FirefoxProfileFinder.cs (≈ lines 52–63) can fail when Default=1 appears before Path or incorrectly pair a Path from one profile with Default=1 from another because [\s\S]+? can cross section boundaries.
- Fix: parse profiles.ini by profile sections (split on ^[Profile) and inspect each section's Path/Default/Name/IsRelative (recommended), or update the regex to forbid matching across the next "[Profile" and allow Default=1 to appear either before or after Path.
The only issue I see here is that users would lose manually entered custom browsers. Caches would be rebuilt so that would be transparent to the user. Plus, they have a new option to choose either way. I can easily switch this back to |
@jjw24 Any ideas about this? |
|
@jjw24 - 👍
Yes, completely refactored, rewritten. The only thing I have reused / updated is XAML/.cs |
I'm closing this and opening a new PR to replace the existing Note: I have incorporated all feedback from this PR in to the new code. |
Replacement for
BrowserBookmark
PluginThis is a new plugin to provide browser bookmarks and favicon functionality. It replaces the existing 'BrowserBookmark` plugin. It improves performance, extensibility, and UX over the original.
Favicons are now available without the need to restart the app, with instant updates for Chrome/ium browsers and a three-hour refresh cycle for Firefox.
Features & Enhancements
Favicon Handling:
Real-time & Periodic Updates:
FileSystemWatcher
with debouncing to prevent excessive reloads.places.sqlite
file, Firefox bookmarks are now reloaded on a periodic 3-hour timer.Architectural & Performance
Fully Asynchronous Loading: The entire bookmark and favicon loading pipeline is asynchronous (
async/await
).Parallel Processing: Favicon processing is parallelized to significantly speed up the initial caching process.
Separation of Concerns: The logic is cleanly divided:
BookmarkLoaderService
: Handles discovering and loading bookmarks from different browser types.BookmarkWatcherService
: Manages real-time file system monitoring for Chromium browsers.FaviconService
: Responsible for all favicon extraction, fetching, and caching.BrowserDetector
: Contains the logic to automatically identify a browser's engine from its directory path.UX & UI Improvements