will-lumley / FaviconFinder

A small swift library for iOS & macOS to detect favicons used by a website.
MIT License
161 stars 33 forks source link

Fastest Way To Get Favicon With Sizes #77

Closed mergesort closed 1 month ago

mergesort commented 3 months ago

Hey @will-lumley, I've been using your project for a while now and wanted to say thank you, and thank you for adding Linux support!

I have code that looks like this, which runs async as part of a process where I download a whole bunch of metadata for a website.

struct FaviconParser {
    static func fetchFaviconURL(from url: URL) async throws -> ImageMetadata? {
        guard let favicon = try? await FaviconFinder(url: url).downloadFavicon() else { return nil }

        let width: CGFloat?
        let height: CGFloat?

        #if os(Linux)
        width = nil
        height = nil
        #else
        width = favicon.image.size.width
        height = favicon.image.size.height
        #endif

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

I was upgrading to 5.0 today and noticed that the code wouldn't compile. I tried to do a naive translation to what I found in the updated documentation, but hadn't quite come up with a way to accomplish the same task with these two conditions.

  1. I'd like to get the image's size, even when on Linux.

    • This seems possible by manually parsing the sizeTag string, but that seems like a downgrade over the previous type-safe width and height that were provided by the API.
    • Are these size tags always guaranteed to be a string in the format of (widthxheight)? I've only come across integers so far, but is it possible for a height to be 64.5px tall?
  2. I'd like to avoid downloading the whole image to derive a URL and size, so the operation is fast.

    • I don't know if the 4.x series was doing that? I had assumed (maybe mistakenly) it was only fetching the URL itself.
    • Is the best way to do this to call faviconFinder.fetchFaviconURLs() and iterate through all of the asset's sizeTags to see which one is the largest? I saw in the documentation you can call faviconFinder.fetchFaviconURLs().download().largest() but that will download all of the images to compute their sizes rather than using the already available sizeTags.

This was the naive implementation I came up with when trying to migrate to 5.0, but it assumes that the first image is the largest rather than using the sizeTags to figure out which image is the largest. It should be easy enough to do, but I wanted to post this and ask first to make sure I'm on the right track.

struct FaviconParser {
    static func fetchFaviconURL(from url: URL) async throws -> ImageMetadata? {
        let faviconFinder = FaviconFinder(url: url)
        guard let favicon = try await faviconFinder.fetchFaviconURLs().first else { return nil }

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

extension FaviconURL {
    var width: CGFloat? {
        guard let sizeTag else { return nil }

        if let width = self.parseDimensions(from: sizeTag)?.width {
            return CGFloat(width)
        } else {
            return nil
        }
    }

    var height: CGFloat? {
        guard let sizeTag else { return nil }

        if let height = self.parseDimensions(from: sizeTag)?.height {
            return CGFloat(height)
        } else {
            return nil
        }
    }

    private func parseDimensions(from input: String) -> (width: Double, height: Double)? {
        guard let components = self.sizeTag?.split(separator: "x") else { return nil }

        guard components.count == 2 else { return nil }

        guard let width = Double(components[0]), let height = Double(components[1]) else {
            return nil
        }

        return (width, height)
    }
}

Would love to have your guidance, I'm sure that my solution isn't as good as it could be, and think that it likely would be good to have built into the library itself. Thanks again!

will-lumley commented 1 month ago

Hi @mergesort. First of all, thank you for the thank you - it's very appreciated. Second of all, sorry it took me so long to get around to going through the issues & PRs - will be working on increasing my response times going forward. Third of all - this is a great feature request, and one that is definitely needed.

You're right that the FaviconFinder API has changed, as it was a bit clunky when you tried to do anything advanced. Now you can download the FaviconURLs, download them, and sort them, etc.

I appreciate the code you provided, and will use them to integrate a solution built directly into the library.

You'll hear back from me soon!

will-lumley commented 1 month ago

@mergesort Can you let me know if the functionality provided in the latest PR gives you what you're after?