Last Comment Bug 366075 - on shutdown, expire history entries based on the browser.history_expire_days
: on shutdown, expire history entries based on the browser.history_expire_days
Status: RESOLVED WONTFIX
: relnote
Product: Firefox
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: Trunk
: All All
: P2 normal with 10 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
: 476868 (view as bug list)
Depends on: 520165
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-05 13:14 PST by (not reading, please use seth@sspitzer.org instead)
Modified: 2011-01-24 04:52 PST (History)
28 users (show)
mconnor: blocking‑firefox3-
dietrich: blocking‑firefox3.5-
mconnor: wanted‑firefox3+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
wip (9.05 KB, patch)
2007-10-15 14:49 PDT, Dietrich Ayala (:dietrich)
no flags Details | Diff | Review
more (9.35 KB, patch)
2007-10-16 00:58 PDT, Dietrich Ayala (:dietrich)
no flags Details | Diff | Review

Description (not reading, please use seth@sspitzer.org instead) 2007-01-05 13:14:15 PST
[places] on shutdown, expire history entries based on the browser.history_expire_days

from ria:

Setting browser.history_expire_days to 0 has no effect since Bug 355738 landed,
it does not delete the history entries from the sqlite file; history is still
present in the sidebar. Although it stops gathering new history.
The sanitize function (Tools > Clear Private Data) works however.
Could be that this is an independent bug, for I see it also without imported
history.

So the steps are:
- Start Firefox in a new profile
- Visit www.cnn.com
- Set Tools > Options > Privacy > History > Remember visited pages to 0 days
- Close the browser
When you start the browser you'll see that www.cnn.com is still present; should
be empty. 

Firefox 2.0 appears to do this work in nsGlobalHistory::CloseDB(), see
http://lxr.mozilla.org/seamonkey/source/toolkit/components/history/src/nsGlobalHistory.cpp#3136

minefield doesn't seem to be doing this.  

also, I wonder if we should expire history entries when we observe the pref changes, and not on shutdown.
Comment 1 Phil Ringnalda (:philor) 2007-01-05 14:28:31 PST
See bug 328598 - you don't want to expire on shutdown.
Comment 2 (not reading, please use seth@sspitzer.org instead) 2007-01-05 14:38:05 PST
thanks for the pointer, phil.

so, what about the idea of doing the expiration in the pref observer (in nsNavHistory.cpp) when the user changes the pref (but only if the "new" expire days is less than the "old" expire days.)

we'll still be paying a price for doing the expiration, but at least it will be associated with the action taken, and not on shutdown.

note, firefox 2.0 does not behave this way, but I think this would be more correct, as disabling the "Remember pages visited" pref would immediately clear history.

comments?
Comment 3 Brett Wilson 2007-01-05 19:54:48 PST
To avoid delaying shutdown, places expires history as you browse. On shutdown, it does some extra work to catch some edgecases (the *Paranoid functions). This code is in toolkit/components/places/src/nsNavHistoryExpire.cpp

I'm not sure if the 0 days case is handled properly. In this case, OnAddURI
may never get called, and this is the function that schedules incremental expiration. OnExpirationChanged should handle all non-zero cases, although the items won't immediately be deleted (I think it's 5 items deleted per page visited).

For the 0 day case, you may want to call nsNavHistoryExpire::ClearHistory in OnExpirationChanged when the days are 0.

If it's really important that when the user decreases the expiration time those items get deleted immediately (I doubt this is important), you may want to modify ExpireItems. It takes the maximum number of things to expire as a parameter. Currently, we use 0 to mean expire all history. You may want to do something like making -1 expire all history, and 0 expire as many items as possible given the current expiration time. Then call this from OnExpirationChanged.
Comment 4 Ria Klaassen (not reading all bugmail) 2007-01-26 00:02:39 PST
Problem is (also) that none of the history is deleted when I change browser.history_expire_days from 600 days to 1 day. I changed it 24 hours ago and al my history is still there.
Comment 5 Mike Connor [:mconnor] 2007-08-11 23:40:34 PDT
Might need to relnote this for M8, moving out to M9.
Comment 6 Dietrich Ayala (:dietrich) 2007-10-15 14:49:29 PDT
Created attachment 285002 [details] [diff] [review]
wip

the problem was that we currently do zero expiration at shutdown in some cases, even when there might be history to expire.

when that was combined with our really slow incremental expiration (which also doesn't expire oldest to newest btw), it basically would never catch up.

this fix does the following:
- always run expiration at shutdown
- do expiration when 15 mins idle, hooking into the history vacuum timer for this (might want to alternate or something)

expiring 6mos+ of history took about 20 secs w/ a debug build. i'd prefer to err on the side of privacy here though, and fix perf in bug 395066.
Comment 7 Dietrich Ayala (:dietrich) 2007-10-16 00:58:42 PDT
Created attachment 285053 [details] [diff] [review]
more
Comment 8 Mike Connor [:mconnor] 2008-03-17 11:52:09 PDT
Not blocking on this bug for final ship. Would take a safe enough patch if one comes through.
Comment 9 Marco Bonardo [::mak] 2008-04-19 03:20:48 PDT
(In reply to comment #6)
> the problem was that we currently do zero expiration at shutdown in some cases, even when there might be history to expire.

i'm not sure this is still true

> (which also doesn't expire oldest to newest btw)

this is no more true after latest changes
Comment 10 Dietrich Ayala (:dietrich) 2008-05-15 08:56:09 PDT
(In reply to comment #4)
> Problem is (also) that none of the history is deleted when I change
> browser.history_expire_days from 600 days to 1 day. I changed it 24 hours ago
> and al my history is still there.
> 

filed bug 433893 for this
Comment 11 Dietrich Ayala (:dietrich) 2008-05-15 09:01:03 PDT
Fixing this bug requires a full and complete truncation of history to match the prefs at shutdown, overriding the partial expiration mechanisms we have in place now.
Comment 12 Samuel Sidler (old account; do not CC) 2008-08-29 18:06:21 PDT
Since we're printing this as a known issue for Firefox 3, we should look at fixing for 3.1.
Comment 13 Dietrich Ayala (:dietrich) 2008-10-20 17:51:02 PDT
Approving blocking request, this should be fixed. as well as at shutdown, the user expects history to be pruned after changing the setting.

We could do this w/o blocking UI by moving the expiration queries into the background.
Comment 14 Dietrich Ayala (:dietrich) 2008-12-02 11:12:46 PST
Aaaannnnd removing the blocking flag: no major problems reported since the 3.0 release, so this doesn't need to block 3.1. We're making more progress on moving non-browsing-synchronous tasks to the background, so fixing this bug is becoming feasible.
Comment 15 Nochum Sossonko [:Natch] 2009-02-04 14:31:41 PST
*** Bug 476868 has been marked as a duplicate of this bug. ***
Comment 16 Gervase Markham [:gerv] 2009-11-26 07:23:23 PST
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
Comment 17 Tomas 2011-01-23 17:01:07 PST
Does https://bugzilla.mozilla.org/show_bug.cgi?id=520165 make this wont fix?
Comment 18 Marco Bonardo [::mak] 2011-01-24 04:52:11 PST
(In reply to comment #17)
> Does https://bugzilla.mozilla.org/show_bug.cgi?id=520165 make this wont fix?

yes

Note You need to log in before you can comment on or make changes to this bug.