317833 - Personal Toolbar

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect, P1)

Description

[Ben Goodger (use ben at mozilla dot org for email)]

Bring Personal Toolbar functionality back, plus add a small "Places" button at the start for easy access.

Comment 1

[Annie Sullivan]

Attachment: Fixes open in tabs, fixes open commands in context menu grayed out, work on pasting

Added mousedown to the command updater, so that when the activeView is set on personal toolbar mousedown, the enabled/disabled states of the commands are updated.  This fixes a bug where "Open in..." links were sometimes disabled when they should have been enabled.

Added a check in openLinksInTabs() to make sure that only urls are opened, not folders.

Worked on fixing pasting of folders.  Added the flavour for folders to the nsITransferable object in the paste function, changed the call to _getFolderCopyTransaction() from PlacesController to PlacesControllerDragHelper (where it is declared), and sent data instead of data.folderId as the first parameter.  But the call to bms.getFolderTitle(folderId) throws an NS_ERROR_ILLEGAL_VALUE exception, which means that the database query for the folderId returned no results.  So I'm not sure if these changes help.

Comment 2

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 204569 [details] [diff] [review]
Fixes open in tabs, fixes open commands in context menu grayed out, work on pasting

>   paste: function() {
>     var xferable = 
>         Cc["@mozilla.org/widget/transferable;1"].
>         createInstance(Ci.nsITransferable);
>     xferable.addDataFlavor(TYPE_X_MOZ_PLACE);
>     xferable.addDataFlavor(TYPE_X_MOZ_URL);
>     xferable.addDataFlavor(TYPE_UNICODE);
>+    xferable.addDataFlavor(TYPE_X_MOZ_PLACE_CONTAINER);

Is this the right place to be adding the container type? The order that you add things to the transferable is very important. The item added first generally has precedence. Other places in code add the container first, perhaps it should be added that way here too. 

Otherwise, r=[e-mail]

Comment 3

[Brian Ryner (not reading)]

Comment on attachment 204569 [details] [diff] [review]
Fixes open in tabs, fixes open commands in context menu grayed out, work on pasting

>--- browser/components/places/content/controller.js	29 Nov 2005 17:12:02 -0000	1.24
>+++ browser/components/places/content/controller.js	30 Nov 2005 18:11:27 -0000
>@@ -448,19 +448,23 @@ var PlacesController = {
>    */
>   openLinksInTabs: function PC_openLinksInTabs() {
>     var node = this._activeView.selectedNode;
>     if (this._activeView.hasSingleSelection && this.nodeIsFolder(node)) {
>       var queries = node.getQueries({});
>       var kids = this._hist.executeQueries(queries, queries.length,
>                                            node.queryOptions);
>       var cc = kids.childCount;
>-      for (var i = 0; i < cc; ++i)
>-        this._activeView.browserWindow.openNewTabWith(kids.getChild(i).url, 
>+      for (var i = 0; i < cc; ++i) {
>+        var node = kids.getChild(i);
>+        if (!this.nodeIsURL(node))
>+          continue;
>+        this._activeView.browserWindow.openNewTabWith(node.url, 
>                                                       null, null);
>+      }

I think this would be a little clearer with the |if| inverted:

if (this.nodeIsURL(node)) {
  this._activeView.browserWindow.openNewTabWith(node.url, null, null);
}

Looks good otherwise.

Comment 4

[Ben Goodger (use ben at mozilla dot org for email)]

Annie, I'm going to close this one out. Can you file separate bugs on remaining features that you know need to be implemented? This will make tracking our progress much easier. 

Comment 5

[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