Closed Bug 961893 Opened 11 years ago Closed 9 years ago

Use #-moz-resolution URL fragment to get higher resolution favicons

Categories

(Firefox OS Graveyard :: Gaia::Homescreen, defect, P2)

defect

Tracking

(b2g-v2.0 affected, b2g-v2.1 affected, b2g-v2.2 affected)

RESOLVED WONTFIX
Tracking Status
b2g-v2.0 --- affected
b2g-v2.1 --- affected
b2g-v2.2 --- affected

People

(Reporter: sync-1, Unassigned, NeedInfo)

References

Details

(Keywords: polish, Whiteboard: [systemsfe], [priority], [2.0-flame-test-run-2], ux-tracking)

Attachments

(3 files)

Mozilla build ID: 20140105004001 DEFECT DESCRIPTION: [Browser]the Desktop Bookmarks is lack of resolution REPRODUCING PROCEDURES: 1. Open "www.baidu.com" 2. press Star icon to add shortcut to home 3.go back to the desktop to look at the bookmark EXPECTED BEHAVIOUR: resolution of the Desktop Bookmarks is OK ASSOCIATE SPECIFICATION: TEST PLAN REFERENCE: TOOLS AND PLATFORMS USED: USER IMPACT: 张春梅 chunmei zhang 固话:021-51790200 (7035) 手机:13641988648(上班时间内关机) Mail:chunmei.zhang.com Skpye:Chunmei.zhang5 REPRODUCING RATE: For FT PR, Please list reference mobile's behavior:
Can you include a screenshot? Also, does this reproduce on a 1.1 Buri build?
Component: Gaia::Browser → Gaia::Homescreen
needinfo for comment 1
Flags: needinfo?(sync-1)
Attached image 2014-01-27-10-59-57.png (deleted) —
Screenshot for the blur Baidu icon, it is the one above some chinese characters. Also can reproduce this issue on Buri, however i think this should be Baidu issue since we probably get icon fro Baidu
Flags: needinfo?(sync-1)
Created an attachment (id=630528) baidu favicon
Created an attachment (id=630528) baidu favicon
Attached image baidu favicon (deleted) —
Created an attachment (id=630528) baidu favicon
If there is any issue on homescreen (code that draws icon-able bookmarks like that) is here https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/page.js#L290
(In reply to comment #6) > Comment from Mozilla:If there is any issue on homescreen (code that draws > icon-able bookmarks like that) is here > > https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/page.js#L290 > It's OK when setting "ctx.mozImageSmoothingEnabled = true" in L309
It was done in bug 814460 Sam, I would like to know what implications could appear if we add the code described in comment 8. Thanks a lot Sam
Flags: needinfo?(sjochimek)
Enabled mozImageSmoothingEnabled will pixelated the 16x16px given by http://www.baidu.com/favicon.ico. The homescreen has been optimized to use 32x32px favicon for bookmark icon (for a scale ratio of 1) So two solutions here: 1. baidu should serve a 32x32px instead of a 16x16px in order to have a good looking bookmark (curl -o icon.png http://www.baidu.com/favicon.ico && file icon.png) 2. Or we can add more logic here depending on the icon size we got and try to have the best result by enabling or not image smoothing.
Flags: needinfo?(sjochimek)
(In reply to comment #9) > Comment from Mozilla:Enabled mozImageSmoothingEnabled will pixelated the > 16x16px given by http://www.baidu.com/favicon.ico. > The homescreen has been optimized to use 32x32px favicon for bookmark icon (for > a scale ratio of 1) > > So two solutions here: > 1. baidu should serve a 32x32px instead of a 16x16px in order to have a good > looking bookmark (curl -o icon.png http://www.baidu.com/favicon.ico && file > icon.png) > > 2. Or we can add more logic here depending on the icon size we got and try to > have the best result by enabling or not image smoothing. > The resolution of the icon given by http://www.baidu.com/favicon.ico is 32px32px.
(In reply to comment #10) > (In reply to comment #9) > > Comment from Mozilla:Enabled mozImageSmoothingEnabled will pixelated the > > 16x16px given by http://www.baidu.com/favicon.ico. > > The homescreen has been optimized to use 32x32px favicon for bookmark icon (for > > a scale ratio of 1) > > > > So two solutions here: > > 1. baidu should serve a 32x32px instead of a 16x16px in order to have a good > > looking bookmark (curl -o icon.png http://www.baidu.com/favicon.ico && file > > icon.png) > > > > 2. Or we can add more logic here depending on the icon size we got and try to > > have the best result by enabling or not image smoothing. > > > > The resolution of the icon given by http://www.baidu.com/favicon.ico is > 32px32px. > Sorry, I found The resolution of the icon given by http://www.baidu.com/favicon.ico is 16x16px on FFOS and is 32x32px on PC browser.
blocking-b2g: --- → 1.3?
Not blocking - this not a regression & is a non-critical issue.
blocking-b2g: 1.3? → backlog
Whiteboard: [systemsfe]
(In reply to comment #12) > Comment from Mozilla:Not blocking - this not a regression & is a non-critical > issue. > Dears, Please give me a patch as Sam's advice. It can improve experience of our product, because the icon of homescreen is so important.
Whiteboard: [systemsfe] → [systemsfe][priority]
Whiteboard: [systemsfe][priority] → [systemsfe], [priority]
I'll look into this. We had a similar problem dealing with small icons in Metro and used a icon-in-a-box treatment when necessary. I'm not sure if that's wanted or appropriate here but I'll review a few options.
Assignee: nobody → sfoster
Initial observations - the favicon.ico has a 16x16 and 32x32 image in there. Maybe a follow-up bug could implement a better UX for 16px icons but as this is a blocker ISTM this bug is about ensuring we use the 32px image when available.
Looks like we should be able to use img.src = iconPath+'#-moz-resolution=32,32' here. As I understand it, when gecko loads a .ico file it just returns the first entry in there - which is typically the smallest - e.g. 16x16. The -moz-resolution suffix allows you to indicate a prefered size when a .ico file contains more than one image. If that size is not available, it should give you the closest (next smallest IIRC). If that image is e.g. a .png not .ico bundle, no harm is done. This works for blobs too, so you can do window.URL.createObjectURL(blob) + '#-moz-resolution=' + size + ',' + size; See this related desktop fx bug 828508. I'll work on a patch to test this out.
Tim, Ben, I gather you have looked into this issue. Can you comment on my proposed solution and if this covers the issue, and maybe any other STR or regression risks I should be aware of? To clarify, this is not baidu.com specific - it can apply to just about any site.
Flags: needinfo?(timdream)
Flags: needinfo?(bfrancis)
It's good to know we have bug 828508. I am not sure if we need to patch anywhere more than home screen.
Flags: needinfo?(timdream)
Hi Sam, This is the thinking behind bug 963047 where we can do more to use higher quality icons based on metadata provided in a web page. The implementation of the W3C Manifest spec will also provide another way for higher quality web app icons to be provided. It seems that in the case of .ico files (although in my experience many .ico files are just a PNG or JPEG with an .ico file extension), there is another way that we can choose from multiple provided icons. I wasn't aware of the #-moz-resolution=32,32 syntax and if that works from content (as opposed to chrome) and provides us with another way to get higher resolution icons then that sounds great! We just need to figure out how that fits into the broader algorithm for selecting an icon, but I think the W3C spec provides some guidance on that.
Flags: needinfo?(bfrancis)
Quick demo of the resolution fragment working in content: https://dl.dropboxusercontent.com/u/1719101/share/favicon-sizes.html .. the favicon referenced has a 16px and 32px image in it. It may be worth examining the performance of using the resolution fragment when for example we know the image isn't a .ico, or we know the image is actually a png or contains only a single image with known size. Its presumably not free, but may be negligible.
This is blocked by bug 963047 which should improve the way we store and retrieve the best-size icon for a given page. The solution should be some combination of taking advantage of multiple rel=icon links, #-moz-resolution fragments and size-aware icon storage. If we want a quick fix we could put one together but I would rather fix it properly in 963047.
Depends on: 963047
Target Milestone: --- → 1.4 S5 (11apr)
Hey Ben, Needinfo'ing you per sprint planning and Sfoster's comments
Flags: needinfo?(bfrancis)
Target Milestone: 1.4 S5 (11apr) → 1.4 S6 (25apr)
I'd suggest this is not a blocker for 1.4. Gecko loads the smaller of the 2 images provided by the favicon.ico, so the boomark icon ends up being scaled and not looking so hot. It is functional however. We are investigating a solution in bug 962630 and bug 963047. As I said there may be a quick bandaid fix here if we still think its a priority.
Discussed in IRC, ben concurs and suggests we target 2.0 or 2.1 for this when we migrate all bookmarks to the homescreen.
Flags: needinfo?(bfrancis)
Target Milestone: 1.4 S6 (25apr) → 2.0 S1 (9may)
Target Milestone: 2.0 S1 (9may) → 2.0 S2 (23may)
I'm not currently working on this
Assignee: sfoster → nobody
Target Milestone: 2.0 S2 (23may) → ---
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Whiteboard: [systemsfe], [priority] → [systemsfe], [priority], [2.0-flame-test-run-2]
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
Does this issue still exist?
Keywords: qawanted
(In reply to Michael Henretty [:mhenretty] from comment #27) > Does this issue still exist? This issue is reproducible on Flame 2.2 master, Flame 2.1, and Flame 2.0. I also did a little experiment on favicons from different websites. Refer to my screenshot bookmarking baidu.com, facebook.com, abc.com, cnn.com, goolge.com, and wikipedia.org. Out of all, only abc and wikipedia were bookmarked with high quality favicons on Flame 2.2 and 2.1; on Flame 2.0, all websites were bookmarked with low resolution favicons. Device: Flame (shallow flash) BuildID: 20141006064312 Gaia: 470826d13ae130a5c3d572d1029e595105485fb0 Gecko: e0d9ad4be606 Version: 35.0a1 (2.2 Master) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0 Device: Flame (shallow flash) BuildID: 20141006062615 Gaia: 778ebac47554e1c4b7e9a952d73e850f58123914 Gecko: bc87917b3b95 Version: 34.0a2 (2.1) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Device: Flame (shallow flash) BuildID: 20141006044157 Gaia: 26670a3193b57ead978ef2faefc2a679ea57f8d4 Gecko: c06b369ccda7 Version: 32.0 (2.0) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 Note: I didn't check Flame v180 base-only since comment 26.5 indicated that this had been occurring on earlier 2.0 builds.
QA Whiteboard: [QAnalyst-Triage+][lead-review+] → [QAnalyst-Triage?][lead-review+]
Flags: needinfo?(jmitchell)
Keywords: qawanted
QA Whiteboard: [QAnalyst-Triage?][lead-review+] → [QAnalyst-Triage+][lead-review+]
Flags: needinfo?(jmitchell)
It can reproduce on v2.0.
[Blocking Requested - why for this release]:it's bad UE, block internal QA
blocking-b2g: backlog → 2.0?
Flags: needinfo?(wehuang)
Hi Sam, understood the current timing of 2.0 but I saw you've tried to work on it earlier this year for 2.0 per comment#26. Would you help us learn the latest status? Do you plan to make it work in 2.0, or for later releases?
Flags: needinfo?(wehuang) → needinfo?(sfoster)
Forwarding status request to Dale who was working on bookmarks recently. Dale, apparently the low-res favicon-derived bookmark icons issue still exists in all branches. Should it be fixed or is there a plan to fix?
Flags: needinfo?(sfoster) → needinfo?(dale)
We implemented apple-touch and html5 link rel icon support in 2.2, we didnt touch the code in which the favicon that packages multiple images it seems like if we just chuck #-moz-resolution=64,64 (or what resolution we want) at the end if any .ico icon we try to load, would that solve this problem? it looks from your example as long as it isnt a blob url it will pick the highest res and not attempt to scale it up, so seems good to me
Flags: needinfo?(dale)
[Triage] Some improvement already done in 2.2 but not yet cover this case per comment#28 and comment#33. Nom. to 2.2 considering current 2.0 phase.
blocking-b2g: 2.0? → 2.2?
(In reply to Dale Harvey (:daleharvey) from comment #33) > We implemented apple-touch and html5 link rel icon support in 2.2, we didnt > touch the code in which the favicon that packages multiple images > > it seems like if we just chuck #-moz-resolution=64,64 (or what resolution we > want) at the end if any .ico icon we try to load, would that solve this > problem? it looks from your example as long as it isnt a blob url it will > pick the highest res and not attempt to scale it up, so seems good to me By this method, when using xhr to request the icon, it can't get the highest icon. But It can get the highest icon when using Image object.
Whiteboard: [systemsfe], [priority], [2.0-flame-test-run-2] → [systemsfe], [priority], [2.0-flame-test-run-2], 2x-uxnom
Not blocking but lets put it on our polish list.
blocking-b2g: 2.2? → ---
Keywords: polish
Assignee: nobody → bfrancis
Summary: [Sora][Browser]the Desktop Bookmarks is lack of resolution → Use #-moz-resolution URL fragment to get higher resolution favicons
Unassigning myself as after investigating this I don't think this is the best way to solve this problem. I think we need to start by saving multiple icon sizes for a bookmark and picking the most appropriate icon at run time, a bit like this https://github.com/benfrancis/gaia/commit/4bafd014fbf05aa8b446e524ce9dca3cc657c597 We can still try to use the URL fragment, but I think we need to do the above first.
Assignee: bfrancis → nobody
I'm pretty sure that bug 1201796 solves this problem in a better way. We should probably resolve this bug, but I'd like to be sure I'm not missing something. Ben, can you confirm that there's nothing further to do here once DDD is supported for ICOs? (Feel free to reach out to me on IRC; this may be something that's easier to discuss there.)
Flags: needinfo?(bfrancis)
Ben - can we close this bug?
Whiteboard: [systemsfe], [priority], [2.0-flame-test-run-2], 2x-uxnom → [systemsfe], [priority], [2.0-flame-test-run-2], ux-tracking
Mass update: Resolve wontfix all issues with legacy homescreens. As of 2.6 we have a new homescreen and having these issues open is confusing. All issues will block bug 1231115 so we can use that to re-visit any of these if needed.
Blocks: 1231115
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: