Replies: 1 comment
-
|
I'm fine with a simple regex solution. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
The IconDownloader currently tried to find website favicons by either using "https://icons.duckduckgo.com" or by appending "/favicon.ico" to the entry's (resolved) URL and several heuristically determined alternative URLs. This requires several requests to the site and external services, and involves some unnecessarily complicated logic that still misses several corner cases (see #4125).
I'd propose to add an optional feature that bypasses this logic and instead uses the page's source code to find the favicon location. We've already had a discussion regarding this feature 2 years ago (#5509) that raised some concerns w.r.t. parsing the source code, which is why I'd like to discuss this proposal before filing a PR.
I think we can get away with regex parsing. It may fail in some cases (XML is not a regular language) such that an XML based approach would be more "correct" (we'd of course have to make sure to avoid security issues like external references etc., but this is possible with the Qt framework). Still, i believe that the corner cases where regex is insufficient will be too few (if they even exist in the wild) to be worth the extra effort and potential security risks.
Whats your stance on the issue?
Beta Was this translation helpful? Give feedback.
All reactions