Closed
Bug 423126
Opened 17 years ago
Closed 7 years ago
favicons aren't backed up to *.json
Categories
(Firefox :: Bookmarks & History, defect)
Firefox
Bookmarks & History
Tracking
()
People
(Reporter: zeniko, Unassigned)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
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•17 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...
Comment 2•17 years ago
|
||
Do we currently store them in the SQLite? Is it a matter of just copying some data URI strings?
Comment 3•17 years ago
|
||
(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.
Comment 4•17 years ago
|
||
Not blocking, but a patch with tests would get my a+ ...
Flags: blocking-firefox3? → blocking-firefox3-
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?
Comment 6•17 years ago
|
||
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
Updated•16 years ago
|
Flags: blocking-firefox3.1?
Comment 8•16 years ago
|
||
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
Updated•16 years ago
|
Assignee: nobody → adw
Comment 10•16 years ago
|
||
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.
Comment 11•16 years ago
|
||
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•16 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.
Comment 13•16 years ago
|
||
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.
Comment 14•16 years ago
|
||
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)
Comment 15•16 years ago
|
||
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
Updated•16 years ago
|
Flags: wanted-firefox3.1+
Updated•16 years ago
|
Flags: wanted-firefox3.2?
Updated•15 years ago
|
Assignee: adw → nobody
Comment 16•15 years ago
|
||
Any chance to get this in 3.6 final?
Comment 17•15 years ago
|
||
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.
Comment 18•15 years ago
|
||
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
Comment 20•15 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•14 years ago
|
Comment 22•14 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.
Comment 23•14 years ago
|
||
This doesn't block for the same reasons it didn't block past releases.
Comment 24•14 years ago
|
||
should this bug have keyword dataloss ?
Comment 25•14 years ago
|
||
Don't think so - the icons can be redownloaded (and would expire eventually too I think).
Comment 26•14 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.
Comment 27•14 years ago
|
||
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•14 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•14 years ago
|
Target Milestone: Firefox 3.6a1 → ---
Comment 29•14 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•14 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.
Comment 31•14 years ago
|
||
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.
Comment 33•11 years ago
|
||
not a regression, since we never added icons to json backups from the beginning
Keywords: regression
Updated•9 years ago
|
Priority: P2 → --
Comment 34•9 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
Comment 35•8 years ago
|
||
This is still an issue on latest builds (54.0a1, 53.0a2, 52.0 RC and 52.0 ESR) across platforms.
status-firefox51:
--- → affected
status-firefox52:
--- → affected
status-firefox53:
--- → affected
status-firefox54:
--- → affected
status-firefox-esr45:
--- → affected
status-firefox-esr52:
--- → affected
Comment 36•8 years ago
|
||
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.
Comment 37•8 years ago
|
||
Too late for firefox 52, mass-wontfix.
Comment 38•7 years ago
|
||
wontfixing per comment 36, we need a different solution, surely we won't store favicons in the json file.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
Updated•2 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•