Provide Capability to Expire History Entries by Age

NEW
Unassigned

Status

--
enhancement
3 years ago
6 months ago

People

(Reporter: david, Unassigned, Mentored)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][gfb][lang=js])

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

3 years ago
The capability for end-users to set a number of days beyond which browser history entries would expire and be removed was deleted from Toolkit/Places about 2011.  Repeated bug reports have been submitted requesting this capability be restored, including Bug #643254, Bug #660567, Bug #660646, and Bug #701180.  

Marco Bonardo's blog described the elimination of the capability.  There were many comments from end-users there opposing the change.  I can no longer find those comments.  

I am not alone in wanting this capability.  The Expire History by Days extension has 5,523 users.  That extension, however, cannot be considered a fix for this because (1) it cannot be installed without the end-user converting it for SeaMonkey and (2) there is no commitment to maintain it.
(Reporter)

Comment 1

3 years ago
Marco Bonardo cited the extension Expire History by Days in a comment in bug #660646 as a work-around.  This extension, however, does not appear to work in SeaMonkey.  

There being no SeaMonkey version, I downloaded the Firefox version (1.1.3) of the extension, converted it via the Extension Converter for SeaMonkey at <http://addonconverter.fotokraina.com/>, and installed the converted result.  All conversion options were selected.  In the installed extension, I set the "Expire visits older than these days" to 30, which had been my setting before the capability was removed from Toolkit/Places.  

At 11:19am Pacific time on 7 February 2015, I still see history entries for 4:29pm on 6 January 2015, 31.8 days ago.  

Note that the Web page for the Expire History by Days contains several review comments (none by me) that the capability should never have been removed from Toolkit/Places.

Comment 2

3 years ago
I agree that this seems like core functionality that should have never been removed in the first place. Even if it were just power users who were requesting, it seems a bit silly to not even provide an about:config entry such as the deprecated browser.history_expire_days option. 

Where did the original conversation about days vs. pages happen? I'd love to determine the reasoning for removal, because I am currently at a loss.

Comment 3

3 years ago
> Where did the original conversation about days vs. pages happen?
Do you know the bug number where that option was removed.
Flags: needinfo?(david)
(Reporter)

Comment 4

3 years ago
Some discussion was at <http://blog.bonardo.net/2010/01/20/places-got-async-expiration>, which cites bug #520165.  In the latter, ALL discussion was by the developer until the patches to implement were in review; and all of the non-developer comments were by reviewers.  Note that the Subject of that bug -- "Make Places expiration async" -- fails to indicate the removal of the capability to expire history by age.  I seem to recall there were many comments (possibly more than 100) on the blog, mostly opposing the change; all such comments have disappeared.  

There is also bug #516940.  Again, the only comments were by the developer until the patches to implement were in review.  

There is also a Mozilla Wiki at <https://wiki.mozilla.org/Firefox/Projects/Places_async_expiration>.  However, that has no discussion.
Flags: needinfo?(david)

Comment 5

3 years ago
Interestingly form history expiration is done by days:

> "Why do form history expiration at all? Three main reasons: privacy, relevance, and performance. 
> Privacy and relevance are fairly obvious — retaining data forever isn’t good practice, and data 
> unused for 6 months is highly unlikely to be relevant in the future. "

https://blog.mozilla.org/dolske/2009/05/13/happy-form-history-expiration-day/
(Reporter)

Comment 6

3 years ago
Version 1.2.0 of the extension Expire History by Days does seem to work with SeaMonkey BUT only after conversion for SeaMonkey compatibility at <http://addonconverter.fotokraina.com/>.  

I consider extensions to be only temporary workarounds, however, as they require updates to compensate for changes in the underlying "vanilla" browser code.  In this case, there is no assurance that conversion of Expire History by Days by <http://addonconverter.fotokraina.com/> will remain possible for future changes either to the extension itself or to the underlying browser code.

Comment 7

3 years ago
The extension's statistics page is not public so I resorted to archive.org to get usage numbers from 2013-2016, it shows steady growth with no sign of slowing:

https://i.imgur.com/3w2kaIN.png

The reliability of the extension should also be considered, v1.1.2 released 2015-12-29 was broken and wasn't fixed until v1.2.0 2016-02-06 for a total of 40 days not working. Someone reported it wasn't working on the first day it broke, most people may not have noticed though, I didn't notice until I had a month's worth of history when I should have only had a few days. Bringing it back into official code would give it the benefit of code review, automated tests and alpha/beta testing making it more reliable.

Updated

3 years ago
Mentor: philip.chee
Whiteboard: [good first bug][gfb][lang=js]

Comment 8

2 years ago
Hello! 

I'm new to open source contribution and I came across this as a good first bug to fix (restore original functionality :). Could this bug be assigned to me so I can fix it?

I also see that Philip Chee is assigned as the Mentor for this bug - would you be available to help me with this: get situated with the process and workflow for Mozilla bugfixing, provide insight and guidance, etc? 

