1526304 - Clear History - Clear now gives: Error sanitizing history: TypeError: PlacesUtils.history.removeVisitsByTimeframe is not a function

Status: RESOLVED FIXED

(Thunderbird :: Preferences, defect)

Description

[Jorg K (CEST = GMT+2)]

I can't see in a hurry were this was removed. Looks like it was in nsIBrowserHistory.idl:
https://dxr.mozilla.org/mozilla-esr60/source/toolkit/components/places/nsIBrowserHistory.idl#46

Mark and Marco, can you please give us a tip.

Comment 1

[Mark Banner (:standard8)]

That would be bug 1089691 where they were removed and replaced by the new async history API.

Comment 2

[Marco Bonardo [:mak]]

bug 1261313?

You can now use methods exposed by History.jsm, accessible through PlacesUtils.history. In the removeVisitsByTimeframe case you can now use PlacesUtils.history.removeVisitsByFilter (to remove visits) or PlacesUtils.history.removeByFilter (to remove whole pages will all of their visits)

Comment 3

[Jorg K (CEST = GMT+2)]

Thanks, Mark & Marco, that will give us a good start.

Comment 4

[Jorg K (CEST = GMT+2)]

Oops, that one fell through the cracks.

Comment 5

[Benjamin Flanagin]

Attachment: bug-1526304-fix.patch

Replaced ...removeVisitsByTimeFrame with ...removeVisitsByFilter, and set the filter keywords to match the desired effect.

Tested the build and there was no errors given when I cleared my history, and it seems to have removed the data properly though I had little data to remove.

Comment 6

[Jorg K (CEST = GMT+2)]

Comment on attachment 9068776 [details] [diff] [review]
bug-1526304-fix.patch

Looks OK to me apart from the indentation. Let's not forget this for the coming beta.

P.S.: You don't need to add f+, you're the author so we assume that you think that it works for you.

Comment 7

[Benjamin Flanagin]

I based the indentations on what I found in Fire Fox's code when it referenced the function.
and I'll try to remember to leave off the f+

Comment 8

[Benjamin Flanagin]

Attachment: bug-1526304-fix.patch

Added [Author <email address>] to Commit section of patch, and reworded the title to match what the patch does.

Is she ready for check-in?

Comment 9

[Jorg K (CEST = GMT+2)]

Attachment: bug-1526304-fix.patch

I'm attaching a corrected patch to avoid further iterations. There were a few issues:

1. Never use tabs, ever. We use indentation of two spaces, apart from Calendar code which uses four. So clearly the indentation was wrong.
2. In the HG header, you need to provide the user/author: #User Benjamin Flanagin <[e-mail]>
3. In the commit message, try to say in one line what you did.

Comment 10

[Jorg K (CEST = GMT+2)]

Attachment: bug-1526304-fix.patch

And you should also run the linter to check for linting issues.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6e8451e1c4f9
Port bug 1089691: Use PlacesUtils.history.removeVisitsByFilter() when clearing history. r=jorgk

Comment 12

[Jorg K (CEST = GMT+2)]

TB 68 beta:
https://hg.mozilla.org/releases/comm-beta/rev/ef8268c9d3448b1bbad95943960472bbcf0299f4