favicons aren't backed up to *.json

RESOLVED WONTFIX

Status

()

defect
--
major
RESOLVED WONTFIX
11 years ago
2 years ago

People

(Reporter: zeniko, Unassigned)

Tracking

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 -
blocking-firefox3.5 -

Firefox Tracking Flags

(firefox-esr45 affected, blocking2.0 -, firefox51 affected, firefox52 wontfix, firefox-esr52 affected, firefox53 affected, firefox54 affected)

Details

Attachments

(1 attachment)

Reporter

Description

11 years ago
Steps to reproduce:
1. Bookmark some pages with favicons
2. Close Firefox and delete places.sqlite
3. Restore your bookmarks from the JSON backup

Actual result:
All pages show the default "blank page" icon.

Expected result:
Bookmarks are easy to distinguish from the beginning thanks to the favicons. This is especially important for when the favicon was manually assigned (e.g. for JavaScript bookmarklets).
Flags: blocking-firefox3?
Reporter

Comment 1

11 years ago
In case performance/size should be the issue, at least the favicons' URLs should be backed up (resp. the full data: URLs where there's no http(s) one).

NB: Otherwise there's currently the choice between losing tags (HTML) or favicons (JSON). Not really the ideal perspective for data conscious users...
Do we currently store them in the SQLite? Is it a matter of just copying some data URI strings?
(In reply to comment #2)
> Do we currently store them in the SQLite? Is it a matter of just copying some
> data URI strings?

Yes, they are stored in the table moz_favicons. Adding Dietrich who could answer your latter question.
Not blocking, but a patch with tests would get my a+ ...
Flags: blocking-firefox3? → blocking-firefox3-

Comment 5

11 years ago
Bug 419731 and Bug 424669 have just been resolved. Since titles are now fully restored, this would be the final piece of the json backup. Is nobody able to get a quick working patch for FF3?
Also happens with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008040904 Minefield/3.0pre
OS: Windows XP → All
Hardware: PC → All
Depends on: 429832

Updated

11 years ago
Duplicate of this bug: 444530
Flags: blocking-firefox3.1?
Would definitely like to see this fixed, but won't block 3.1 for it.
Flags: wanted-firefox3.1+
Flags: blocking-firefox3.1?
Flags: blocking-firefox3.1-
Priority: -- → P2
Target Milestone: --- → Firefox 3.1
Duplicate of this bug: 471278

Updated

11 years ago
Assignee: nobody → adw
Here's a stab at it.  It simply serializes favicon URIs on backup and setAndLoadFaviconForPage's on restore.

Works fine, but can someone who knows the code better tell me if it's wise to (asynchronously) load an entire database's worth of favicons after restore?  I suspect not... It would be easy to store the data URIs directly in the JSON, at the cost of course of a bigger backup file.

I will try and find a way to get some numbers to compare the tradeoff.
I've done some preliminary testing of these two approaches:

1. Store only favicon remote URLs.  Asynchronously load data after restore.
2. Store both remote URLs and data URIs.  During restore, manually set each favicon's data.

The upshot is, not surprisingly, approach 1 is faster on both backup and restore, and its resulting JSON is smaller.

I tested the completion time of calls to PlacesUtils.backupBookmarksToFile and PlacesUtils.restoreBookmarksFromJSONFile.  There were 3 rounds of tests with 10000, 1000, and 100 bookmarks each.  Each round I tested both approaches, for each approach both backup and restore, and for each of those I ran the test 5 times and took the average time.  I did the same for a control, meaning I left out favicon info altogether (i.e., the code as it is now).

Here's how the two approaches compared to the control:

10000 bookmarks:
  Approach 1:
    Backup:  16.2405 % more time to complete than control
    Restore:  7.1314
  Approach 2:
    Backup:  34.1658
    Restore: 35.4294

1000 bookmarks:
  Approach 1:
    Backup:  20.7603
    Restore: 13.5342
  Approach 2:
    Backup:  34.1601
    Restore: 83.0130

100 bookmarks:
  Approach 1:
    Backup:  17.0163
    Restore: 14.1269
  Approach 2:
    Backup:  28.9044
    Restore: 80.4761

Even less surprising is the JSON backup file size:

10000 bookmarks (1000 and 100 are similar):
  Approach 1:  46.5646 % bigger than control
  Approach 2: 600.0019

My assumptions in the test bookmarks were:
- Each bookmark has a unique favicon
- Data URI average size is 798 characters

A third approach would be a combination of the first two, where we store full data URIs for some bookmarks and only remote URLs for others.  Obvious candidates for storing full data URIs are favicons behind walls that require user interaction to climb, like passwords.  A drawback of approach 1 is that such favicons are lost unless the user visits them before his history is cleaned up and they're expired -- which happens no later than when he closes the browser.  But there doesn't appear to be a way to tell whether a favicon URI returned by the favicon service is behind a password.  https yes, but while many passwords are over https, not all are.  Maybe it's the best we can do?

Other drawbacks of approach 1:
- If user quits browser before a favicon loads, it will be lost for good.  The more bookmarks or the slower the connection, the longer he has to wait.  If he's not online at all when he restores, not only will he not be able to see his favicons, they'll be gone the next time he starts the browser.
- Adds more network activity after restore (duh).

Comments?

Comment 12

11 years ago
I would think that many users wouldn't really want to have a flurry of favicon downloads on restore. Fully backing up the icons to the JSON itself sounds like the best route to me. Besides, HTML backups store favicons using data URIs so the most logical thing to do here is to do the same.
As we talked on IRC, i think a good approach could be to backup https for the reasons you say, but also backup favicons for bookmarks on the toolbar, since they appear immediately and some user has toolbar without labels (restore for them makes the toolbar unusable). And probably also bookmarks where the favicon is set by the user (see bookmarklets case above). That should reduce the volume of favicons data to save to about 1/3, for the remaining part we could save the favicon uri.
We could change expiration of the empty favicons to allow more time for favicon service to lazy/async load missing ones, and only expire unused ones.
This needs further considerations and Dietrich's approval clearly, before putting hands on code.

About the performances, the backup happens often on shutdown, but luckily only on the first shutdown in the day (personally i would reduce the idle timeout, i'll ask dietrich about that). the restore is an op where the user is expecting to have some waiting... so till the loss is some second probably won't be an issue.
actual more important use cases we found:
- bookmarklets with a user defined icon
- toolbar bookmarks (users without labels on toolbar), only direct children
- recently accessed (or most visited) bookmarks (with an hard limit)
This could be a high-impact change, as we run backup daily. Too late in the 3.1 cycle for this. Re-targeting to 3.2.
Target Milestone: Firefox 3.1 → Firefox 3.2a1
Flags: wanted-firefox3.1+
Flags: wanted-firefox3.2?

Updated

10 years ago
Assignee: adw → nobody

Comment 16

10 years ago
Any chance to get this in 3.6 final?
Tomas, unless one of the devs knows something I don't, probably not. I don't even think anyone is working on this right now.
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
Duplicate of this bug: 538489

Comment 20

9 years ago
Don't want to be annoying, but is it going to be in Firefox 3.7 (or whatever is the next major version)? What are the plans?
I think this is waiting for a decision between the methods proposed in Comment 11.

Updated

9 years ago
blocking2.0: --- → ?
status2.0: --- → ?
Flags: wanted-firefox3.6?

Comment 22

9 years ago
(In reply to comment #11)
> I've done some preliminary testing of these two approaches:
> 
> 1. Store only favicon remote URLs.  Asynchronously load data after restore.
> 2. Store both remote URLs and data URIs.  During restore, manually set each
> favicon's data.
> 
> ...
> 
> Comments?

IMHO we need the second approach. Back-up is a very important feature, and in no case any data should be lost (or don't call it back-up then). With the first approach, restoring a favicon depends on so many external factors: unstable internet connection, resources that need authorisation, unexpected browser closing, etc. It's not an option. The second way seems to be perfectly reliable, though at the cost of speed and file size. To me it's worth losing some extra time and a couple of megabytes just to be sure that I can always restore 100% of my data.
This doesn't block for the same reasons it didn't block past releases.
blocking2.0: ? → -
status2.0: ? → ---
should this bug have keyword dataloss ?
Don't think so - the icons can be redownloaded (and would expire eventually too I think).

Comment 26

9 years ago
(In reply to comment #22)
> no case any data should be lost (or don't call it back-up then)
No. Backup/restore is not for normal operation, it is for emergency. Therefore, only important and/or user defined data should be stored. Favicon is nothing out of this. For anyone who disagree: page content is far more important than favicon, but isn't backup.

I strongly suggest to WONTFIX this bug.
Firefox should be quick.

PS. If someone do care about his bookmarks, then he should use OS-provided backup, or Weave, or something like Scrapbook addon.
I don't agree...
If exporting is slow devs must simply speed up this like in other browsers or even programs and all exported data should be preserved

Comment 28

9 years ago
(In reply to comment #26)
> 
> PS. If someone do care about his bookmarks, then he should use OS-provided
> backup, or Weave, or something like Scrapbook addon.

Sync (nee Weave) will only include the bookmark because favicons are stored separately (which is also the reason for this bug), therefore, it is not an alternative.

Another competitor, Xmarks (nee Foxmarks) must also be considered. AFAICT-Xmarks does sync favicons with bookmarks. For the devs to concede this to Xmarks can be seen as Firefox not being able to "compete". A minor point, perhaps, but still a shortcoming against the competition.

Updated

9 years ago
Target Milestone: Firefox 3.6a1 → ---

Comment 29

9 years ago
It would be nice if this could be worked on for 3.6.x prior to 4.0 release, this would ease the transition for people who like to make new profiles(/install fresh) with new major versions. This function shouldn't be something that relies on extensions.

Comment 30

8 years ago
(In reply to comment #28)

> Another competitor, Xmarks (nee Foxmarks) must also be considered.
> AFAICT-Xmarks does sync favicons with bookmarks. For the devs to concede this
> to Xmarks can be seen as Firefox not being able to "compete". A minor point,
> perhaps, but still a shortcoming against the competition.

Xmarks doesn't support favicons anymore. i don't think it's a wise move to rely on third party software for bookmark management.

i strongly agree with Sid, Virtual_ManPL, and Scott A.; and strongly disagree with Shawn Wilsher, and Mike.

favicos are neglected by some portion of web users. but for information seeking professionals, favicons are vital. if you are an academic, journalist, investigator, archeologist, or software developer bookmarks accumulate in your browser and become increasingly essential. information seeking is what Internet is about. and favicons are great usability enhancers for Internet users.

IMHO, for a modern browser project, neglecting favicons is [epic fail].

loosing favicon is loosing important data. bookmark users seek for favicons in the toolbars. they don't deal with reading the title. a bookmark is more a favicon than a title.

relying on a web-sourced favicon management (only storing the URI of the .ico file) is not the robust way of browser development. firefox should "store locally". and "backup locally". and "restore locally".

finally, Firefox 4.0 users are faced with the problem too.
Exactly, I have over 6MB of exported HTML file of bookmarks.
If I want to restore backup from .json I need to open manually over 10.000 links to get a favicon...

What's the status in fixing this bug ?
Will it make to Firefox 5 ?
If not what's the plan.
Duplicate of this bug: 663557
not a regression, since we never added icons to json backups from the beginning
Keywords: regression
Priority: P2 → --

Comment 34

4 years ago
Not only the favicons are missing, the whole import function is broken for many years yet.
https://bugzilla.mozilla.org/show_bug.cgi?id=950533
This is still an issue on latest builds (54.0a1, 53.0a2, 52.0 RC and 52.0 ESR) across platforms.
I think we need a different solution for this, now that favicons will change, each page can have more than one.
It's unlikely we'll ever add favicon payloads to the json, it would become huge.
The best solution would be to fetch favicons in background for any page lacking one, we should probably spend time figuring out the possible privacy and perf implications there, rather than spending time on this.
Too late for firefox 52, mass-wontfix.
wontfixing per comment 36, we need a different solution, surely we won't store favicons in the json file.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.