Closed Bug 329741 Opened 15 years ago Closed 13 years ago
.dat, formhistory .dat, downloads .rdf should be deleted when the user clears private data
<Jesse> does history.dat ever get deleted after switching to places? <Ryan> I don't think so... If not, it could be a privacy problem -- the sites you visited in the weeks before you upgraded to Firefox 2 will be listed on your computer forever, whether you clear history or not.
No we never delete history.dat. Right now this is by design so people can run places and non-places builds side-by-side, and also because we keep changing the database, requiring people to re-import. For release, we should have some kind of plan. Perhaps when you clear your history the first time we go and delete it.
Priority: -- → P5
Summary: history.dat is never deleted after Places import (?) → history.dat should be deleted at some point
Target Milestone: --- → Firefox 2 beta2
> No we never delete history.dat. Right now this is by design so people can > run places and non-places builds side-by-side, and also because we keep > changing the database, requiring people to re-import. That's reasonable for alphas. > For release, we should have some kind of plan. Perhaps when you clear your > history the first time we go and delete it. But then if you never clear history, your history from the ~2 weeks before you upgraded to Firefox 2 will be on your HD forever.
Maybe a better idea is to notice when we import history.dat and set a preference that indicates the expiration date of this file. This would be today + the number of days your history is valid for. On shutdown we check this preference, and if we are past the date, we delete history.dat and the preference.
*** Bug 333199 has been marked as a duplicate of this bug. ***
We should be careful for people who use multiple versions of the browser with the same profile. If the files are in use (perhaps because they've been changed since we upgraded) then we shouldn't do any automatic deleting.
Summary: history.dat should be deleted at some point → history.dat, formhistory.dat, and bookmarks.html should be deleted at some point
*** Bug 334201 has been marked as a duplicate of this bug. ***
For history.dat, modification date of the file + the usual history expiration pref dtrt, since the contents would have expired by then even when using a version using that file.
14 years ago
OS: Mac OS X 10.2 → All
Hardware: Macintosh → All
14 years ago
Target Milestone: Firefox 2 beta2 → ---
*** Bug 362003 has been marked as a duplicate of this bug. ***
FWIW: I'm looking at some password manager changes, which will probably include moving the saved passwords to a MozStorage (SQL) file. So it will have the same problem as Places with respect to old files.
Need a clear policy here, for the pwmgr file format changes we deleted on upgrade, but that was higher-risk.
Flags: blocking-firefox3? → blocking-firefox3+
we should not delete bookmarks.html, as we still write out bookmark data to that file which can be used by other browsers, and by ourselves on force places db migration (on schema version change).
Summary: history.dat, formhistory.dat, and bookmarks.html should be deleted at some point → history.dat, formhistory.dat should be deleted at some point
Need to have a solution in place by beta 1, though we might not activate that solution until we get to the RCs
Target Milestone: --- → Firefox 3 beta1
re-targeting to what might be beta 1.
Target Milestone: Firefox 3 M7 → Firefox 3 M8
(In reply to comment #7) > For history.dat, modification date of the file + the usual history expiration > pref dtrt, since the contents would have expired by then even when using a > version using that file. > this was reasonable when the default history expiration was 9 days. however, it has recently changed to 180 days.
here's a patch that deletes the old history file at 9 days past last-modified time. a couple of possible improvements: 1. if the history days pref is not 180, use it, else use 9. 2. delete the old history file whenever all of history is cleared. does this approach sound good?
Comment on attachment 275137 [details] [diff] [review] fix v1 instead of 180, can you check if mExpireDays is equal to the default value of the expire pref (from the default pref branch?) 180 may change on us. (We have to hard code 9, however.)
Comment on attachment 275188 [details] [diff] [review] fix v2 r=sspitzer consider a comment or verbose #define (DEFAULT_EXPIRE_DAYS_IN_FIREFOX_2) to explain the 9 constant.
Attachment #275188 - Flags: review?(sspitzer) → review+
(In reply to comment #12) > Need to have a solution in place by beta 1, though we might not activate that > solution until we get to the RCs > holding off on checking this in.
Status: NEW → ASSIGNED
Assignee: dietrich → nobody
Status: ASSIGNED → NEW
Target Milestone: Firefox 3 M8 → Firefox 3 M9
dietrich / sdwilsh / dwitte: what about cookies.txt and downloads.rdf? It seems like we should delete those as well.
Summary: history.dat, formhistory.dat should be deleted at some point → history.dat, formhistory.dat, downloads.rdf should be deleted at some point
My concern is what happens if the user goes back to 2.0? Do we just nuke it and leave them "high and dry"?
i decided to have cookies.txt be deleted upon successful migration to cookies.sqlite on trunk, for precisely this reason... i figured the downside of having orphaned private data outweighed the dataloss risk/inconvenience of not being able to go back and forth between trunk/branch and such. so, not an issue with cookies.
Target Milestone: Firefox 3 M9 → Firefox 3 M10
Moving to M11 since we want to do this close to the end.
Target Milestone: Firefox 3 M10 → Firefox 3 M11
Do we want to land this before or after b3? I guess immediately after?
Whiteboard: [has patch] → [has patch][ready to land when needed]
> Do we want to land this before or after b3? I guess immediately after? note, the last patch in this bug only covers history.dat, and not other files. keeping in mind that users might switch back and forth between fx 2 / 3 (for legit reasons, especially at first, for example if an add-on is not supported). Note, we are currently leaving bookmarks.html alone for this reason. Perhaps what we really want is to remove these files, if they exist, when the user choose to clear private data, in particular the private data that was associated with those fx 2 files. (So when you clear history, we will attempt to remove history.dat, instead of doing it in ::Init() of the history service, like the attached patch does.) something similar would happen for downloads.rdf and formhistory.dat
Whiteboard: [has patch][ready to land when needed]
Attachment #275188 - Flags: review+
Summary: history.dat, formhistory.dat, downloads.rdf should be deleted at some point → history.dat, formhistory.dat, downloads.rdf should be deleted when the user clears private data
Comment on attachment 275199 [details] [diff] [review] for checkin morphing bug and taking back my r+, because I think what we really want to do is remove these files when clearing private data. I guess one scenario where this could bite us is if the user attempted to manually clear private data by selecting all history and deleting it, but then again, they aren't guaranteed that will clear private data (like Tools | Clear Private Data... should do) mconnor/dietrich/jesse, what do you guys think?
Attachment #275199 - Flags: review-
Sure, we should add that. However, the CPD approach doesn't cover users who don't clear private data ever, but expect their history to be gone in XXX days. About switching back and forth between Fx2/3: This patch uses the last-modified-time of the history.dat on disk to determine whether it should be deleted, so if it's getting used, we won't remove it.
Comment on attachment 275199 [details] [diff] [review] for checkin >+ // if expired, remove >+ PRInt64 expireDays = (mExpireDays == defaultExpireDays) ? >+ DEFAULT_EXPIRE_DAYS_IN_FIREFOX_2 : mExpireDays; nitpickery I suppose, but why is this necessary when the default is being increased? There doesn't seem to be a privacy reason to not keep history.dat around for as long as the current implementation is keeping history regardless of whether that value comes from defaults, so isn't this extent of simulating when history.dat contents would have expired when using fx2 is superfluous?
Assignee: dietrich → mconnor
Priority: P3 → P2
Note to self: bug 412381 added support in the password manager for deleting old signons.txt files when clearing private data.
adds cleanup code to the clear all functions in the right places, so consumers don't have to think about it. no tests yet, but throwing this out there for now.
Attachment #275199 - Attachment is obsolete: true
I think you mean copyright 2008 in the license blocks ;)
Comment on attachment 309005 [details] [diff] [review] patch with tests >Index: toolkit/components/places/src/nsNavHistory.cpp >+ nsresult rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, >+ getter_AddRefs(oldHistoryFile)); >+ NS_ENSURE_SUCCESS(rv, rv); >+ rv = oldHistoryFile->Append(NS_LITERAL_STRING("history.dat")); >+ NS_ENSURE_SUCCESS(rv, rv); Why not use NS_APP_HISTORY_50_FILE to match the code in nsNavHistory::Init? >Index: toolkit/components/satchel/test/unit/head_satchel.js >+cleanUpFormHist(); >+ >+ >\ No newline at end of file Add one, get rid of the extra trailing spaces? In general you're treating failure to remove the file as a fatal error - should we do that last in these methods, to ensure that we at least clear the "live" data before possibly failing to remove the old files? I suppose the removal is unlikely to fail, so it probably doesn't matter.
Attachment #309005 - Flags: review?(gavin.sharp) → review+
(In reply to comment #35) > (From update of attachment 309005 [details] [diff] [review]) > >Index: toolkit/components/places/src/nsNavHistory.cpp > > >+ nsresult rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, > >+ getter_AddRefs(oldHistoryFile)); > >+ NS_ENSURE_SUCCESS(rv, rv); also, please use |if (NS_FAILED(rv)) return rv;| here, because this function frequently fails (eg when running some kinds of test) and adds to debug warning spew.
Attachment #309005 - Attachment is obsolete: true
Checking in toolkit/components/satchel/Makefile.in; /cvsroot/mozilla/toolkit/components/satchel/Makefile.in,v <-- Makefile.in new revision: 1.3; previous revision: 1.2 done Checking in toolkit/components/satchel/src/nsStorageFormHistory.cpp; /cvsroot/mozilla/toolkit/components/satchel/src/nsStorageFormHistory.cpp,v <-- nsStorageFormHistory.cpp new revision: 1.21; previous revision: 1.20 done RCS file: /cvsroot/mozilla/toolkit/components/satchel/test/Makefile.in,v done Checking in toolkit/components/satchel/test/Makefile.in; /cvsroot/mozilla/toolkit/components/satchel/test/Makefile.in,v <-- Makefile.in initial revision: 1.1 done RCS file: /cvsroot/mozilla/toolkit/components/satchel/test/unit/formhistory.dat,v done Checking in toolkit/components/satchel/test/unit/formhistory.dat; /cvsroot/mozilla/toolkit/components/satchel/test/unit/formhistory.dat,v <-- formhistory.dat initial revision: 1.1 done RCS file: /cvsroot/mozilla/toolkit/components/satchel/test/unit/head_satchel.js,v done Checking in toolkit/components/satchel/test/unit/head_satchel.js; /cvsroot/mozilla/toolkit/components/satchel/test/unit/head_satchel.js,v <-- head_satchel.js initial revision: 1.1 done RCS file: /cvsroot/mozilla/toolkit/components/satchel/test/unit/test_bug_329741.js,v done Checking in toolkit/components/satchel/test/unit/test_bug_329741.js; /cvsroot/mozilla/toolkit/components/satchel/test/unit/test_bug_329741.js,v <-- test_bug_329741.js initial revision: 1.1 done Checking in toolkit/components/places/src/nsNavHistory.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v <-- nsNavHistory.cpp new revision: 1.274; previous revision: 1.273 done RCS file: /cvsroot/mozilla/toolkit/components/places/tests/unit/history.dat,v done Checking in toolkit/components/places/tests/unit/history.dat; /cvsroot/mozilla/toolkit/components/places/tests/unit/history.dat,v <-- history.dat initial revision: 1.1 done Checking in toolkit/components/places/tests/unit/test_history.js; /cvsroot/mozilla/toolkit/components/places/tests/unit/test_history.js,v <-- test_history.js new revision: 1.10; previous revision: 1.9 done Checking in toolkit/components/downloads/src/nsDownloadManager.cpp; /cvsroot/mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp,v <-- nsDownloadManager.cpp new revision: 1.172; previous revision: 1.171 done RCS file: /cvsroot/mozilla/toolkit/components/downloads/test/unit/test_bug_329741.js,v done Checking in toolkit/components/downloads/test/unit/test_bug_329741.js; /cvsroot/mozilla/toolkit/components/downloads/test/unit/test_bug_329741.js,v <-- test_bug_329741.js initial revision: 1.1 done
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
This is causing various tests to fail. ../../../../_tests/xpcshell-simple/test_places/unit/test_adaptive.js: FAIL ../../../../_tests/xpcshell-simple/test_places/unit/test_browserhistory.js: FAIL ../../../../_tests/xpcshell-simple/test_places/unit/test_expiration.js: FAIL ../../../../_tests/xpcshell-simple/test_places/unit/test_frecency.js: FAIL ../../../../_tests/xpcshell-simple/test_places/unit/test_history.js: FAIL http://tinderbox.mozilla.org/showlog.cgi?tree=Firefox&errorparser=unittest&logfile=1205846207.1205848462.18696.gz&buildtime=1205846207&buildname=WINNT%205.2%20qm-win2k3-01%20dep%20unit%20test&fulltext= NEXT ERROR ../../../../_tests/xpcshell-simple/test_places/unit/test_adaptive.js: FAIL ../../../../_tests/xpcshell-simple/test_places/unit/test_adaptive.js.log: >>>>>>> *** test pending Test 0 same count, diff rank, same term; no search [Exception... "'Failure' when calling method: [nsIDirectoryServiceProvider::getFile]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: ../../../../_tests/xpcshell-simple/test_places/unit/test_adaptive.js :: prepTest :: line 192" data: no] *** FAIL *** Exception: [Exception... "Component returned failure code: 0x80520015 (NS_ERROR_FILE_ACCESS_DENIED) [nsIFile.remove]" nsresult: "0x80520015 (NS_ERROR_FILE_ACCESS_DENIED)" location: "JS frame :: ../../../../_tests/xpcshell-simple/test_places/unit/head_bookmarks.js :: clearDB :: line 92" data: no] <<<<<<<
Though I've been told that that snippet might be par for the course and simply not output unless the test failed.
Whiteboard: [has patch][needs review gavin] → [has patch]
Target Milestone: --- → Firefox 3 beta5
nsNavHistory::Init now depends on NS_APP_HISTORY_50_FILE, this should hopefully fix all the bustage (worked on my machine, unit test boxes are still cycling).
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
You need to log in before you can comment on or make changes to this bug.