Last Comment Bug 361923 - Can't use remote search engine icons larger than 10KB
: Can't use remote search engine icons larger than 10KB
Status: NEW
[fxsearch]
: ux-error-recovery, ux-implementation-level
Product: Firefox
Classification: Client Software
Component: Search (show other bugs)
: Trunk
: All All
: P3 minor 35 with 2 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
: 973201 1248038 (view as bug list)
Depends on:
Blocks: fx34-searchui 1103315 1106043
  Show dependency treegraph
 
Reported: 2006-11-26 19:40 PST by Alex
Modified: 2016-08-29 02:01 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Alex 2006-11-26 19:40:50 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0

I tested this with a few OpenSearch plugins (not sure if it's like this with Sherlock). I specified an icon (and used the documentation to make sure I did everything correct) in the plugin, but Firefox still used the favicon of the site I tried to install the plugin from.

Reproducible: Always

Steps to Reproduce:
1. Go to any website with a (OpenSearch) search plugin. (Here's a site with a lot: http://www.searchplugins.net/ )
2. Install a search plugin whose icon is different from the favicon.
3. The icon appears different from what is specified.
Actual Results:  
I found the icon used by Firefox was the favicon of the site I installed the plugin from.

Expected Results:  
I expected to see the icon specified by the plugin, not the favicon.

I have no extensions that involve search plugins installed.
Comment 1 :Gavin Sharp [email: gavin@gavinsharp.com] 2006-11-28 07:59:21 PST
Alex, do you have a specific example that I can test?
Comment 2 Alex 2006-11-28 12:38:00 PST
Hmm... Now that I see it, I think if you specify it via Javascript it works; I just tried one on Mycroft and it worked, but noticed the icon is specified via Javascript.

You can try this OpenSearch plugin: http://www.searchplugins.net/pluginlist.aspx?q=mozilla+addons&mode=title (try any without the favicon- the favicon for the site is orange)
Comment 3 Alex 2006-11-28 12:45:06 PST
...Never mind, any of those did work. This one I created doesn't work, however.  If you compare the two, they're identical (code-wise). Both icons exist.

http://www.searchplugins.net/createos.aspx?number=1177&view=source
(^doesn't work)
http://www.searchplugins.net/createos.aspx?number=340&view=source
(^does work)
Comment 4 Mycroft Project (CC) 2006-11-28 18:00:58 PST
Er, this is the javascript for OpenSearch admission at Mycroft:

function addOpenSearch(name,ext,cat,pid,meth)
{
  if ((typeof window.external == "object") && ((typeof window.external.AddSearchProvider == "unknown") || (typeof window.external.AddSearchProvider == "function"))) {
    if ((typeof window.external.AddSearchProvider == "unknown") && meth == "p") {
      alert("This plugin uses POST which is not currently supported by Internet Explorer's implementation of OpenSearch.");
    } else {
      window.external.AddSearchProvider(
        "http://mycroft.mozdev.org/installos.php/" + pid + "/" + name + ".xml");
    }
  } else {
    alert("You will need a browser which supports OpenSearch to install this plugin.");
  }
}

The icon is specified within the OpenSearch description.
(Sherlock icons are specified via Javascript)
Comment 5 Alex 2006-11-28 18:11:01 PST
Whoops, I meant to take a look into the Javascript at Mycroft, but apparently, forgot.

Anyways, does anyone else see this problem?
Comment 6 Mycroft Project (CC) 2006-11-28 18:15:52 PST
I see the 'wrong' icon for #1177

It's returning:
iconLoadCallback: load failed, or the icon was too large!
http://search.live.com/favicon.ico is a rather fat 29.22 kB

Seems that:
const MAX_ICON_SIZE   = 10000;
http://lxr.mozilla.org/mozilla/source/browser/components/search/nsSearchService.js#90
http://lxr.mozilla.org/mozilla/source/browser/components/search/nsSearchService.js#1327

It seems that it defaults back to the loading site's favicon but I didn't trace that far...
Comment 7 :Gavin Sharp [email: gavin@gavinsharp.com] 2006-11-28 18:51:09 PST
Ah, yes. It will reject large icons, because generating a data: URI for them could take a long time (and potentially hang the UI). The limit I chose is rather arbitrary - of the large (1000+) set of search engines I had, only 6 or so had icons greater than 10kb. We should probably consider increasing it if it proves to be a problem, and we should definitely make it clearer to plugin developers what's happening without making them resort to flipping browser.search.update.log (I'm thinking a message in the Error console would be appropriate).
Comment 8 Mycroft Project (CC) 2006-11-28 19:49:25 PST
Well given we're talking about 16x16 icons they really shouldn't need to be that big - in fact I don't really see how they can be much bigger than 16*16*4 = 1k (for 32 bit) but I don't know much about images...

As far as I'm concerned from a Mycroft point of view, submitted icons are limited to 5k anyway (well now that I've corrected an omission in the OpenSearch submission form anyway :) ) so a 10k limit is irrelevant.

