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

Custom icons duplicated #232

Closed
louib opened this issue Jan 27, 2017 · 7 comments
Closed

Custom icons duplicated #232

louib opened this issue Jan 27, 2017 · 7 comments
Assignees
Milestone

Comments

@louib
Copy link
Member

louib commented Jan 27, 2017

Custom icons for the same website are added multiple times.

Expected Behavior

We should at least try to detect duplicates and not fetch the icon multiple times for the same website.

Current Behavior

Custom icons for the same website are added multiple times.

screen shot 2017-01-27 at 1 09 20 pm

Steps to Reproduce (for bugs)

  1. Add entry with a website
  2. Click Download favicon multiple times.

Context

I was trying to reproduce #225

Your Environment

  • KeePassXC version/commit used: 6409661
  • Qt version: Qt 5.7.1
  • Operating System and version: MacOS 10.11.16
@phoerious phoerious added the bug label Jan 27, 2017
@phoerious phoerious added this to the v2.2.0 milestone Jan 27, 2017
@droidmonkey
Copy link
Member

Would you match the base url only? Some sites have different icons for sub domains.

@louib
Copy link
Member Author

louib commented Jan 28, 2017

@droidmonkey this and making sure clicking multiple times download from within the window just fetches the icon 1 time.

@paolostivanin
Copy link

paolostivanin commented Feb 7, 2017

Maybe by comparing the checksum? I think it's always the same if the icon is the same (haven't tried).
Could be worth a try though :)

@droidmonkey
Copy link
Member

That's a good idea although it would be best to precalculate and store the checksum/hash in the db somehow.

@paolostivanin
Copy link

Something like an hash table maybe?

{ 
   google.com:
      { icon: favicon.ico,
        cks: checksum
      },
   gmail.google.com:
      { icon: favicon.ico,
        cks: checksum
      },
}

@phoerious
Copy link
Member

phoerious commented Feb 7, 2017 via email

@paolostivanin
Copy link

Yep, good idea!

@droidmonkey droidmonkey modified the milestones: v2.2.0, v2.2.1 May 30, 2017
@droidmonkey droidmonkey self-assigned this Sep 23, 2017
droidmonkey added a commit that referenced this issue Sep 24, 2017
* Fixes #904, icons are saved at or below 128x128
* Fixes #403, crash occurs due to dialog on non-gui thread
* Fixes #232, icon hashes calculated and compared against
droidmonkey added a commit that referenced this issue Sep 27, 2017
* Fixes #904, icons are saved at or below 128x128
* Fixes #403, crash occurs due to dialog on non-gui thread
* Fixes #232, icon hashes calculated and compared against
droidmonkey added a commit that referenced this issue Sep 29, 2017
* Fixes #904, icons are saved at or below 128x128
* Fixes #403, crash occurs due to dialog on non-gui thread
* Fixes #232, icon hashes calculated and compared against
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

No branches or pull requests

4 participants