Ranchero-Software / NetNewsWire

RSS reader for macOS and iOS.
https://netnewswire.com/
MIT License
8.39k stars 532 forks source link

NNW should use the first supported favicon #911

Open adiabatic opened 5 years ago

adiabatic commented 5 years ago

Alternate title: NNW's metadata grabber selects favicons in formats it doesn't support.

After fiddling around with my website a bit, I ended up with this list of favicons, sorted from fastest to most widely supported:

<link rel='icon' type='image/svg+xml' href='/Images/googly.svg'>
<link rel='icon' type='image/webp'    href='/Images/googly.webp'>
<link rel='icon' type='image/png'     href='/Images/googly.png'>

Unfortunately, NNW will use RSHTMLMetadata's -firstLinkTagWithMatchingRel: and pick the wrong one, leading to a blank spot at the top right of the reading pane and a generic globe icon next to the feed name in the list.


My suggestion

In https://github.com/brentsimmons/RSParser/blob/master/Sources/HTML/RSHTMLMetadata.m, add -(RSHTMLTag*)firstLinkTagMatchingRel:(NSString*)relToMatch andAnyTypeListedMustBeOneOfThese:(NSArray*)types (yes, the name needs work; suggestions gratefully appreciated). This behavior would ensure that sites that haven't bothered to list their favicons' MIME types continue to work.

Then, add a companion to RSHTMLMetadata's -resolvedLinkFromFirstLinkTagWithMatchingRel: called, most likely, -resolvedLinkFromFirstLinkTagWithMatchingRel:andAnyTypeListedMustBeOneOfThese: (again, awful name, help please).

…and then modify RSHTMLMetadata's -initWithURLString:tags: to have something like this line:

    _faviconLink = [self resolvedLinkFromFirstLinkTagWithMatchingRel:kShortcutIconRelValue andAnyTypeListedMustBeOneOfThese:@[@"image/png"]]; // TODO: make and use a proper constant; also verify newfangled NSArray syntax works

Why only PNG? Because it's the only format that gets used for favicons that Apple supports. Why a list? Because Apple might start supporting WebP one day, or there might be .gif favicons out there in the wild that I don't know about.

Incidentally, would this be a candidate for a "good first issue" tag?

gingerbeardman commented 2 years ago

Also of interest:

https://medium.com/web-dev-survey-from-kyoto/favicon-nightmare-how-to-maintain-sanity-7628bfc39918

<link rel="icon" href="/favicon.ico" sizes="any"><!-- 32×32 -->
<link rel="icon" href="/favicon.svg" type="image/svg+xml">
<link rel="apple-touch-icon" href="/apple-touch-icon.png"><!-- 180×180 -->
<link rel="manifest" href="/site.webmanifest">

To that end, I would suggest NetNewsWire supports the favicon in the same way the browser does, according to the spec:

If there are multiple s, the browser uses their media, type, and sizes attributes to select the most appropriate icon. If several icons are equally appropriate, the last one is used. If the most appropriate icon is later found to be inappropriate, for example because it uses an unsupported format, the browser proceeds to the next-most appropriate, and so on. (MDN Contributors 2021a)