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

FaviconFinder no longer retrieves favicon when header image is present #103

Closed
mergesort opened this issue Oct 21, 2024 · 2 comments · Fixed by #104
Closed

FaviconFinder no longer retrieves favicon when header image is present #103

mergesort opened this issue Oct 21, 2024 · 2 comments · Fixed by #104
Assignees
Labels
bug Something isn't working

Comments

@mergesort
Copy link
Contributor

mergesort commented Oct 21, 2024

I've been using this code to fetch favicons, and it's been relatively unchanged. Since upgrading to 5.1.3 (from a previous 5.x version, I don't remember which one specifically because I was working off my own branch with other fixes), instead of returning favicons the library is now returning header images, when they are present on a website.

You can see it here, in the top left icon where I display favicons, it's now displaying the header image that my websites have. That means when a user saves a link, they now see the header image presented twice, rather a favicon and the header image.

This is the code I'm using to retrieve the Favicon, with seemingly no major changes since upgrading.

struct FaviconParser {
    static func fetchFaviconURL(from url: URL) async throws -> ImageMetadata? {
        let faviconFinder = FaviconFinder(url: url)
        let favicons = try? await faviconFinder.fetchFaviconURLs()

        guard let favicon = try? await favicons?.largest() else { return nil }

        return ImageMetadata(
            url: favicon.source,
            width: nil,
            height: nil
        )
    }
}

If this is new functionality that has been added, can there be an explicit way to exclude header images from being parsed? (Or better yet, specify that you wish to include header images when scraping a webpage for favicons.)

Thank you very much as always @will-lumley!

@will-lumley will-lumley added the bug Something isn't working label Nov 3, 2024
@will-lumley
Copy link
Owner

@mergesort Thanks for raising this. There is now a parameter named acceptHeaderImage of Bool type in the FaviconFinder.Configuration. This value is false by default, making the functionality opt-in.

Another user had raised this as an issue as they wanted the library to download these image types. I had a knee-jerk reaction which led to me implementing it immediately without thinking of the consequences. I see now that I should have thought about the wider reaching effects of this - and will think about these sorts of things going forward.

Thanks again for reaching out and taking the time to raise this issue, and for your patience.

@mergesort
Copy link
Contributor Author

Thank you very much for this fix! I can definitely understand how you got there, it does seem like a helpful feature. But I do appreciate the reversion and keeping the default as it was before. 🙇🏻‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants