Closed Bug 117895 Opened 23 years ago Closed 13 years ago

Favicons should use a separate cache when in bookmarks

Categories

(SeaMonkey :: UI Design, defect)

Product:
SeaMonkey
Component:
UI Design
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(Not tracked)

Status:
RESOLVED WORKSFORME

People

(Reporter: jesse.houwing, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Cheezy patch to store icons in bookmarks.html
1.62 KB, patch
Details | Diff | Splinter Review 
My bookmarks are like a christmastree right now, with icons appearing and
disappearinbg everywhere. This happens quite often, as I browse a lot and icons
get kicked from the cache regularly.

I'd like the icons to stay. So would it be possible to put them in say a
seperate cache or make them non expirering, even if they should be kicked as normal?

They are like that in IE, and it seems logical.
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 2000 → All
Hardware: PC → All
Target Milestone: --- → Future

*** This bug has been marked as a duplicate of 113430 ***
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → DUPLICATE
Blocks: 120352
This bug is not really a dup of 113340 since 113340 talks about problems with
the way favicons currently cached and this bug proposes a potential solution.

I am setting severity to major since there is a huge number of favicon bugs that
would be fixed if this is implemented.
Blocks: 113340
Severity: enhancement → major
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
*** Bug 116530 has been marked as a duplicate of this bug. ***
The bug 116530 (dup of this one) proposed using bookmarks.html as a storage area
for favicons.


Hyatt, I'm changing the component and giving this to you because it doesn't
really involve a problem with the cache.
Assignee: gordon → hyatt
Status: REOPENED → NEW
Component: Networking: Cache → XP Apps: GUI Features
Blocks: 113430
No longer blocks: 113340
Old Summary: Favicons shoudl use a seperate cache when in bookmarks
New Summary: Favicons should use a separate cache when in bookmarks
Summary: Favicons shoudl use a seperate cache when in bookmarks → Favicons should use a separate cache when in bookmarks
No longer blocks: 120352
Status: NEW → ASSIGNED
Blocks: 120352
Why not storing the icons INTO the "bookmarks.html" file ? Someone will say that
it will be costly (big size increase (disk and memory)), but I don't think so.
An icon is only a 16x16 bitmap (quite small) with a maximum of 256 colors (256
pixels, so never more than 256 colors). It can be easily compressed with
efficient algorithm such as PNG, and then encoded into a property of a bookmark
entry.

It has many good points:
- The icons can't be lost (they are no more in the common cache).
- Easy to implement. Nothing complex to program. One bookmark entry = one icon.
- When you copy the "bookmarks.html" file, the icons are also "copied" (because
they are included).
- The file can be edited "by hand" with a text editor. Later, icons management
functions will be also easy (user custom icons).
- Etc...

Thanks.
I REALLY like idea of storing them in bookmarks.html, it would be really nice to
be able to create icons for websites that don't have them, or have an icon that
looks too much like another icon.  I heard someone suggest a really neat use for
them, to stick a whole bunch of bookmarks on the personal toolbar with no
associated text, to have one click access to your favorite sites (hey, can I
patent that Mr. Bezos?)  But if the icons disappear randomly like they do now,
it isn't worth the trouble.  If they were stored in bookmarks.html it'd be easy
to transfer this setup to other systems, and they'd automatically be transported
as part of roaming profiles!
WDDX is very suitable for this. See www.openwddx.org for more info.
First, using bookmarks.html as a storage was already proposed - see comment #4
(and also bug 116530 - a dup of this one).

I agree that this might be a good idea. As for "this might make bookmarks.html
too large" - let's do an experiment. Currently I have bookmarks.html of size
81,907 with 374 bookmarks in it.. 64 of those have icons defined, 52 of those
icons are distinct. Those icons occupy 92,253 bytes - so my bookmarks file will
probably be doubled or trippled if we include icons, but it's not an order of
magnitude and IMHO it's worth it.

> I REALLY like idea of storing them in bookmarks.html, it would be really nice to
> be able to create icons for websites that don't have them, or have an icon that
> looks too much like another icon

This would need to be supported separately - currently every time you visit a
site, the previously listed icon is overwritten with whatever the site specifies
(or removed if it does not specify anything). You'll need to file a separate RFE
for being able to override that. OTOH, I am sure that such override mechanism
would *not* suffer from the current disappearing problem since that is cased by
caching and override icons are probably going to be local and would not need to
be cached...

Bug 116530 opened 2001-12-22 04:55
This bug opened 2002-01-03 03:07
How can 116530 be a dup of this one? It should be the other way around.
*** Bug 116530 has been marked as a duplicate of this bug. ***
Hakon, bug 116530 is a dup of this one and not the other way around because this
bug is *more general* - it asks for *some* separate cache, while bug 116530
proposes to use bookmarks.html as such cache. It makes more sense to keep this
one open because we might decide to use something else, not bookmarks.html for
storing icons.
Fair enough ;-)
qa -> paw
QA Contact: tever → paw
--> QA -  over to you Claudius
QA Contact: paw → claudius
I don't think the icons should be added to the bookmarks file.
This would make it even more difficult to sync this file between different
computers, as I have to do between my laptop and my desktop PC.
How would that make it harder? You can still copy the bookmarks file from one
computer to the other and all the bookmarks will be copied, including the
favicons, which in my opinion makes it even better :).


