This plugin changes Last Modification Time even for entries which it does not modify

navossoc / KeePass-Yet-Another-Favicon-Downloader

Yet Another Favicon Downloader for KeePass 2.x

MIT License

537 stars 31 forks source link

This plugin changes Last Modification Time even for entries which it does not modify #35

Closed NeatNit closed 4 years ago

NeatNit commented 4 years ago

Title says it all.

Reproduction:

Create or find an entry with no URL (or which would otherwise fail to provide a favicon)
Right-click -> Download Favicons
YAFD reports "Error" (or "Not Found"), and the entry will not be modified but the entry's Last Modification Time will be updated to the current time.

Expected behavior: the Last Modification Time should not be updated whenever the icon is not updated, i.e. in any of the following situations:

Could not find a URL
The website has no favicon
The website has a favicon, but it hasn't changed since the last time this plugin was used (compare the newly downloaded icon with the existing icon)

navossoc commented 4 years ago

Title says it all.

It says all and a little bit more. 🤣

Anyway, that is a good point.

Please check it here: https://github.com/navossoc/KeePass-Yet-Another-Favicon-Downloader/releases/tag/v1.2.3.0-pre

Probably I'll release it next week, if everything goes smoothly.

Cheers.

[]'s

NeatNit commented 4 years ago

Seems to work fine in a basic check, thanks! That was awesomely quick!

Edit: out of curiosity, does this plugin check websites for updated favicons if they've been downloaded in the past? and how does it recognize whether it's been changed? (does it compare the old file and new file, or what?)

navossoc commented 4 years ago

@NeatNit it will always download the favicon. Then it will determine if the icon has been changed hashing the downloaded data of the icon.

So, if you download a icon today and in the next month you download the same exactly file it will produce the same hash and will not be change the icon on the entry (with this changes you proposed).

It also uses the same hash to share a custom icon between multiple entries, avoiding cluttering the database with many resources.

JohnLGalt commented 4 years ago

Very nice implementation! Thanks!

navossoc commented 4 years ago

So @NeatNit, did you find any problems? I think it is good to go public.

NeatNit commented 4 years ago

Honestly I didn't use it at all after that comment, I don't have much more to do with it :P

But in my short testing back then it was fine. So yeah, go ahead.