splitted from Bug 363621 RemovePagesFromHost is doing work in reverse mode, it is selecting places with annotation set to expire never, not deleting all visits, and removing annotations for not selected places... some strange kind of things happens there, we should revert it to the correct ordered work: - find places within host range - delete all visits for those places - expire annotation - delete dangling items in moz_places - fix frecency - use a general batch observer call (for perf, since we are deleting a bunch of urls)

this fixes the "wrong way" behaviour onDeleteURI single calls have been removed for perf, since this is deleting urls in bunch we should only fire a Begin/EndUpdateBatch

Comment on attachment 301483 [details] [diff] [review] patch thanks. please add a test for this (or fix the current ones, if there are any).

added a full dedicated test, FAIL with old function, PASS with new function the test adds a bunch of visits, then checks that pages have been removed correctly and that annotations were not deleted by error

- created a new internal method RemovePagesInternal - RemovePage, RemovePages, RemovePagesFromHost will now end up into that method (RemovePage calls RemovePages), good for mantainability and code size win

too much code will make you forgot to include headers :\

Comment on attachment 301713 [details] [diff] [review] patch + test >+// nsNavHistory::RemovePagesInternal > // >-// Removes a bunch of uris from history. >-// Has better performance than RemovePage when deleting a lot of history. >-// Notice that this function does not call the onDeleteURI observers, >-// instead, if aDoBatchNotify is true, we call OnBegin/EndUpdateBatch. >-// We don't do duplicates removal, URIs array should be cleaned-up before. >+// Deletes a list of placeIds from history. >+// This is an internal method used by RemovePages and RemovePagesFromHost. >+// Takes a comma separated list of place ids. Hrm, it's irksome that it takes a delimited string instead of an array. But that'd add yet another iteration over the set, so not worth it really. Please add a comment that this method does not do notification. r=me otherwise

added comment. Ready for check-in

Comment on attachment 301867 [details] [diff] [review] for check-in Drivers: this is an important fix, actual code can cause dataloss of annotations and privacy concern since some url is not really removed from history.

Comment on attachment 301867 [details] [diff] [review] for check-in thnx Marco!

Checking in toolkit/components/places/public/nsIBrowserHistory.idl; /cvsroot/mozilla/toolkit/components/places/public/nsIBrowserHistory.idl,v <-- nsIBrowserHistory.idl new revision: 1.12; previous revision: 1.11 done Checking in toolkit/components/places/src/nsNavHistory.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v <-- nsNavHistory.cpp new revision: 1.245; previous revision: 1.244 done Checking in toolkit/components/places/src/nsNavHistory.h; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.h,v <-- nsNavHistory.h new revision: 1.134; previous revision: 1.133 done RCS file: /cvsroot/mozilla/toolkit/components/places/tests/unit/test_415757.js,v done Checking in toolkit/components/places/tests/unit/test_415757.js; /cvsroot/mozilla/toolkit/components/places/tests/unit/test_415757.js,v <-- test_415757.js initial revision: 1.1 done

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