luckyrat / KeePass-Favicon-Downloader

A KeePass plugin that downloads and stores favicons. A favicon is the little icon / logo used to identify many websites, typically displayed in the browser's address bar, bookmark list and on tabs.
70 stars 19 forks source link

Minor code cleaning: #5

Closed boxmaker closed 10 years ago

boxmaker commented 10 years ago

Code cleaning:

Replace url and protocol string with Uri object. Remove useless function reconcileURI. (Class Uri have constructor with same functionality). function getFaviconStandardLocation replaced with one line:252. The Uri class overrides Equals and implements the == operator in order to determine equality (line:435)

Fix error:

Check present protocol in url (line:244) (now URL field without scheme [like dropbox.com instead http://dropbox.com] finaly work correct). Add set UserAgent (line: 340 & 425) some site don't response without it (example turbosquid.com) getFaviconExplicitLocation return responseURI which uses to download from standard location (usefull when site move to new domain but in url we store old one).

Update HtmlAgilityPack to last version (1.4.6).

luckyrat commented 10 years ago

Nice work. If you could re-request a pull after one small change, I'll bring this into the next release.

The reconcileURI function behaves slightly differently to the Uri constructor so we need to keep it and use it in the places where you've used the constructor. The reason is that using the standard constructor will throw an exception when the second parameter is a full URI (it requires a relative URI).

If the 2nd parameter passed to the reconcileURI function is an absolute URI, it will be returned, essentially ignoring the first parameter entirely. Since we're dependent on what others have written in their web page source code, we definitely hit some pages where absolute URIs would be fed into that 2nd parameter.

Thanks!