465082 - Add "Forget About This Site" to History sidebar's site container context menu

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, enhancement, P3)

Description

[u60234]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081114 Minefield/3.1b2pre

When the History sidebar is set to group by "By Site" or "By Date and Site", the context menu for a site container should have the "Forget About This Site" item. It is a bit unintuitive to have to expand the site container and bring up the context menu for an individual page to delete the complete history for the site.

Steps to Reproduce:
1.Open the History sidebar.
2.Click the sidebar's "View" button and select "By Site".
3.Open the context menu for a site container.

Actual Results:  
There is no way to delete all history (cookies, cache, downloads, permissions, etc.) for the site. There is only a "Delete" command that AFAIK only removes the site from the visited site's list.

Expected Results:  
There should be a "Forget About This Site" command that delete all history related to the site.

Comment 1

[Marco Bonardo [:mak]]

you can delete the host container that should make exactly what you're asking for...

Comment 2

[Marco Bonardo [:mak]]

oh sorry, that will only delete history, not cookies and so on

Comment 3

[Mike Connor [:mconnor]]

Would be nice to get early in 3.2.

Comment 4

[Michael Kohler [:mkohler]]

Attachment: Patch v1

Comment 5

[Marco Bonardo [:mak]]

Comment on attachment 375589 [details] [diff] [review]
Patch v1

>diff --git a/browser/components/places/content/controller.js b/browser/components/places/content/controller.js
>--- a/browser/components/places/content/controller.js
>+++ b/browser/components/places/content/controller.js
>@@ -232,19 +232,27 @@ PlacesController.prototype = {
>       this.copy();
>       break;
>     case "cmd_paste":
>       this.paste();
>       break;
>     case "cmd_delete":
>       this.remove("Remove Selection");
>       break;
>-    case "placesCmd_deleteDataHost":
>-      let uri = PlacesUtils._uri(this._view.selectedNode.uri);
>-      PlacesUIUtils.privateBrowsing.removeDataFromDomain(uri.host);
>+    case "placesCmd_deleteDataHost":     

nit: trailing spaces here

>+      if(this._view.selectedNode.uri.match(/place/)) { 

you should use PlacesUtils.nodeIsHost

>+        var uriSubstring = "" + this._view.selectedNode.uri.match(/domain=.+&/);
>+        var uriComplete = "http://" + uriSubstring.slice(uriSubstring.indexOf("=")+1, uriSubstring.indexOf("&"));
>+        let uri = PlacesUtils._uri(uriComplete);
>+        PlacesUIUtils.privateBrowsing.removeDataFromDomain(uri.host);

Every query node has a getQueries method, so you should get the first query in the queries array, and use query.domain

>+      }
>+      else {
>+        let uri = PlacesUtils._uri(this._view.selectedNode.uri);
>+        PlacesUIUtils.privateBrowsing.removeDataFromDomain(uri.host);
>+      }

get an host var, and then call removeDataFromDomain(host) out of the if/else

Comment 6

[Michael Kohler [:mkohler]]

Attachment: includes the changes from comment 5

includes changes from comment 5.

Comment 7

[Marco Bonardo [:mak]]

Comment on attachment 376048 [details] [diff] [review]
includes the changes from comment 5

>diff --git a/browser/components/places/content/controller.js b/browser/components/places/content/controller.js
>--- a/browser/components/places/content/controller.js
>+++ b/browser/components/places/content/controller.js
>@@ -233,18 +233,24 @@ PlacesController.prototype = {
>       break;
>     case "cmd_paste":
>       this.paste();
>       break;
>     case "cmd_delete":
>       this.remove("Remove Selection");
>       break;
>     case "placesCmd_deleteDataHost":
>-      let uri = PlacesUtils._uri(this._view.selectedNode.uri);
>-      PlacesUIUtils.privateBrowsing.removeDataFromDomain(uri.host);
>+      var host;
>+      if(PlacesUtils.nodeIsHost(this._view.selectedNode)) {

space between if and (

r=me with those fixed, thanks!

Comment 8

[Michael Kohler [:mkohler]]

Attachment: includes the space after if (checkin-ready)

Added the space after the if.

Comment 9

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/18243f70a33c

can't make 1.9.1 due to l10n changes.

Comment 10

[Marco Bonardo [:mak]]

nvm, this can make 1.9.1

Comment 11

[Marco Bonardo [:mak]]

Comment on attachment 376258 [details] [diff] [review]
includes the space after if (checkin-ready)

this fix is really small and low risk, enables the Forget This site context menu on Site containers.

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 376258 [details] [diff] [review]
includes the space after if (checkin-ready)

a191=beltzner and please add a test or flip the in-litmus? flag

Comment 13

[Marco Bonardo [:mak]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/129dd637477d

Comment 14

[Marcia Knous [:marcia]]

Verified fixed on the trunk using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090527 Minefield/3.6a1pre.

Verified fixed on the 1.9.1 branch using  Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090527 Shiretoko/3.5pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090527 Shiretoko/3.5pre.

Comment 15

[Marcia Knous [:marcia]]

https://litmus.mozilla.org/show_test.cgi?id=7538 is already in Litmus to cover this case so setting the flag.

Comment 16

[Gervase Markham [:gerv]]

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