cylonid / NativeAlphaForAndroid

GNU General Public License v3.0
642 stars 58 forks source link

Failed to retrieve icon #2

Open the133448 opened 4 years ago

the133448 commented 4 years ago

I've attempted 2 different websites both which have a suitable favicon.ico but get the can't get icon error when attempting to add a shortcut.

cylonid commented 4 years ago

@the133448 Can you post the URLs?

VR-25 commented 4 years ago

This happens, but only sometimes. One of the affected websites is PayPal dot com.

cylonid commented 4 years ago

@VR-25 If you mean by "sometimes" that for one and the same URL sometimes it works and sometimes it does not, then it's most likely a connection issue (there is a timeout for fetching the favicon). Cannot reproduce it for PayPal, however this may be due to the fact PayPal automatically forwards the user to the region-specific version of its website and certain regions may not have a proper icon set. So which country are we talking?

Stimmenhotel commented 3 years ago

I have tried: Facebook.com eBay.de explosm.net

all not receiving any icons.

Paypal seems to work for me.

Since I am on mobile I can't yet check what kind of favicon they use. But while using KeePass I discovered many websites that are not using an dot ico file anymore. Some use svg and the names are all different.

Don't know how Native Alpha checks for them.

Edit: After trying to view the code on my phone I switched to PC.

Github is the first website (have tried about 6) which works with finding a icon.

https://github.com/fluidicon.png
https://github.githubassets.com/pinned-octocat.svg <--- I think this is the only one found by Native Alpha
https://github.githubassets.com/favicons/favicon-dark.png
https://github.githubassets.com/favicons/favicon-dark.svg

Edit2: Wikipedia working too

https://www.wikipedia.org/static/apple-touch/wikipedia.png
https://www.wikipedia.org/static/favicon/wikipedia.ico <--- I think this one, but can't say for sure

These are not working: Ebay.de <link rel=icon href=https://pages.ebay.com/favicon.ico> Facebook: <link rel="shortcut icon" href="https://static.xx.fbcdn.net/rsrc.php/yo/r/iRmz9lCMBD2.ico" /> Explosm.net

<link rel="apple-touch-icon" sizes="57x57" href="//files.explosm.net/img/favicons/site/apple-touch-icon-57x57.png">
<link rel="apple-touch-icon" sizes="60x60" href="//files.explosm.net/img/favicons/site/apple-touch-icon-60x60.png">
<link rel="apple-touch-icon" sizes="72x72" href="//files.explosm.net/img/favicons/site/apple-touch-icon-72x72.png">
<link rel="apple-touch-icon" sizes="76x76" href="//files.explosm.net/img/favicons/site/apple-touch-icon-76x76.png">
<link rel="apple-touch-icon" sizes="114x114" href="//files.explosm.net/img/favicons/site/apple-touch-icon-114x114.png">
<link rel="apple-touch-icon" sizes="120x120" href="//files.explosm.net/img/favicons/site/apple-touch-icon-120x120.png">
<link rel="apple-touch-icon" sizes="144x144" href="//files.explosm.net/img/favicons/site/apple-touch-icon-144x144.png">
<link rel="apple-touch-icon" sizes="152x152" href="//files.explosm.net/img/favicons/site/apple-touch-icon-152x152.png">
<link rel="apple-touch-icon" sizes="180x180" href="//files.explosm.net/img/favicons/site/apple-touch-icon-180x180.png">
<link rel="icon" type="image/png" href="//files.explosm.net/img/favicons/site/favicon-32x32.png" sizes="32x32">
<link rel="icon" type="image/png" href="//files.explosm.net/img/favicons/site/android-chrome-192x192.png" sizes="192x192">
<link rel="icon" type="image/png" href="//files.explosm.net/img/favicons/site/favicon-96x96.png" sizes="96x96">
<link rel="icon" type="image/png" href="//files.explosm.net/img/favicons/site/favicon-16x16.png" sizes="16x16">
<link rel="manifest" href="//files.explosm.net/img/favicons/site/manifest.json">
<link rel="mask-icon" href="//files.explosm.net/img/favicons/site/safari-pinned-tab.svg" color="#0000ff">
<link rel="shortcut icon" href="//files.explosm.net/img/favicons/site/favicon.ico">
<meta name="apple-mobile-web-app-title" content="Cyanide & Happiness">
<meta name="application-name" content="Cyanide & Happiness">
<meta name="msapplication-TileColor" content="#da532c">
<meta name="msapplication-TileImage" content="//files.explosm.net/img/favicons/site/mstile-144x144.png">
<meta name="msapplication-config" content="//files.explosm.net/img/favicons/site/browserconfig.xml">
<meta name="theme-color" content="#ffffff">
cylonid commented 3 years ago