It would really make it harder to *sync* bookmarks.
I add bookmarks on my laptop and on my home computer and then have to
integrate all the changes. So I have to view all the differenes between the two
bookmarks files.
And if favicons are integrated in this file there are even more differences.

And favicons simply don't belong there, they are part of the site and not of the
bookmark.
But many people want the favicons to stay with their bookmarks. That makes it
(the bookmarks file) quite a logical place to put them.

Another option is to put them in a seperate cache folder to save the favicons.

Maybe it would be a better to ask for a bookmark import/export function so that
you can import/export your bookmarks without hand editting (but that would be
another bug).
<A HREF="c21">comment 21:</A>
Images are binary data and do not belong in a text file.
With your argumentation one could simply add the whole
cache to the bookmarks file.

Import/Export is no alternative since this is not scriptable.
I sync my two computers with a lot of development and mail files
in some minutes and don't want to increase this time by a big
factor only for mozilla.
These are small, but you could always avoid that small penalty by disabling
favicons.  IMHO, if they aren't persistent across crashes and other cache
clearing activities, they are next to useless anyway.  Since they are shown in
bookmarks, that's the logical place to keep them, so that they are maintained
when bookmarks files are shared, copied, used as part of a remote profile, etc.

Perhaps this should be treated as an enhancement request to add an option to
store favicons in the bookmarks file, and always look for them there instead of
in cache when displaying bookmarks.  Whatever favicon is defined for a site
would be stored into the bookmarks file when the bookmark is added, and stay
there no matter whether the site changes or removes theirs.  If the site updated
theirs and the difference between your bookmarks and the URL bar bugged you, you
could always add it as a new bookmark and delete the old one as the easy
solution to update the icon if you care (or add a checkbox to the bookmarks
properties page for 'always update the icon' if enough people care)  This would
also allow end users to define their own icon via hand editing of the bookmarks
file to add the icon definition.  Maybe someday someone would add something to
the bookmarks manager to provide a way to do this in a non-power-user way, but I
realize that probably won't be desired by enough people to rate very high on the
enhancement radar for quite some time, unless someone wants the idea enough that
they just decide to do it themselves.
It's not clear whether this is necessarily the best solution. One alternative,
for example, is just to always request icons that have expired from cache (see
bug 116832 attachment 76974 [details] [diff] [review] for a patch that does this). I've been running with
such setup for a while and it works pretty good.
But why re-request them if you can just store them. This re-requesting seems not
neccesairy, and in my opinion is just as bad as the IE way :)
Re: comment #24:

