[places] history sidebar, delete (when grouped by date, or date and site) doesn't work properly spun off from bug #359462, which is just about the command enabling issues with the history sidebar. details coming...

there's a partial patch for this in bug #359462, but it needs work. to elaborate, there are two issues: 1) top level delete by date doesn't work 2) deleting a site (when grouped by day and site) or specific url (when grouped by date or date and site) can delete other references (from other days). as pointed out by mconnor, ff 2.0 has the following issue: if you delete a url from the sidebar when grouped by date, it will delete it from all days (a refresh might be necessary to see that it has been removed.) for the delete by date code, dmills points out that I could generalize and re-use the nsNavHistoryExpire.cpp code, instead of rewriting the code.

things to do: 1) handle "end of epoch" delete by date. For last "bucket" (older than 6 days), we should be passing in 0 for endTime, and then adjusting the SQL query to just be visits greater than beginTime. 2) figure out why after delete the history sidebar does not update / invalidate properly. 3) move common / duplicated code from nsNavHistory.cpp into nsNavHistoryExpire.cpp. possibly move nsNavHistory::RemovePages() and nsNavHistory::RemovePagesFromHost(), and rely on nsNavHistoryExpire to do all the work. Already, nsNavHistory::RemovePages() and nsNavHistory::RemovePagesFromHost() have todo items for erasing annotations. may need the same for erasing favicons. 4) remove my dump statements in controller.js spin off bugs to worry about 1) implement nsNavHistoryExpire::EraseAnnotations() // FIXME bug 319455 expire annotations

> 2) figure out why after delete the history sidebar does not update / > invalidate properly. discussed this with dietrich and dan yesterday. After doing some debugging (and after working with mano on bug #366136), found the problem: // nsNavHistoryResult::OnPageExpired (nsINavHistoryObserver) // // Don't do anything when pages expire. Perhaps we want to find the item // to delete it. NS_IMETHODIMP nsNavHistoryResult::OnPageExpired(nsIURI* aURI, PRTime aVisitTime, PRBool aWholeEntry) { return NS_OK; }

actually, we might want to do this work in EraseHistory(), but only for pages without visits. for pages with visits, we will something else. continuing to investigate.

last visited also doesn't work properly, because we are deleting by uri, and not expiring a visit. when in "view by last visited" you can duplicate uris, so deleting one can cause several to disappear. dietrich and I come across this, and some other weirdness (which I'll spin off to another bug) last week.

clearing tm for now, but I'd like to say a5 or a6.

> when in "view by last visited" you can duplicate uris, so deleting one can cause several to disappear. see bug #382026 > top level delete by date doesn't work see bug #376706 > deleting a site (when grouped by day and site) or specific url (when grouped by date or date and site) can delete other references (from other days). see bug #384674 ugh.

