-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add a URL that preserves the URL path when trying to resolve favicons. #8565
Add a URL that preserves the URL path when trying to resolve favicons. #8565
Conversation
Im currently working on a PR that will fix various favicon download related issues by searching the page's HTML. This is just a quick fix that fixes this specific issue with the hope of making the backport. |
Codecov ReportBase: 64.44% // Head: 64.37% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #8565 +/- ##
===========================================
- Coverage 64.44% 64.37% -0.07%
===========================================
Files 340 339 -1
Lines 44142 43996 -146
===========================================
- Hits 28446 28319 -127
+ Misses 15696 15677 -19
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@libklein FYI, the preferred approach based on their wiki to rebase, rather than merge: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Might even be worth a backport, what do you think @droidmonkey?
Add an additional URL that appends /favicon.ico to the path of the URL. This works as follows:
https://google.com/my/path
will becomehttps://google.com/my/favicon.ico
andhttps://google.com/my/path/
will becomehttps://google.com/my/path/favicon.ico
.Fixes #8564.
Testing strategy
Added unit tests.