573638 - Allow page-icon: protocol to pass-through to network when an icon is not in the database

Status: NEW

(Toolkit :: Places, defect, P3)

Description

[Marco Bonardo [:mak]]

using moz-anno:icon: is nice for treeviews but often implementers need just to get icon data from the database if it's present, from network if it's not present locally. We should provide this kind of pass-through protocol.

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

I *think* the moz-anno protocol handler can just create a HTTP channel and return that. So it shouldn't be too difficult in that respect.

I'm hoping it'll be enough to just request /favicon.ico

Comment 3

[Justin Dolske [:Dolske]]

P2 for Reading List work, but worth considering if there's a simpler way to do this? EG kick off an explicit favicon fetch as items come into the RL so Places has it?

Comment 4

[Marco Bonardo [:mak]]

(In reply to Justin Dolske [:Dolske] from comment #3)
> P2 for Reading List work, but worth considering if there's a simpler way to
> do this? EG kick off an explicit favicon fetch as items come into the RL so
> Places has it?

That's possible, provided history is enabled.
Unfortunately it wouldn't work for permanent pb, cause we only store favicons for pages that are in history or bookmarks.

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Yea, I looked into alternatives recently (including adding it as a field that's synced). This still ended up looking like the easiest for now.

Further into the future, I do wonder about maybe changing how the favicon service works a bit, to allow storing favicons for arbitrary pages (or rather, pages not in history/bookmarks). And allowing an external component like the ReadingList determine when a favicon should be evicted from the DB. Far from a trivial change though, I know.

Comment 6

[Marco Bonardo [:mak]]

(In reply to Blair McBride [:Unfocused] (I don't read bugmail - needinfo? me!) from comment #5)
> Further into the future, I do wonder about maybe changing how the favicon
> service works a bit, to allow storing favicons for arbitrary pages (or
> rather, pages not in history/bookmarks). And allowing an external component
> like the ReadingList determine when a favicon should be evicted from the DB.
> Far from a trivial change though, I know.

if we do that, and I'd be fine with the decision, we first need bug 977177.
I don't want places.sqlite to become the favicons trashcan :)

Comment 7

[Justin Dolske [:Dolske]]

Moving to v2 -- we have the preview image, and that's sufficient to ship with for v1.