(In reply to comment #9) > > when in "view by last visited" you can duplicate uris, so deleting one can > cause several to disappear. > > see bug #382026 which was WONTFIXed and refers to bug #385245 (non-obviously (for this issue) summarized as "history sidebar very slow (way slower than fx2)") as the open bug to watch. Is there now a better one? There seem to be several similar scattered about. Since: there are two very different deletion behaviors depending upon the (currently obscured from the user) underlying URLs, and the user's expectation may depend entirely upon which behavior he happened upon first, and there's no undelete in the history sidebar yet (hence data lost), and it doesn't seem unreasonable to expect to be able to keep recent history visible in the sidebar, yet be able to un-clutter it with predictable effect, and I'm impatient, and so on, I wish this were already fixed. Thanks for listening :).

I guess this also covers the case when you try to delete all items from 4 days ago, for instance?

pushing to M10.

actual behaviour: - BY LAST VISITED deleting one visit will delete all visits to that url (expected behaviour per Bug 382026) working fine - BY MOST VISITED deleting one visit will delete all visits to that url (expected behaviour) working fine - BY DATE deleting one visit will delete all visits to that url also in other days (expected behaviour per Bug 384674?) deleting the day folder does not work and should be fixed - BY SITE works but it's slow - BY DATE AND SITE works on domain (slow as before), does not work on date since deleting a visit is mostly a privacy fact, imho we should continue on the line of deleting all visits for an url when deleting an url, in all sorted views (also time-driven ones), as stated by bug 382026 and bug 384674 so only remaining issues should be: - delete a day folder should work (Bug 376706): catch all urls into that timeframe and delete ALL visits for them - delete by host is slow: try moving more work into subqueries if that's ok i'm starting looking at this.

fyi: bug 385245 may change how the query folders in the history sidebar are constructed, thereby changing how we need to do deletes from these views.

yes, but i think that we could get the first and last item time into the container and delete between them, so it should not be affected still have to consider when grouped by date and site we also have problems with annos queries into RemovePagesFromHost, some query is totally wrong (argh!) like i pointed out in IRC. will patch that similar to RemovePages, with a single onUpdateBatch call

splitted out removePagesByHost into bug 415757

i'm changing this into a sort of meta bug to track other sidebar delete bugs

With my patch for 385245 deleting by site does not work - I will fix this in the PlacesUtils.nodeIsHost and submit new patch. It may still be little dangerous to work on both bugs in parallel.

ondrej please note Bug 415757 that is probably landing soon

As a reminder: after landing of Bug 376706 we should ensure that time is always setup in day containers, as the max(visit_date) of items that it contains (particularly in Ondrej's patch for Bug 385245)

(In reply to comment #23) > As a reminder: after landing of Bug 376706 we should ensure that time is always > setup in day containers, as the max(visit_date) of items that it contains > (particularly in Ondrej's patch for Bug 385245) > This would not be that easy, because I do not know the time unless the day is expanded. With "Date and Site" I do not know it unless I would expand all sites. However, I can set the time to the endTime of the query which represents the day. This should be very much the same for your needs.

yes, that should be the same, i delete between the previous container time (excluded) and the actual container time (included)

I observe a related behavior on trunk builds (osx 10.4, WinXP). After a browsing session I will selectively delete SOME history entries via the history window/sidebar. They disappear from the history view but the location bar autocomplete will still present some of them when matching what I type even after browser restarts. This is the closest existing bug I found, so I post here.

Jake: are those entries starred/bookmarked?

(In reply to comment #27) > Jake: are those entries starred/bookmarked? > No, they are not. Sorry, should have made that clear. Am still investigating to make reproducible.

have you imported bookmarks from a firefox 2 profile?

(In reply to comment #29) > have you imported bookmarks from a firefox 2 profile? > No. Brand new profile, created with the build in question. I first noticed this bug some weeks ago in nightlies, possibly correlated but not necessarily linked with work on the awesomebar. I know nothing of the inner workings, do the two have separate storage systems? Installed extensions: AdblockPlus, NoScript. I have discovered that NoScript sometimes screws with FF3.0x UI functions such as preventing typing in the locationbar or blocking JS bookmarklets. Not sure if this is relevant here though.

are those items still there if you do a clear private data and clear browsing history?

(In reply to comment #31) > are those items still there if you do a clear private data and clear browsing > history? > That removes all history and autocomplete items as expected.

please, install Sqlite Manager extension (https://addons.mozilla.org/en-US/firefox/addon/5817). The next time that you see a dangling item into the autocomplete open Sqlite Manager and execute this SQL query: SELECT h.url, b.id, a.expiration, n.name FROM moz_places h LEFT JOIN moz_historyvisits v ON h.id = v.place_id LEFT JOIN moz_annos a ON h.id = a.place_id LEFT JOIN moz_anno_attributes n ON n.id = a.anno_attribute_id LEFT JOIN moz_bookmarks b ON h.id = b.fk WHERE (b.id IS NOT NULL OR a.id IS NOT NULL) AND SUBSTR(url, 0, 6) != 'place:' identify the dangling url in the table, and paste here the values for id, expiration and name columns. thank you.

Please investigate the difference between location bar "typed in" visits compared to linked to visits. My guess is the ones you're seeing in the location bar after sidebar history deletion are actually "typed in location bar" visits. It looks to me like history delete from the sidebar is not removing the entry from location bar typed in visits like Clear Private Data > Browsing History does.

delete in the sidebar really should delete ALL visits for items, and also delete pages that are not bookmarked/starred/annotated. All other pages should be deleted, typed urls should not be a problem until they are not falling into previous categories.

I have filed <a href=https://bugzilla.mozilla.org/show_bug.cgi?id=417298">Bug 417298</a> as this has moved in a slightly different direction. I have found out a few more things that are documented there.

actually i can't see other delete problems in the sidebar, but i'd prefer mantaining this open until Ondrej's patches for the sidebar land, so we can add dependant bugs if needed.

well, ondrej's patch for sidebar has landed, delete is working fine and it's quite fast too (apart in most visited and last visited but that is mostly due to huge treeview perf problems). we can consider this fixed.

almost reopened due to case 2 in comment #1 'til I noticed the later reference to bug 363621. Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko/2008022704 Minefield/3.0b4pre

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