The problem is if you have a crash or clear the cache, then display your
bookmarks you could potentially require dozens of network connections to grab
the icons.  Some sites may be slow or inaccessible.  Even if all the sites were
on the local network it would slow down display of the bookmarks unacceptably. 
There is already a delay when doing so (I have about 30 bookmarks plus a half
dozen folders with another couple dozen inside) which is just slow enough to be
noticeable and annoying.
> it would slow down display of the bookmarks unacceptably.

Have you actually tried using my patch? If you did, you would know that it does
not delay the display of bookmarks at all! You always see the bookmarks
immediatelly (as if w/o the patch), only sometimes the icons are missing
(usually only for a fraction of a second) - and as soon as they are downloaded,
they are displayed.
I tried it yesterday, and made my bookmarks ligth up like a christmastree from a
cocacola commercial, I want them to stay permanently, even if I'm offline. 

At work we also have roaming profiles, so cache is always empty after you've
logged it, which will show me this christmas tree every time I start up.

Another consideration is that it will take up extra network traffic, won't work
for people who start "offline", compose emails etc, and then go online. Until
that moment many bookmarks won't show icons.

I'm still in favour of a sepearte cache, be it in a cache directory, a bookmarks
file, or any other store (rdf?).
BTW bookmark's file is still HTML file, viewable via browser (many people use it
as My links page on their homepages). I'm not sure, if binary content inside
this file is the best solution. 
It could be done with a CDDATA tag, if the file is XHTML compliant, it could be
within a hidden div or span which should just make it parsable by a xhtml
compliant browser.

But I agree that a seperate cache, or a subfolder within the current cache, or a
seperate rdf file would be better solutions, from a HTML point of view.
Blocks: 114824
Using data: protocol (RFC 2397) to prepend or to add and extra attribute to the
bookmark link in bookmark.html would be simple and not all that costly. 
Favicons would also be visible when viewing bookmark.html as a web page.

Attachment 6634 [details] [diff] in bug 121793 shows just how easily this approach can be
implemented. )-: Please read that entire bug before posting concerns here that
have already been address there. :-( You could use this same approach to embed a
sound or whatever into each bookmark, but that would be silly ... or would it? 

However, with all that said, I still am a bit leery about this approach.  I
suspect there is a major usability concern that I'm overlooking.  I'll get back
when I put my finger on it.
*** Bug 201572 has been marked as a duplicate of this bug. ***
I'm trying to work on a phoenix extension that will show the "fav icon" or "site
icon" for a site in the statusbar when you hover over a link.  There is a
mozilla bug http://bugzilla.mozilla.org/show_bug.cgi?id=135921 for the same
functionality.    Here is the problem.  If there is no standard way for me to
retrieve an icon for a site from the cache.  Here is an example.  Mozilla.org
uses the link icon statement to define a site icon. <link rel="icon"
href="http://mozilla.org/images/mozilla-16.png" type="image/png">.  There is no
way for me to figure out if they have a site image unless I load the page and
look for this statement.  If site and fav icons were all cached with the same
type of key.  It would be simple to pull this out.
i vote for keeping the favicons in a separate location from bookmarks.html. it
CAN indeed be put there, but it doesn't belong. how about favicons.xml (or rdf,
or whatever)? in such a separate xml file, the favicons can be stored as cdata
objects, and the domains will be listed as regular text inside an xml file. this
way, one could copy only bookmarks.html, or copy also favicons.rdf and
everyone's happy.

alternatively, they can also be dtored in a sepatate directory.
*** Bug 208033 has been marked as a duplicate of this bug. ***
some favicons even stay on mine such as yahoo and mozillazine for up to weeks
before leaving, so if this can be fixed for 0.7 or maybe sooner that'd be great.
re: comment #31
If more than one bookmark uses a given icon, wouldn't you then need to store
that icon for each bookmark in order for it to show up next to each link when
viewing bookmarks.html as a web page? (i hope that made some sense)
i don't care how the favicons seperate themselves from the cache, but it sure
would be nice if they would hurry up!  lol.  my favorites toolbar has icons
*only* in it, and when the favicons disappear, it's extremely annoying.
Hmm this is the first "bug" I've voted for -- I hope I'm doing this right. Do I
click on the "vote for this bug" link or the button marked "commit"? I'm going
with the former. Bombs away!

