Last Comment Bug 677999 - Make home screen bookmark icons pretty
: Make home screen bookmark icons pretty
Status: VERIFIED FIXED
:
Product: Fennec Graveyard
Classification: Graveyard
Component: Bookmarks (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: Firefox 8
Assigned To: Mark Finkle (:mfinkle) (use needinfo?)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-10 12:19 PDT by Ian Barlow (:ibarlow)
Modified: 2011-08-24 08:31 PDT (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Assets for Home Screen Bookmark icons (17.31 KB, application/zip)
2011-08-10 12:19 PDT, Ian Barlow (:ibarlow)
no flags Details
patch (19.00 KB, patch)
2011-08-11 14:18 PDT, Mark Finkle (:mfinkle) (use needinfo?)
wjohnston2000: review+
Details | Diff | Splinter Review
example screenshot (54.00 KB, image/png)
2011-08-11 14:19 PDT, Mark Finkle (:mfinkle) (use needinfo?)
no flags Details
Inner shadow for favicons (3.01 KB, image/png)
2011-08-12 09:07 PDT, Ian Barlow (:ibarlow)
no flags Details

Description Ian Barlow (:ibarlow) 2011-08-10 12:19:51 PDT
Created attachment 552169 [details]
Assets for Home Screen Bookmark icons

Let's add a background to our homescreen bookmarks, as shown in these mocks: http://cl.ly/2M1O213w3C3g451o3k1S

1. If no favicon is available, use a star as the default icon
2. If a favicon is available, scale the icon to 32px and place in the center of the background
Comment 1 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-11 14:18:20 PDT
Created attachment 552496 [details] [diff] [review]
patch

This patch impls the logic in comment 0
Comment 2 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-11 14:19:23 PDT
Created attachment 552499 [details]
example screenshot

here's what it looks like. "About:" has no favicon, so it uses the "star" overlay
Comment 3 Wesley Johnston (:wesj) 2011-08-11 18:22:17 PDT
Comment on attachment 552496 [details] [diff] [review]
patch

Review of attachment 552496 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not a huge fan of this, but the code looks fine to me.

Are we trying to create another distinction between installed webapps and bookmarks here? If not, I think we should probably reuse/extend:

http://mxr.mozilla.org/mozilla-central/source/mobile/chrome/content/browser-ui.js#1044

and only do this if there is nothing big and shiny to use.

::: mobile/chrome/content/BookmarkHelper.js
@@ +73,5 @@
>      this.box.hidden = true;
>    },
>  
>    createShortcut: function BH_createShortcut(aTitle, aURL, aIconURL) {
> +    // The background images are 72px, but Android will resize ass needed.

I see you've added a secret spelling "error" here.
Comment 4 Ian Barlow (:ibarlow) 2011-08-12 06:15:17 PDT
Mark, these are looking great. 

One comment -- the favicons are a bit fuzzy, is this screenshot using mbrubeck's crisp scaling yet (instead of bilinear)?
Comment 5 Ian Barlow (:ibarlow) 2011-08-12 09:07:33 PDT
Created attachment 552670 [details]
Inner shadow for favicons

If it's possible, let's layer this shadow / glow image on top of our favicons, to give them some depth.
Comment 6 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-12 11:35:31 PDT
(In reply to Wesley Johnston (:wesj) from comment #3)

> I'm not a huge fan of this, but the code looks fine to me.

Not a fan of using backgrounds? Or the way we use Image to overlay layers?

> 
> Are we trying to create another distinction between installed webapps and
> bookmarks here? If not, I think we should probably reuse/extend:

We will look into reusing what we can

> >    createShortcut: function BH_createShortcut(aTitle, aURL, aIconURL) {
> > +    // The background images are 72px, but Android will resize ass needed.
> 
> I see you've added a secret spelling "error" here.

fixed :)
Comment 7 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-12 11:36:49 PDT
http://hg.mozilla.org/mozilla-central/rev/1dd81c324ac7
Comment 8 Andreea Pod 2011-08-24 08:31:15 PDT
Verified fixed on: Mozilla /5.0 (Android;Linux armv7l;rv:9.0a1) Gecko/20110824 Firefox/9.0a1 Fennec/9.0a1
Device: LG Optimus 2X (Android 2.2)

Added google.com and about: as home screen bookmarks and checked if they look like in the screenshot.

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