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

Download icon #1146

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

Conversation

cassnyo
Copy link

@cassnyo cassnyo commented Oct 28, 2021

Related to #596.

  • Adds a new "Download" button in the right side of the URL field.
  • Uses DuckDuckGo's favicon API to search and download the favicon based on the URL.
  • Downloaded favicon is saved to local database. It also avoids duplicated icons if they have been downloaded previously using this functionality.

@J-Jamet
Copy link
Member

J-Jamet commented Oct 29, 2021

Thanks for your pull request but there is a problem. KeePassDX was created to not have internet permissions so I can't integrate it like this (that's why I created the icon download system from the website instead).

Ideally, it should be an extension and the user should be informed during installation that it will add network permissions.

@cassnyo
Copy link
Author

cassnyo commented Nov 19, 2021

That's possible to do, but I don't know how you would like to maintain it because it would need to be a new app (with just a service) uploaded to the Play Store.

I could extract the download logic to a new app and see if it works for you 🤔

@J-Jamet
Copy link
Member

J-Jamet commented Nov 22, 2021

In my mind, if this functionality is integrated it would be an APK to download from github with a link from the app, I don't want to manage separate plugins on the play store.
If you want to extract the logic, yes if you want, right now I'm on other topics so I'll look in detail later.

@dreamsyntax
Copy link

I would be against this as favicon attacks are becoming more common. And as main dev mentioned, no internet permission and it should stay that way.

@sheikh-azharuddin
Copy link

As a temporary solution, why not include a bundle of favicons in the app itself from common websites just like andotp app? While saving the credentials, the app must also smartly choose the favicon based on the url domain if available in database...

Screenshot_20220127-121458854

@Ilithy
Copy link

Ilithy commented Jan 27, 2022

Good idea, we can also imagine creating a community github repo where users could add their favicons to a database (of favicons) that could then be loaded directly into keepassdx (this would avoid having to download a large number of favicons via the keepassdx website)

@J-Jamet
Copy link
Member

J-Jamet commented Jan 27, 2022

I had thought about it and even made an implementation with the smallest possible svg but it still makes the APK a lot heavier
(with raster images, it takes even more space), so a bundle of icons must to be done by plugin in order not to weigh down the application.
I had tried by making a script with the source code of Google which allows to convert SVG (https://github.com/simple-icons/simple-icons) to Android vectors but I also had problems because there were way too many vectors on some images (so made empty images to convert manually). Finally I gave up and makes the website more maintainable.
It is possible for someone who is super motivated but it will be also necessary to maintain the package with constant new icons, so have to make another script that will extract them from the repository to create the plugin.

@sheikh-azharuddin
Copy link

Ok in that case is it not possible to use system installed icon packs to pick the image like how launchers are using?

@J-Jamet
Copy link
Member

J-Jamet commented Jan 27, 2022

That's a good question, I honestly don't know. I would need some sample code to see if the launchers have special permissions and how the icon packs are stored but I think they are just icons uploaded to the launcher space so it's like making icon plugins.

The main problem for me is the number of icons, because there will be more and more and I don't want to favor big services by choosing the most popular ones and not the new ones which can be very legitimate too.

And the more there are, the more maintenance it requires. From memory, there were more than 2000 icons during my tests with SVGs, it made Android studio crash even if they are small files. In addition, we have to make an efficient search system which is much more complicated to do with Android than in javascript with Angular.

@sheikh-azharuddin
Copy link

sheikh-azharuddin commented Jan 27, 2022

I found aegis app, the open source authenticator which is built on same principal local storage is using icon pack feature to add the icons...maybe same can be used here

https://github.com/aegis-icons/aegis-icons/blob/master/FAQ.md#with-icon-pack

@ghost
Copy link

ghost commented Aug 11, 2023

@sheikh-azharuddin Yes, the implementation of Aegis is quite good for this situation, I hope he will consider it in the nearest time possible.

And thank you for the wonderful app.

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

Successfully merging this pull request may close these issues.

None yet

5 participants