I don't know you normally do for that sort of error message - browser.search.log works well enough for me.
Comment 9 Jorrit Schippers 2007-02-18 04:47:49 PST
It's not 10 KB, it's 10000 bytes. For instance, http://www.vbulletin.com/forum/favicon.ico does not work, while it's 9.9 KB (it's a bad icon anyway, but I thought it would be useful to mention what the exact maximum is)
Comment 10 Jezz 2011-08-05 07:20:38 PDT
Is there any chance this 10000-byte limit could be increased? Even just to 20000 bytes?

I'd like to use http://www.amazon.com/favicon.ico with an OpenSearch plugin I'm creating for myself, however it won't work because it's 17542 bytes in size. The reason it's so "big" is because this .ico file contains several different sizes of the icon, including 48x48, 32x32, 24x24 and 16x16 - each of them being 32-bit.

I dunno why Amazon has done that, but nevertheless that's how it is, and I'd love to be able to use their file as it is. I can't imagine there would be a huge burden on Firefox to increase this limit, especially as there are only a small handful of sites that do have such large favicon files (as someone wrote above, most sites use just a single 16x16x4 icon file, which is very small).
Comment 11 Mycroft Project (CC) 2013-12-11 09:52:34 PST
A few examples of big favicon.ico:
(NB  View Image Info reports the 16x16 page size not the actual file size)
http://www.biblegateway.com/favicon.ico (13 046 bytes)
http://rae.es/favicon.ico (10 862 bytes)
http://nashape.com/favicon.ico (32 988 bytes)

The above were memorable (and slightly weird) recent examples /
(I went looking for one with a 64x64 page for the last.)

But Google / Amazon are getting bigger...
Google's favicon now includes 16 and 32 square icons
and as Jezz says Amazon's includes 24 and 48 square icons as well.

My approach in the past has been just to chop the 16x16 out but as mobile now wants (if not fully supports as far as I can tell) 32x32 I'm less inclined to do this. And I've seen sites with multiple bit depth icons so that the file size is greater than 10 000 bytes even with just 16 and 32 square icons.

Not sure what the best maximum would be though...
Comment 12 Mycroft Project (CC) 2013-12-11 12:24:29 PST
Because I found it while looking for something else and mostly as a reminder to myself in the future:
https://github.com/audreyr/favicon-cheat-sheet
Comment 13 Alice0775 White 2014-02-15 10:38:22 PST
*** Bug 973201 has been marked as a duplicate of this bug. ***
Comment 14 Alice0775 White 2016-02-12 13:53:52 PST
*** Bug 1248038 has been marked as a duplicate of this bug. ***
Comment 15 alex_mayorga 2016-02-14 04:00:58 PST
¡Hola Marco!

Coming from https://bugzilla.mozilla.org/show_bug.cgi?id=1248038 Thanks Alice!

What's the "right way" to fix this bug?

Do outreach to the sites when this is found?

A few sites where I've found this bug FWIW:

https://es.wiktionary.org/wiki/
https://fr.wiktionary.org/wiki/
http://www.wolframalpha.com/

