Skip to content
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.

Sign up for GitHub

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

Implemented suggested changes for PR#3079 #3360

Closed
wants to merge 1 commit into from
Closed

Implemented suggested changes for PR#3079 #3360

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ public class Main : ISettingProvider, IPlugin, IReloadable, IPluginI18n, IContex

private static List<Bookmark> _cachedBookmarks = new List<Bookmark>();

private static Dictionary<string, string> faviconCache = new Dictionary<string, string>();

private static Settings _settings;

private static bool _initialized = false;
Expand Down Expand Up @@ -68,7 +70,7 @@ public List<Result> Query(Query query)
{
Title = c.Name,
SubTitle = c.Url,
IcoPath = @"Images\bookmark.png",
IcoPath = GetFaviconPath(c.Url),
Score = BookmarkLoader.MatchProgram(c, param).Score,
Action = _ =>
{
Expand All @@ -90,7 +92,7 @@ public List<Result> Query(Query query)
{
Title = c.Name,
SubTitle = c.Url,
IcoPath = @"Images\bookmark.png",
IcoPath = GetFaviconPath(c.Url),
Score = 5,
Action = _ =>
{
Expand All @@ -104,6 +106,35 @@ public List<Result> Query(Query query)
}
}

private string GetFaviconPath(string url)
{
try
{
var uri = new Uri(url);
var domain = uri.Host;

if (faviconCache.TryGetValue(domain, out var cachedFaviconUrl))
{
return cachedFaviconUrl;
}
else
{
var encodedDomain = Uri.EscapeDataString(domain);
var faviconUrl = $"https://www.google.com/s2/favicons?domain={encodedDomain}&sz=64";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since quite a lot of our users are in China, where Google is banned, do we have another option for favicon api? I did a bit research and can't find a reliable one. So what about making it customizable or a toggle?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VictoriousRaptor I'm trying to find a way to retrieve bookmark data from local storage.
Even though I don’t know much about programming, I can tell that this approach shouldn’t be used.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@onesounds If your moving forward with PR #3361 would you like me to close this PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ZainGill45 I'll close it. Thanks for the PR.


// Store the favicon URL in the cache
faviconCache[domain] = faviconUrl;
return faviconUrl;
}
}
catch (Exception ex)
{
Log.Exception("Main", "Failed to generate favicon URL", ex);

// Fallback to default icon
return @"Images\bookmark.png";
}
}

private static Channel<byte> _refreshQueue = Channel.CreateBounded<byte>(1);

Expand Down
Loading