@Stimmenhotel The reason for eBay and Facebook is mentioned in the FAQ: No high-resolution icon available. With very few exceptions, ICO files only contain small resolutions like 32x32 and that is just not enough to display a nice, large icon on a modern screen.

The first choice for an icon is querying the PWA Manifest. If there is none, then it falls back to the icons (Android Chrome PNG, Apple Touch PNG, favicon.ico) defined in the HTML headers you posted. Native Alpha tries to select the highest resolution based on the metadata provided. If the resolution is not sufficient, then an error is raised.

The problem with explosm.net is that the icon paths in the PWA manifest are wrong. They are set relative to root while they should be relative to the current folder where the PWA manifest is stored: Path from PWA manifest: https://files.explosm.net/favicons/android-chrome-192x192.png?v=2 => Fails. Correct path: https://files.explosm.net/img/favicons/site/android-chrome-192x192.png => Works.

Of course I could go on querying until a correct, resolvable path is found (in this case the icons in the HTML header would work fine) but in general it is not possible to foresee every potential mistake regarding messed up paths. At some point you have to rely on the information provided.

In any case, the option for the user to select a custom icon is already implemented, so this should be less of a problem in future. I just didn't have time yet to test everything and prepare the release.

Still, thanks for the heads up regarding FB and eBay. For "prominent" sites where the implemented icon fetcher does not work I usually add a hardcoded path to the set of icon candidates to make sure something is found.

Stimmenhotel commented 3 years ago

Ah okay thanks, the low resolution thing makes sense.  I tried to understand the code, but I am not very capable understanding Java (yet).  The hard coded parts were obvious, could have hint me into the right direction. 

~~ But the ability to add custom icons... I haven't found this in 0.84 yet.  The label of an entry stores the shown name I belive?  The dialog after submitting the wanted website won't show a field for a custom URL either, of the fetching~~

Found it in the latest commits, nevermind.

Stimmenhotel commented 3 years ago

Thanks for the new update. Few of my sites working great now.

But one is triggering a crash: URL: https://c.darfichrein.de/checkin (Corona checkin for my university) Crashlog: https://del.dog/unesteroxy

cylonid commented 3 years ago

@Stimmenhotel Thank you for the report, the crash will be fixed in the next hotfix release (likely this week). If this is a site developed by your university, you might want to inform them that the high-res icons they claim to have available do not exist (or at least the paths are not correct) :sweat_smile: By the way, please open a new issue for something like this the next time. While the error cause is related to not correctly set icons, the crash itself is caused by a problem in Native Alpha and therefore is different from the icon retrieval problems discussed in this thread.

Stimmenhotel commented 3 years ago

No it is not. But I will message them about this. Also, I thought about opening a new issue, but since this was still open and even somehow is the same error, I have chosen this one. But yeah crashing is something else than not being able to receive a favicon.

stefanmichalk commented 3 years ago

I will fix this for darfichrein.de ;-)

cylonid commented 3 years ago

@Stimmenhotel This issue should be fixed now. I didn't include a hardcoded path for darfichrein.de since a fix was already promised ;)