That didn't seem to work. Let's try "Commit." Excelsior!
Why is this marked Major and not Enhancement?
Blocks: majorbugs
I agree that I'd rather not have the icons directly in bookmarks.html, but I
definitely want them preserved somewhere other than the volatile cache.  The
super power of these things, IMO, is the ability visually scan through a large
list of bookmarks and find what you're seeking.

If putting them in bookmarks.html is the only way to do it, then I'd accept that
over losing them regularly.
As others have pointed out, from a user's perspective the importance here is
that many of us use favicons as bookmarks without text, to save a lot of space
on the toolbar, and have a lot more bookmarks visible there.

When the PC crashes, we lose them all, and have to reconsitute, which is very
annoying.


If making a separate cache for favicons, it could be considered to have it sync
in the backgroud at every x hours, days or weeks. The value x could be changable
via  the the properties for the bookmark or the moz-preferences. Of course a
manual update is a must.
It makes the most sense to use a separate cache than to embed the favicons into
the bookmarks, since that way they would still be very flexible; meaning to say
that it is not hard to move/copy a folder whenever you have to delete your
profile or anything in that sense.
And would this cache be similar to the RDF caches used for panel locations,
downloads, form/page history and the like? Being able to back up your favicons
alongside your bookmarks and having Firefox restore both when you create a new
profile for whatever reason would be useful.
It seems that Ian Pottinger's solution from &#1089;omment #31 (RFC 2397 "data:"
protocol) is the most natural way.

I know, some of you think that bookmarked favicons should not be stored directly
into bookmarks.html file. Well, folks, you must have never looked into
bookmarks.html -- it already contains icon="..." attribute for each <a> element
with favicon. All we need is make "http://..." URLs obsolete there, and use
"data:..." instead -- the bug will be fixed immediately. (I hope "data:" URLs
aren't ever searched in common cache, are they? Naturally, those "data:" URLs
should be immediate supply for icon images.) There'll be no need for CDDATA (or
whatever other wrapping) to make favicon data invisible to RDF/XHTML/XML/HTML or
whatever other parser, since ICON="data:..." already is a kind of SGML-compliant
attribute, no forbidden symbols/glyphs/codes, just lots of genuine ASCII. And
even for those who love hand-editing (or hand sync) of their bookmarks, it won't
make a pain to skip 10 or 30 lines of what is obliously not a text.

Please note also: the favicon file has to be "data:"-encoded AS IS. The above
proposed Rastignac's solution from comment #7 (favicon converted to 256 colour
PNG 16x16) incorrectly assumes that favicon is always the same image. However,
Windows ICO files are actually image libraries sometimes containing several
icons suitable for different resolutions (from 16x16 to 48x48) and different
color spaces (2, 16, 256 colors or truecolor). Those ICO files should be
entirely (AS IS) stored (in "data:"-encoded format) inside bookmarks.html file,
if only we are going to have bug 110894 (favicons on webpage "shortcuts") fixed,
with genuine icons _immediately_ being created and attached to shotcut (which is
dragged-and-dropped bookmark item).

[Please visit bug 120352 (which is a metabug for most of icon issues), where
some other applications of original icons are mentioned. Those are reasons for
us to keep unmodified favicon data stored.]

And, to make sure that favicon image itself is up to date, we may use
LAST_MODIFIED="..." or LAST_VISIT="..." attributes (which are previous for each
ICON="..." in bookmark tag inside bookmarks.html), checking it against certain
data in HTTP response.

