Closed Bug 913878 Opened 12 years ago Closed 12 years ago

SVG favicon with sizes="any" is loaded, but not rendered

Categories

(Firefox for Android Graveyard :: General, defect)

Product:
Firefox for Android Graveyard
Component:
General
Platform:
ARM
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED DUPLICATE of bug 914027

People

(Reporter: twm, Unassigned)

Details

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:23.0) Gecko/20100101 Firefox/23.0 (Beta/Release) Build ID: 20130803192641 Steps to reproduce: Authored a page with these link tags in <head>: <link rel="shortcut icon" href="/static/icon.ico"> <link rel="icon" sizes="152x152" type="image/png" href="/static/icon.touch.png"> <link rel="icon" sizes="any" type="image/svg+xml" href="/static/icon.svg"> Loaded page in Firefox for Android Aurora. Actual results: Firefox fetches the icon: The SVG icon is loaded, apparently because sizes="any" is preferred: 192.168.0.13 - - [07/Sep/2013:07:56:03 +0000] "GET /static/icon.svg HTTP/1.1" 200 2152 "-" "Mozilla/5.0 (Android; Tablet; rv:25.0a2) Gecko/25.0a2 Firefox/25.0a2" However, the generic globe icon is displayed instead. On the next reload, the .ico version of the icon is displayed (horribly pixellated because Firefox picks the 16px one for some reason). Expected results: The SVG icon should be displayed, or an icon in a format which is supported should be The patch in Bug 715263 doesn't seem to take into account the type="" link attribute when selecting icons. It should probably ignore image/svg+xml entirely as the android.graphics.BitmapFactory used to render it in the Java GUI doesn't support it, and prefer other formats to image/x-icon or image/vnd.microsoft.icon because it only decodes the 16px version of the icon from the .ico format.
OS: Linux → Android
Hardware: x86_64 → ARM
Any idea if this is a problem particular Android? Does desktop Firefox behave any differently?
Product: Core → Firefox for Android
Version: 25 Branch → unspecified
Pretty sure it's Android-specific. browser.js [1] seems to send notices to the Java UI about <link rel="icon"> tags it finds in the page, which are eventually loaded and resized via Favicons.java [2]. The issue is that browser.js doesn't consider the <link type> attribute when deciding what to send to the Java UI, and the Java UI can't decode SVG images. Not that I'm familiar with the codebase, but their placement in the source tree would seem to indicate Android. Additionally, the Java UI only seems to decode the smallest (usually, 16x16) icon when given an ICO format file, so loading that format is undesirable if there are any alternatives. [1]: https://mxr.mozilla.org/mozilla-aurora/source/mobile/android/chrome/content/browser.js#3395 [2]: https://mxr.mozilla.org/mozilla-aurora/source/mobile/android/base/Favicons.java (Bug 751712 is <link sizes> support for desktop.)
(In reply to Tom Most from comment #2) > Pretty sure it's Android-specific. browser.js [1] seems to send notices to > the Java UI about <link rel="icon"> tags it finds in the page, which are > eventually loaded and resized via Favicons.java [2]. The issue is that > browser.js doesn't consider the <link type> attribute when deciding what to > send to the Java UI, and the Java UI can't decode SVG images. Not that I'm > familiar with the codebase, but their placement in the source tree would > seem to indicate Android. The type attribute is ignored because it is redundant - the file type of the downloaded image can be determined by inspecting its magic numbers. If we rely on the type attribute, then no Favicon is displayed if you give an incorrect type attribute but a still valid image - the current way is better. However, there is no support (Unlanded or otherwise) for SVG. Not yet, anway. > Additionally, the Java UI only seems to decode the smallest (usually, 16x16) > icon when given an ICO format file, so loading that format is undesirable if > there are any alternatives. This problem at least is being fixed over in Bug 748100
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to Chris Kitching [:ckitching] from comment #3) > The type attribute is ignored because it is redundant - the file type of the > downloaded image can be determined by inspecting its magic numbers. If we > rely on the type attribute, then no Favicon is displayed if you give an > incorrect type attribute but a still valid image - the current way is better. > > However, there is no support (Unlanded or otherwise) for SVG. Not yet, anway. Do authors commonly set incorrect type attributes on link elements? As an author I would *expect* the browser to exclude formats it doesn't support based on the type attribute---what else is it for, if not that? And if there were a mismatch between the type attribute and the actual type of the resource, that would be a relatively easy-to-discover bug (e.g. an HTML linter can check for reasonable type attribute values, and a link checker can verify that they match). More importantly, the current approach is not forward-compatible. If another browser implements SVG favicon support and authors start adding <link type="image/svg+xml" sizes="any"> tags to their pages (as suggested by the HTML5 spec [1]), they will get no favicon in Fennec. [1]: http://www.whatwg.org/specs/web-apps/current-work/multipage/links.html#attr-link-sizes
type is ignored because we didn't implement it. But we can (and save you a small bit of bandwidth). I have no idea how many sites use it, but I would guess the number that include it with an incorrect type are probably small. We can also implement a Gecko fallback decoder for SVG. You should separate those into two different bugs though.
SVG decoding support probably won't see much use, as most designers seem to prefer individually-rasterized icons at each size so they have an appropriate level of detail. (*I* would use SVG, but I'm not a designer.) Bug 748100 is a much bigger improvement for much less effort (though always picking the largest icon isn't quite right either). Filtering out unsupported types is probably the best way to resolve this bug.
(In reply to Tom Most from comment #4) > Do authors commonly set incorrect type attributes on link elements? As an > author I would *expect* the browser to exclude formats it doesn't support > based on the type attribute---what else is it for, if not that? Ah yes, this would make sense - Gecko shouldn't be notifying Java for formats it doesn't have the capability to decode. > And if there were a mismatch between the type attribute and the actual type of the resource, > that would be a relatively easy-to-discover bug (e.g. an HTML linter can check for > reasonable type attribute values, and a link checker can verify that they > match). It would, but I'm not sure why we should expend more effort to make people's websites work less well. > More importantly, the current approach is not forward-compatible. If another > browser implements SVG favicon support and authors start adding <link > type="image/svg+xml" sizes="any"> tags to their pages (as suggested by the > HTML5 spec [1]), they will get no favicon in Fennec. Yes, because we're selecting just based on size - we should select based on size ignoring formats that are unsupported. Blinkly obeying the type parameter in the decoding step remains unnecessary. (In reply to Tom Most from comment #6) > Bug 748100 is a much bigger improvement for much less effort (though always > picking the largest icon isn't quite right either). Yes it is. We use Favicons all over the UI in various menus etc. We want the highest quality resource for this we have available - so we decode the largest we can get our hands on, then downscale it to 128x128 if it's larger than that (Since this is plenty large enough for current use), and then downscale again where necessary (Caching the results at each step) Why would we want to intentionally decode a smaller image, except if the larger one is larger than we need?
Strangely, the current logic in browser.js appears to notify Java individually of every Favicon as it is detected. This makes sense, since we might not download all of the appropriate link tags at once. I think the solution, then, is to implement the appropriate logic in Java to only launch a new download task if the new Favicon is more appealing than the old Favicon, and if it's of a format we can understand.
Splitting this bug as per Wes' suggestion in Comment 5 - not least because this allows me to then take half of it. Bug 914025 - SVG Favicons are not supported. Bug 914027 - The Favicon system attempts to decode Favicons in unsupported formats.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.