Closed Bug 1526304 Opened 2 years ago Closed 1 year ago

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

Categories

(Thunderbird :: Preferences, defect)

defect
Not set
normal

Tracking

(thunderbird68 fixed, thunderbird69 fixed)

RESOLVED FIXED
Thunderbird 69.0
Tracking Status
thunderbird68 --- fixed
thunderbird69 --- fixed

People

(Reporter: jorgk-bmo, Assigned: benjamin)

Details

(Keywords: regression)

Attachments

(1 file, 3 obsolete files)

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.

Flags: needinfo?(standard8)
Flags: needinfo?(mak77)

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

Flags: needinfo?(standard8)
Flags: needinfo?(mak77)

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)

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

Keywords: regression
Assignee: nobody → benjamin

Oops, that one fell through the cracks.

Attached patch bug-1526304-fix.patch (obsolete) — Splinter Review

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.

Attachment #9068776 - Flags: review?(mkmelin+mozilla)
Attachment #9068776 - Flags: feedback+
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.
Attachment #9068776 - Flags: feedback+ → approval-comm-beta+

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+

Attached patch bug-1526304-fix.patch (obsolete) — Splinter Review

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?

Attachment #9068776 - Attachment is obsolete: true
Attachment #9068776 - Flags: review?(mkmelin+mozilla)
Attached patch bug-1526304-fix.patch (obsolete) — Splinter Review

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 <benjamin@thunderbird.net>
  3. In the commit message, try to say in one line what you did.
Attachment #9068968 - Attachment is obsolete: true
Attachment #9069005 - Flags: review+
Status: NEW → ASSIGNED
Keywords: checkin-needed
Attachment #9069005 - Flags: approval-comm-beta+

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

Attachment #9069005 - Attachment is obsolete: true
Attachment #9069006 - Flags: review+
Attachment #9069006 - Flags: approval-comm-beta+

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

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 69.0
You need to log in before you can comment on or make changes to this bug.