Last Comment Bug 501357 - New history clearing features do not send onDeleteURI or onClearHistory Places events
: New history clearing features do not send onDeleteURI or onClearHistory Place...
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: 3.5 Branch
: All All
: -- normal with 1 vote (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on: 630240
Blocks:
  Show dependency treegraph
 
Reported: 2009-06-30 07:01 PDT by Jason Sonnenschein
Modified: 2011-05-02 12:25 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
?


Attachments

Description Jason Sonnenschein 2009-06-30 07:01:43 PDT
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090625 Shiretoko/3.5pre Beetle
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090625 Shiretoko/3.5pre Beetle

When the user removes a single website visit from the history, places sends an onDeleteURI event to all registered listeners.  onClearHistory is sent when the entire history is cleared.  Neither of these events is sent when the user chooses "forget this site" in the history sidebar or "clear recent history" for an hour.

Reproducible: Always

Steps to Reproduce:
1. Visit some web sites
2. Right click an entry in history sidebar and click "forget this site" OR
3. In tools menu, select "clear recent history" and choose the last hour as the time period
Actual Results:  
No clear history or delete URI events were sent to an extension that had registered listeners for these

Expected Results:  
Expected to receive a "delete URI" event for both
Comment 1 Shawn Wilsher :sdwilsh 2009-06-30 09:32:04 PDT
There are a number of API's that we have that don't actually notify observers sadly.
Comment 2 Jason Sonnenschein 2009-06-30 11:21:00 PDT
Well... I'm still trying to hunt down the code for the "forget this site" code, but it's odd that "clear recent history" isn't working.  It looks like it _should_ send the same signal if clearing all history or just an hour, but I've doublechecked and no "clear history" signal is received by observers

from browser/content/browser/sanitize.js:

        if (this.range)
          globalHistory.QueryInterface(Components.interfaces.nsIBrowserHistory_MOZILLA_1_9_1_ADDITIONS).removeVisitsByTimeframe(this.range[0], this.range[1]);
        else
          globalHistory.removeAllPages();

        try {
          var os = Components.classes["@mozilla.org/observer-service;1"]
                             .getService(Components.interfaces.nsIObserverService);
          os.notifyObservers(null, "browser:purge-session-history", "");
        }
        catch (e) { }
Comment 3 Jason Sonnenschein 2009-06-30 11:58:14 PDT
It looks like "forget this host" needs the code added near the 

// History comment inside the function

removeDataFromDomain

in browser/components/privatebrowsing/src/nsPrivateBrowsingService.js
Comment 4 Drew Willcoxon :adw 2009-06-30 16:44:46 PDT
The new clear recent history by timeframe feature is backed by nsIBrowserHistory.removeVisitsByTimeframe() [1], implememented at [2], which as Shawn points out doesn't notify for each deleted visit or URI, only on begin/end batch.  When you remove all history using the clear recent history window, 3.5 acts the same as 3.0's clear private data window and calls nsIBrowserHistory.removeAllPages(), which does notify nsINavHistoryObservers with onClearHistory() [3].

Ideally we should be notifying nsINavHistoryObservers in the timeframe case too, but we're not.  Maybe there should be a new method onClearHistoryTimeframe()?

[1] http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/public/nsIBrowserHistory.idl#114
[2] http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/src/nsNavHistory.cpp#4713
[3] http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/src/nsNavHistoryExpire.cpp#282
Comment 5 Jason Sonnenschein 2009-08-03 19:42:43 PDT
I maintain an extension that replaces the history menu, and would rely on these two signals to know to redraw.

Can anyone help me out to find a workaround until signals are added?  How does the firefox history menu know to redraw because a site was forgotten or an hour of history was cleared?  Or would someone describe how to overload the javascript functions that the firefox gui triggers to forget timeframe or to forget a website (in the history sidebar)?  

Any help would be much appreciated.
Comment 6 Jason Sonnenschein 2009-08-03 20:03:55 PDT
Nevermind found my own answer... just looked through the C source code and it looks like both of these bulk delete functions send a Begin/EndUpdateBatch.
Comment 7 Jason Sonnenschein 2009-08-04 05:08:00 PDT
Just from one developer's point of view... the code already present that gives Begin/EndUpdateBatch signals is sufficient for me, and I'd be fine with the ticket closing if https://developer.mozilla.org/en/nsINavHistoryObserver gets updated to detail that the new deletion mechanisms use the bulkupdate signals.  I'll submit a change request for that documentation.  Other folks, are observer signal changes still necessary here?
Comment 8 Gervase Markham [:gerv] 2009-11-26 06:44:23 PST
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
Comment 9 Marco Bonardo [::mak] 2011-05-02 12:25:07 PDT
Should be fixed by bug 630240, the only case where we don't knotify pages removal is onClearHistory, in this case anything that is not a bookmark is removed, so notifying has a low value.

Note You need to log in before you can comment on or make changes to this bug.