If I had a normal CVS access (which is actually not available for my
workstation, closed by enterprise firewall), I probably would have coded that
junk myself. Hope the above plenty of hints are enough for somebody else to make
a patch...

Anyway, bug 117895 --> my votes
Just be aware that there is nothing limiting the image dimensions or filesize
(afaik) for favicons.  Bookmarks.html will go from a small text file to
something potentially very large.
I'm not quite sure data: URLs are workable.  RFC 2397 mentions attribute/URL/tag
lengths in HTML (section 2. Description) (which we're definitely using -
bookmarks HTML is ugly), and we might be too big for them in many instances.

Or does Gecko ignore the length limits of HTML?  (Or am I not thinking correctly?)
Does XBEL (bug 55057) have that limitation?
IMO, the Summary SHOULD say that favicons should be stored in a semi-permanent
cache, separate from web content, not just in "bookmarks." Unless "bookmarks"
means in Toolbar entries, tooolbar folders, bookmarks menu etc.

I understand that Mr. Hyatt long ago left moz and went to work for Apple; why is
this still assigned to him?
Neil, can you take this one?  If not, can you help find someone else, or give it
to nobody@mozilla.org.

/be
Assignee: hyatt → neil.parkwaycc.co.uk
Status: ASSIGNED → NEW
This is just so those of you who need a quick fix can have it.

It only affect bookmarks you visit since applying the patch.
Comment on attachment 145346 [details] [diff] [review]
Cheezy patch to store icons in bookmarks.html

the channel should probably set the load flag
nsICachingChannel::LOAD_ONLY_FRM_CACHE. either that or this needs to use
asyncOpen, because as-is this is blocking the ui thread.
Saving in bookmarks.html is not a good idea if you ask me.
In my bookmarks there are a dozen of different bookmarks with the same favicon,
with the "bookmarks.html" method it would be saved a dozen times wich isn't
really efficient. You shouldn't have to save the same favicon twice, if you ask me.
At least that fixes the original bug report, icons does not disappear anymore.

