1246424 - Provide Capability to Expire History Entries by Age

Status: NEW

(SeaMonkey :: Bookmarks & History, enhancement)

Description

[David E. Ross]

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.

Comment 1

[David E. Ross]

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

[John Duncan]

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

[Philip Chee]

> Where did the original conversation about days vs. pages happen?
Do you know the bug number where that option was removed.

Comment 4

[David E. Ross]

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.

Comment 5

[Kestrel]

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/

Comment 6

[David E. Ross]

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

[Kestrel]

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.

Comment 8

[Jacques Laroche]

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!

Comment 9

[David E. Ross]

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.

Comment 10

[Jacob Harrow-Mortelliti [:jakehm]]

Not my first bug but looks hard enough.  Please assign it to me.

Comment 11

[Frank-Rainer Grahl (:frg)]

Jacob,

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

Comment 12

[Ganesh Chaitanya Kale]

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

Comment 13

[Jacob Harrow-Mortelliti [:jakehm]]

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.

Comment 14

[Frank-Rainer Grahl (:frg)]

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

[Mykhas Kobernyk]

Attachment: Provide Capability to Expire History Entries by Age

Comment 16

[Mykhas Kobernyk]

Attachment: 1246424.1.patch

Comment 17

[Mykhas Kobernyk]

Attachment: Provide Capability to Expire History Entries by Age

Comment 18

[Frank-Rainer Grahl (:frg)]

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.

Comment 19

[Mykhas Kobernyk]

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?

Comment 20

[Frank-Rainer Grahl (:frg)]

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.

Comment 21

[Mykhas Kobernyk]

Attachment: 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.

Comment 22

[Frank-Rainer Grahl (:frg)]

Attachment: Capture.PNG

Wrong history display.

Comment 23

[Frank-Rainer Grahl (:frg)]

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.

Comment 24

[Dhyey Thakore [:dhyey35]]

Hi Frank, This isn't my first bug but I think it's quite challenging for me too.

I have mozilla-unified setup and ready which I use to build Firefox. Can I build SeaMonkey from that or do I need to clone another repo ( maybe follow https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_SeaMonkey_build )

Comment 25

[Frank-Rainer Grahl (:frg)]

dhyey35 I am not sure it can be done without changing toolkit and you can bet any patches not usable for Fx won't get accepted. The capability was intentionally removed there. But we could put it in a SeaMonkey branch if it really needs to touch mozilla code.

You need to check out comm-esr60 and mozilla-esr60. This is the base for our next release 2.57. SeaMonkey in comm-central is quite broken and unlikely to be fixed soon. We just keep it building for now.

For esr60 check https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Thunderbird_build

You just need to change comm/mail to comm/suite to build SeaMonkey.

You could also use the 2.53 code base with my patches if you need a fully functional SeaMonkey for testing:

http://www.wg9s.com/comm-253/

The patches are based on the release braches at 56 end.

If you are building under Windows I have a VS2017 setup guide available. Just shoot me an email for questions.

Comment 26

[Ian Neal]

Not sure if bug 1526304 comment 2 helps

Comment 27

[Frank-Rainer Grahl (:frg)]

In any case it needs to use the new async api.

On 2.53 it is enabled with browser.places.useAsyncTransactions set to true. I no longer test the old one there too. Already gone from 60/2.57