search

keepassxreboot / keepassxc

KeePassXC is a cross-platform community-driven port of the Windows application “Keepass Password Safe”.
https://keepassxc.org/
Other
20.99k stars 1.45k forks source link

Provide more verbose error messaging for icon download failures #9202

Open Chealer opened 1 year ago

Chealer commented 1 year ago

Overview

When editing an entry which has a URL set, the button at the right to download the website icon is enabled, and activating it normally sets the entry's icon to the website icon.

This unfortunately doesn't work for a large share of entries. I tried it for 15 websites maximum, and many failed, including: https://bandcamp.com https://sso.godaddy.com/ https://fizz.ca https://www.philippecloutier.com https://meetfranz.com

Steps to Reproduce

  1. Create an entry
  2. Set the URL to one which reproduces the problem, such as https://bandcamp.com
  3. Activate the button at the right of the URL field (with an arrow icon and the "Download icon for URL" tooltip)

Expected Behavior

The entry's icon is set to the website's icon.

Actual Behavior

The entry's icon remains unchanged and the following error message appears:

Unable to fetch favicon. 
You can enable the DuckDuckGo website icon service under Tools -> Settings -> Security

There is no explanation of why fetching failed.

Context

Those websites for which downloading failed seem to fail every time, with the exact same error message.

KeePassXC - Version 2.7.4 Revision: 63b2394

Qt 5.15.6 Debugging mode is disabled.

Operating system: Windows 10 Version 2009 CPU architecture: x86_64 Kernel: winnt 10.0.22621

Enabled extensions:

Cryptographic libraries:

droidmonkey commented 1 year ago

Did you enable the duckduckgo setting? It fails because the website defines their favicon with html and not via a well defined location (ie, favicon.ico). This isn't a bug.

Chealer commented 1 year ago

Did you enable the duckduckgo setting?

Unfortunately not. I am evaluating KeePassXC, and nearly all settings are just defaults.

It fails because the website defines their favicon with html and not via a well defined location (ie, favicon.ico).

Hah, welcome to the 2010s. Thanks for your input, but version 5 of HTML was recommended in 2014, and while many websites still provide legacy icons, that's surely not the explanation.

This isn't a bug.

This ticket is indeed a bug report (which the "bug" label indicates). I haven't checked the code, but this must report 1 or 2 defects:

  1. the unexplained failure
  2. probably, the fact that KeePassXC fails to fetch the icon (which could be for a number of reasons)
droidmonkey commented 1 year ago

Why the icon fails is not relevant to the end user, you can't solve it anyway. It either finds the icon or it does not. We have chosen to NOT parse HTML to try and find the favicon defined in one or more elements. Duckduckgo service already does this and you can choose to enable it if you want to.

Chealer commented 1 year ago

Why the icon fails is not relevant to the end user, you can't solve it anyway.

Of course the reason is relevant, but that's not the question. All which needs to be done to resolve this is:

It either finds the icon or it does not. We have chosen to NOT parse HTML to try and find the favicon defined in one or more elements.

The issue here is not that KeePassXC cannot find an icon, but that it cannot fetch it.

I see that you appear to be convinced by your hypothesis. If you are confident enough, I suggest to verify by:

Duckduckgo service already does this and you can choose to enable it if you want to.

It may do that for sites on the Internet, but thanks, that's not a bad workaround.