Thanks so much!
Flags: needinfo?(philip.chee)
(Reporter)

Comment 9

2 years ago
See bug #1261313.  With implementation of that in the Mozilla Toolkit, I believe the "Expire history by days" extension will no longer work.  That makes this RFE even more important.
Not my first bug but looks hard enough.  Please assign it to me.
Jacob,

just start and add a patch to it. If you need help setting up a build environment for SeaMonkey just contact me directly.
Hi Frank,

If nobody is working on this bug, I would like to start on this. But I'm a newbie so would need help in setting up SeaMonkey and fixing the bug. Please let me know your thoughts.

Regards,
Ganesh
I was working on it a little but I couldn't build SeaMonkey.  Something rust related..  anyway you can take it over Ganesh.  Good luck.
Ganesh,

I can help you set up a build environment. Not sure how much I a able to help with the bug. It would probably need an api implemented in js.

Just send me an email. 

jakehm: Did you try to build 2.49 without rust. This should work. I still wonder why you Linux comm-central setup didn't work.

Comment 15

a year ago
Created attachment 8868538 [details] [diff] [review]
Provide Capability to Expire History Entries by Age

Comment 16

a year ago
Created attachment 8868543 [details] [diff] [review]
1246424.1.patch
Attachment #8868543 - Flags: review+

Comment 17

a year ago
Created attachment 8868544 [details] [diff] [review]
Provide Capability to Expire History Entries by Age

Updated

a year ago
Attachment #8868538 - Attachment is obsolete: true
Comment on attachment 8868543 [details] [diff] [review]
1246424.1.patch

You might not want to set review+ :) 

Do you want it reviewed? set ? and iann_bugzilla as reviewer. In this smaller case I can probably review it too.
Attachment #8868543 - Flags: review+
QA Contact: m.kobernyk
Assignee: nobody → m.kobernyk
Status: NEW → ASSIGNED
QA Contact: m.kobernyk

Comment 19

a year ago
Sorry for some mess with patches -- [attachment 8868538 [details] [diff] [review]] and [attachment 8868543 [details] [diff] [review]] are the same and appointed for `/comm-central`, [attachment 8868544 [details] [diff] [review]] is for `/comm-central/mozilla`.

It's not a ready-to-merge patch yet, but more a question on how to implement needed feature in more elegant way. Now it implements:
* GUI to set new parameter;
* old visits removing is completing by IDLE;
* visits are removed in 10 seconds after last pref change; any next pref change is going to reset timer (not implemented in present patch);
* current expiration timer (once in 3*60 secs) and application start fires expiration by days too.

What concerns me in this patch:
* if fires only on 3 types of events;
* it uses IDLE, so may cause some performance issues;
* it seems a foreigner in current `nsPlacesExpiration.js` implementation.

Frank-Rainer, any ideas how could it be realized in more elegant way?
Flags: needinfo?(frgrahl)
Comment on attachment 8868544 [details] [diff] [review]
Provide Capability to Expire History Entries by Age

You need to figure this out without touching mozilla stuff. It was removed from there and just assume it won't come back.
Flags: needinfo?(frgrahl)

Comment 21

a year ago
Created attachment 8873070 [details] [diff] [review]
1246424.patch

Now it doesn't change mozilla stuff, but Suite's PlacesController. Still not sure about productivity (it will load IDLE), activation and checking time and codestyle. Please, review.

Will be glad for links to mustreads about events system in Suite, codestyle approaches and XPCOM implementation in JS-modules.
Attachment #8868543 - Attachment is obsolete: true
Attachment #8868544 - Attachment is obsolete: true
Attachment #8873070 - Flags: review?(iann_bugzilla)
Attachment #8873070 - Flags: review?(frgrahl)
Created attachment 8874459 [details]
Capture.PNG

Wrong history display.
Comment on attachment 8873070 [details] [diff] [review]
1246424.patch

In conteroller.js you query the wrong pref:

>   Services.prefs.getBoolPref("browser.zoom.siteSpecific");

This results in an immediate loss of history during startup. Not what you want :) 

This should do it:

>   Services.prefs.getIntPref("places.history.expiration.days");

With it I encountered a few problems problems:

1.) Constant freezes during a cleanup. The actual expire code needs to be made async and at least the call moved out of controller.js. I would probably call it during startup at the end of profile-after-change and after a pref change.

2.) entered value can be too great. Limit it to 4 digits.

3.) Regardless of which value I set the history was cleared up to yesterday. We might have a problem with our history or the call doesn't work. Maybe related to bug 1347652. See capture.png. An entry from today is shown in "Older than 6 months". Probably this needs to be fixed first if it is not just a display problem.

I am no longer sure that is really a "good firt bug" but would be great if you can fix it. r- for now.
Flags: needinfo?(philip.chee)
Attachment #8873070 - Flags: review?(iann_bugzilla)
Attachment #8873070 - Flags: review?(frgrahl)
Attachment #8873070 - Flags: review-
Assignee: m.kobernyk → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.