Make home screen bookmark icons pretty

VERIFIED FIXED in Firefox 8

Status

Fennec Graveyard
Bookmarks
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: ibarlow, Assigned: mfinkle)

Tracking

Trunk
Firefox 8
x86
Mac OS X

Details

Attachments

(4 attachments)

(Reporter)

Description

6 years ago
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
Summary: Make homepage bookmark icons pretty → Make home screen bookmark icons pretty
Created attachment 552496 [details] [diff] [review]
patch

This patch impls the logic in comment 0
Assignee: nobody → mark.finkle
Attachment #552496 - Flags: review?(wjohnston)
Created attachment 552499 [details]
example screenshot

here's what it looks like. "About:" has no favicon, so it uses the "star" overlay
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.
Attachment #552496 - Flags: review?(wjohnston) → review+
(Reporter)

Comment 4

6 years ago
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)?
(Reporter)

Comment 5

6 years ago
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.
(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 :)
http://hg.mozilla.org/mozilla-central/rev/1dd81c324ac7
Status: NEW → RESOLVED
Last Resolved: 6 years ago
status-firefox8: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 8

Comment 8

6 years ago
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.