¡Gracias!
Alex
Comment 16 Florian Quèze [:florian] [:flo] 2016-02-15 03:29:49 PST
The possible ways forward:
- do outreach to the sites (but this won't ever fix every possible site with the issue...)
- slightly increase the size limit and add an explicit error message in the error console when an image is above the limit (as suggested in comment 7).
- resize the image before saving it. I'm increasingly thinking this is what we should do. See bug 389273 for how this was implemented for favicons.
Comment 17 Marco Bonardo [::mak] 2016-02-15 08:46:52 PST
(In reply to Florian Quèze [:florian] [:flo] from comment #16)
> - slightly increase the size limit and add an explicit error message in the
> error console when an image is above the limit (as suggested in comment 7).

A bump would make sense considering how common is hi-dpi today.

> - resize the image before saving it. I'm increasingly thinking this is what
> we should do. See bug 389273 for how this was implemented for favicons.

Favicons decision has a few downsides to consider:
1. the chosen limit was too strict, it doesn't play well with hi-dpi. But that had a reason, since all icons are stored in the same file as visits, that is also the same file powering the awesomebar. In the search egines case the limit can be larger, since it's unlikely the user has thousands of engines, but still sounds like everything ends up in the search.json file, so the limit should not be large enough to cause performance problems when parsing a file with (let's guess) 100 engines. Requires some measurement of the impact on low-end hardware.
2. Resizing clearly doesn't play well with some formats, for example animated images will become still, multi-image formats (ico) may become single image, and so on.

What I'd suggest is half-way between the 2 approaches:
1. slightly enlarge the current limit
2. resize anything above the limit
3. if the resize has losses (from animated/multi to single image) notify the problem in the error console so it can get noticed by web devs.
4. Add telemetry for how often we have to resize an icon, how much we take to go from plain icon to the final encoded data uris (included resize) and, if possible, how much time we take to encode/decode the search.json file (but maybe we already have this measurement?).
Comment 18 Florian Quèze [:florian] [:flo] 2016-02-15 09:08:56 PST
(In reply to Marco Bonardo [::mak] from comment #17)

> What I'd suggest is half-way between the 2 approaches:
> 1. slightly enlarge the current limit
> 2. resize anything above the limit
> 3. if the resize has losses (from animated/multi to single image) notify the
> problem in the error console so it can get noticed by web devs.

Seems good to me.

> 4. Add telemetry for how often we have to resize an icon, how much we take
> to go from plain icon to the final encoded data uris (included resize) and,
> if possible, how much time we take to encode/decode the search.json file
> (but maybe we already have this measurement?).

Isn't this a bit excessive for a rare edge case? That case is about someone installing a custom search engine (already not very common) with a large icon (uncommon, as this currently causes the icon to be rejected), on low-end hardware (probably not very common for users who like to customize stuff).
Comment 19 Marco Bonardo [::mak] 2016-02-15 09:33:21 PST
(In reply to Florian Quèze [:florian] [:flo] from comment #18)
> Isn't this a bit excessive for a rare edge case?

How we can tell it's rare without data? If we already have such data then I agree it's not worth adding more probes.
Though, the fact this bug exists with various examples, means sites in the wild are doing that. Knowing how often it happens could be useful to tweak the size limit. The search.json measurement is something I expect we should already have, if it impacts startup in any way. The encode time could be overzealous, I agree.
Adding a probe is extremely cheap and it has an expire date, so we can check if the initially selected limit works fine in the wild, and then just remove the probe once expired.
Comment 20 Florian Quèze [:florian] [:flo] 2016-02-15 10:37:16 PST
(In reply to Marco Bonardo [::mak] from comment #19)
> (In reply to Florian Quèze [:florian] [:flo] from comment #18)
> > Isn't this a bit excessive for a rare edge case?
> 
> How we can tell it's rare without data? If we already have such data then I
> agree it's not worth adding more probes.

I have data about default engines that are installed from open search (and I know it's rare). I don't have data about non-default engines installed from open search though, and I guess it would be interesting to collect to better understand how many engines people install.

> Though, the fact this bug exists with various examples, means sites in the
> wild are doing that.

We only have a handful of examples, for a bug that's been around for almost a decade; that makes me think it's rare.

> The search.json measurement is something I expect we should
> already have, if it impacts startup in any way.

We have probes recording the time it takes to initialize the search service (which happens asynchronously), not for the time it takes to parse the json file.
Comment 21 julien.darthenay 2016-02-26 11:53:06 PST
Hello.

Does the Wiktionnaries icons record as "rare"?
Comment 22 julien.darthenay 2016-02-27 12:09:21 PST
If I understand clearly the problem, you are intentionality putting a limit to the icon file to void the search.json file to turn too heavy. Actually I agree a lot with that. I would even say the 10k limit  could even be seen as too large. But, why to keep the whole icon in the search.json? Wouldn't it be better to keep only the 16x16 resolution icons and throw away the rest of the icon file? It is that much difficult to keep only the interesting icons in the file of each search engine?

Here are some a few other too heavy icons (most of them were not a few years ago):
- Wiktionary
- wordreference.com
- start.me
Comment 23 Thomas Bertels 2016-08-29 02:01:43 PDT
Only a few examples exist of search engines favicons larger than 10 000 bytes and all of them seem to be below 20 000 bytes. So it may be a good idea to fix this bug by increasing the limit to 20 000 bytes instead of a lengthier and larger approach.

Optionally, a probe could be added to check how often that new limit is large enough for search engines.

Note You need to log in before you can comment on or make changes to this bug.