Still, I suggest that sometimes current date should be checked against
LAST_MODIFIED attribute, to refresh the certain icon image.
Sorry, I'm somewhat of a computer newbie.  Can someone please tell me clearly
what I'm supposed to do with the "cheezy patch?"  Thanks in advance.
(In reply to comment #56)
> Sorry, I'm somewhat of a computer newbie.  Can someone please tell me clearly
> what I'm supposed to do with the "cheezy patch?"  Thanks in advance.

Save the patch to your hard drive; give it a *.txt name to avoid hassles.  Close
the browser.  Go into the program folder for your browser.  Find the chrome
directory.  View the list of files in it.


**Branch for different toolkits:**
**Branch for different toolkits:**

For Firefox:
Rename browser.jar to browser.zip.  Open it up with your favorite archive
program.  Extract "content/browser/browser.js" from it.  Open it with a suitable
text editor (Wordpad if on Windows due to Unix line endings [?], whatever if
elsewhere).  Do a search for "function HandleBookmarkIcon(iconURL, addFlag)". 
Move down to the line containing "if (addFlag)".  Select this line and the three
below it; they should consist of everything down to
"BMSVC.removeBookmarkIcon(url, iconURL);".

For Mozilla Suite (Seamonkey):
Rename comm.jar to comm.zip.  Open it up with your favorite archive program. 
Extract "content/navigator/navigator.js" from it.  Open it with a suitable text
editor (Wordpad if on Windows due to Unix line endings [?], whatever if
elsewhere).  Do a search for "function HandleBookmarkIcon(iconURL, addFlag)". 
Move down to the line containing "if (addFlag)" (it should be followed by more
text).  Select this line and the line below it; this selection should consist of
everything to "gBookmarksService.removeBookmarkIcon(url, iconURL);" (the end of
the next line, most likely).

**End branch**
**End branch**


Open the *.txt patch file.  Select all the lines starting with a "+"; copy them
to the clipboard.  Move back to the *.js file you had open (browser.js or
navigator.js) and overwrite the selected lines with the lines you just copied
from the patch.  Now go back and delete the "+" at the beginning of each of the
newly pasted lines; all these newly inserted lines should start with whitespace
when you are done.  Save the file.  Now, reinsert it back into the *.zip file
(browser.zip or comm.zip) which you renamed from *.jar (browser.jar or comm.jar)
in its original location.  This should overwrite the original version of the
*.js file; tell the archive program to overwrite the old file if prompted. 
Rename the *.zip file back to its original name (browser.jar or comm.jar).

At this point all patching should be done, and you can start up the browser. 
This should work correctly in Mozilla; I haven't tested in Firefox, but I assume
the relevant XPCOM interfaces are shared by the old and new toolkits.  (I
actually can't test right now because I'm not using my own computer.)

This should *theoretically* work, but your mileage may vary.
JFYI, I've asked one of the unofficial Firefox builders to look at this patch
and test it out in his builds. I will report back on my experiences with
browsing and favicons with this patch applied.
There is a discussion of this bug in the "Firefox Features" Mozillazine forum.
I have posted a long comment there,
at http://forums.mozillazine.org/viewtopic.php?p=470661#470661
Please advise if it would be useful and appropriate for me to post any
of that here.

I have tried the patch with mozilla-firefox and doesn't work here.
Re: comment 60, it doesn't work on my 20040614 build of Firefox either, but
worse, it seems to cause bookmark favicons to not work at all for me (I editted
bookmarks.html, removed the ICON attribute from my slashdot.org bookmark, ran
Firefox, went to the bookmark, exited Firefox, and editted bookmarks.html again.
Neither a data: URL nor the typical ICON="http://..." attribute were present). 
A patch for this went in for bug 174265, for Firefox on the aviary branch; not
sure how/when/if it'll make it to seamonkey.
The bug is solved in the nightly release. 
You can download it from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/

Hmm.. I'm not really comfortable with the idea of my bookmarks file having
binary content. Opera stores favicons in the /images folder and the cache in
then /cache4 folder. I'd rather have my bookmark have a pointer to the URLs of
the images in some attribute of the entry than have it embedded.
Product: Core → Mozilla Application Suite
How about "icon:" scheme?
Currently, "icon:file:///c:/something.exe"
redirects to the system icon image data.

In the same way, "icon:http://www.google.com/"
redirects to "%profile%/icon/_random_.ico"
only when the url(uri) is registered in the,
namely, icon-datasource...?

If it can nicely *delete* obsolete files&#12288;in
this icon folder, favicon for history panel
would be easier to implement.
Since adding the Favicons to the bookmarks file seems to have complications,
what's wrong with adding a Favicons file next to the bookmarks file in a persons
profile folder? This could be a permanent fix or a temporary fix until other
matters are sorted out. This would at least start the ball rolling and resolve a
major user annoyance.
Yeah, the two issues that annoy me with the favicon in bookmark thing are:
* History side bar doesn't get them so easily.
* 'smart' bookmarks (rss feeds, etc) don't get them. :(. It'd be really good if
they could.
No longer blocks: majorbugs
This was fixed by bug 174265 and again by Places.
Status: NEW → RESOLVED
Closed: 23 years ago → 16 years ago
Resolution: --- → FIXED
this is a suite bug, not a firefox bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Resetting A+QA on the assumption that the present (nondefault) ones don't reflect anything current. Neil, if I assumed wrong, feel free to accept again.
Assignee: neil → nobody
Status: REOPENED → NEW
QA Contact: claudius → guifeatures
Component: XP Apps: GUI Features → UI Design
Assignee: nobody → jag
QA Contact: guifeatures
Target Milestone: Future → ---
Assignee: jag → nobody
QA Contact: ui-design
On trunk we are using the Places backend and the favicon service. Resolving as WORKSFORME now that we are API parity with Firefox.
Status: NEW → RESOLVED
Closed: 16 years ago → 13 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: