744479 - Favicon throws an error when a web page is opened up locally on a device

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

1. go to http://paulrouget.com/mwc-demos/apis/
2. save the web page
3. push the web page to your device
4. open up the web page locally through file:///<wherever you placed the web page>

Expected: no errors
Actual:

04-11 10:47:22.456: D/GeckoFavicons(2363): Requesting cancelation of favicon load (41)
04-11 10:47:22.456: D/GeckoFavicons(2363): Creating LoadFaviconTask with URL = file:///sdcard/web/Web%20APIs.html and favicon URL = null
04-11 10:47:22.456: D/GeckoFavicons(2363): Calling loadFavicon() with URL = file:///sdcard/web/Web%20APIs.html and favicon URL = null (42)
04-11 10:47:22.456: D/GeckoFavicons(2363): Favicon URL is now: file:/favicon.ico
04-11 10:47:22.464: D/GeckoFavicons(2363): Calling getFaviconUrlForPageUrl() for file:///sdcard/web/Web%20APIs.html
04-11 10:47:22.464: D/GeckoFavicons(2363): Downloading favicon for URL = file:///sdcard/web/Web%20APIs.html with favicon URL = file:/favicon.ico
04-11 10:47:22.495: E/GeckoFavicons(2363): Error reading favicon
04-11 10:47:22.495: E/GeckoFavicons(2363): java.lang.IllegalArgumentException: Host name may not be null
04-11 10:47:22.495: E/GeckoFavicons(2363): 	at org.apache.http.HttpHost.<init>(HttpHost.java:83)
04-11 10:47:22.495: E/GeckoFavicons(2363): 	at org.apache.http.impl.client.AbstractHttpClient.determineTarget(AbstractHttpClient.java:497)
04-11 10:47:22.495: E/GeckoFavicons(2363): 	at org.apache.http.impl.client.AbstractHttpClient.execute(AbstractHttpClient.java:487)
04-11 10:47:22.495: E/GeckoFavicons(2363): 	at org.apache.http.impl.client.AbstractHttpClient.execute(AbstractHttpClient.java:465)
04-11 10:47:22.495: E/GeckoFavicons(2363): 	at org.mozilla.gecko.Favicons$LoadFaviconTask.downloadFavicon(Favicons.java:288)
04-11 10:47:22.495: E/GeckoFavicons(2363): 	at org.mozilla.gecko.Favicons$LoadFaviconTask.doInBackground(Favicons.java:350)
04-11 10:47:22.495: E/GeckoFavicons(2363): 	at org.mozilla.gecko.Favicons$LoadFaviconTask.doInBackground(Favicons.java:227)
04-11 10:47:22.495: E/GeckoFavicons(2363): 	at android.os.AsyncTask$2.call(AsyncTask.java:264)
04-11 10:47:22.495: E/GeckoFavicons(2363): 	at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:305)
04-11 10:47:22.495: E/GeckoFavicons(2363): 	at java.util.concurrent.FutureTask.run(FutureTask.java:137)
04-11 10:47:22.495: E/GeckoFavicons(2363): 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1076)
04-11 10:47:22.495: E/GeckoFavicons(2363): 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:569)
04-11 10:47:22.495: E/GeckoFavicons(2363): 	at java.lang.Thread.run(Thread.java:856)

Comment 1

[Brian Nicholson (:bnicholson)]

Attachment: Only attempt to load favicons for HTTP/HTTPS

Favicons only available for HTTP/HTTPS, so we shouldn't try to fetch them for other schemes.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

What if the non-http page has a <link rel="shorcut icon" ... />  ? Shouldn't that work?

Comment 3

[Brian Nicholson (:bnicholson)]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> What if the non-http page has a <link rel="shorcut icon" ... />  ? Shouldn't
> that work?

It will work if the non-http points to an http favicon (since the URI is from the favicon, not the page). If the file:/// page refers to a file:/// favicon, this won't work. But this currently doesn't work anyway because we're using the HttpClient for all favicons, which fails locally. Maybe we can address this in a different bug if it's important?

Also, speaking of local favicons, we should be careful that sites aren't requesting local favicons since this is a security problem. When going to http://people.mozilla.com/~bnicholson/test/favicons.html on Fennec, I see a logcat error when we try to load it, which we shouldn't be doing. Going to that site on desktop shows a Security Error in the console. This patch should address this problem since it won't try to load the file:/// URLs.

Comment 4

[Brian Nicholson (:bnicholson)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/53f971a7b77b

Comment 5

[Graeme McCutcheon [:graememcc]]

https://hg.mozilla.org/mozilla-central/rev/53f971a7b77b

(Merged by Ed Morley)