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.

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

Feature: Using DuckDuckGo for favicons #2258

Closed
nfitzen opened this issue Sep 1, 2018 · 7 comments · Fixed by #2309
Closed

Feature: Using DuckDuckGo for favicons #2258

nfitzen opened this issue Sep 1, 2018 · 7 comments · Fixed by #2309
Milestone

Comments

@nfitzen
Copy link

nfitzen commented Sep 1, 2018

I see that in the settings one can use Google for downloading site favicons in case there's no favicon.ico. Can you also add DuckDuckGo's favicon service to this? The format for the image is https://icons.duckduckgo.com/ip3/www.example.com.ico

For example, Google's icon is here: https://icons.duckduckgo.com/ip3/www.google.com.ico

Thank you for reading.

@TheZ3ro
Copy link
Contributor

TheZ3ro commented Sep 3, 2018

seems a very good alternative to the google service

@RealOrangeOne
Copy link
Contributor

With some testing, it also looks like the DuckDuckGo API parses the HTML meta tag rather than naively looking for /favicon.ico, which is really nice! Should drastically increase the hit-rate of icons

@droidmonkey droidmonkey added this to the v2.4.0 milestone Sep 5, 2018
@kneitinger
Copy link
Contributor

I'd like to work on this issue this week, if nobody else has plans to.

Also, to clarify, although @nfitzen originally said "also add", should we consider simply replacing Google with DuckDuckGo?

@droidmonkey
Copy link
Member

droidmonkey commented Sep 19, 2018

Replacing is perfectly acceptable. We did not have any real reason behind Google besides it was available. Please do work on this, I will review and merge when you post the PR.

@kneitinger
Copy link
Contributor

I've implemented this, as well as a fix for #2265, but this will be my first PR for this repo, and I have a couple of questions before opening it.

  1. I've changed the messages Hint: You can enable Google as a fallback under Tools>Settings>Security and Use Google as fallback for downloading website icons to reference DuckDuckGo; how do I deal with translations? Edit only share/translations/keepassx_en_US.ts?
  2. When I run make format from my build dir, it modifies ~110 other files, should I skip this step, and only run clang format against my files?

Thanks!

@droidmonkey
Copy link
Member

Good deal, for (1) don't worry about translations, that is handled by Transifex separately from GitHub. For (2) don't run format at all, we will take care of formatting if necessary.

kneitinger added a commit to kneitinger/keepassxc that referenced this issue Sep 19, 2018
kneitinger added a commit to kneitinger/keepassxc that referenced this issue Sep 20, 2018
droidmonkey pushed a commit to kneitinger/keepassxc that referenced this issue Sep 20, 2018
droidmonkey pushed a commit that referenced this issue Sep 21, 2018
…er (#2309)

* Replace Google with DuckDuckGo for optional fallback favicon fetch URL
Modify the work initially done in #36, and most recently modified in #1786,
to use DuckDuckGo's https://icons.duckduckgo.com/ip3/www.example.com.ico
favicon endpoint.

Fixes #2258

* Close failed favicon fetch progress bars

Name the UrlFetchProgressDialog() with the corresponding URL in order to
be identified by name by its parent when the failed request is handeled
in EditWidgetIcons::fetchFinished(). fetchFinished() retrieves the
relevant UrlFetchProgressDialog() and calls close() on it.

Fixes: #2265
@gilluc
Copy link

gilluc commented Mar 29, 2022

thank you.
i added : width="32" height="32"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants