Dietrich Ayala (:dietrich) Reporter
	Description • 17 years ago

see the bug URL for mockups and notes. should display the list of tags for the item, ellipsis-ized for long lists. tooltip could display full list?

	Christine Yen Assignee
	Comment 1 • 17 years ago

Using Mano's tagging back-end service, just displaying the tags as a node property... unless we want to embed an editor element into the tree.

Also depending on the patch for bug 387746, the finder pane UI bug

	(not reading, please use seth@sspitzer.org instead)
	Comment 2 • 17 years ago

> unless we want to embed an editor element into the tree.

from http://developer.mozilla.org/en/docs/Firefox_3_for_developers#Tree_Improvements

trees now support editing of individual cells. Double-clicking on an editable cell will display a text field in which the user can edit the contents of the cell. 

see bug #201499 and http://wiki.mozilla.org/XUL:Tree

	Asaf Romano (gone)
	Comment 3 • 17 years ago

yeah, well, text-field != editor, I don't think this can be done without significant back-end changes to nsTree*.
 

	Christine Yen Assignee
	Comment 4 • 17 years ago

Managed to leave off the actually adding-the-column-to-XUL bits last time, pretty ready to plug in once there is the ability to add tags via the UI...

At this point only read-only, unless we desperately want the editor element in there (despite Mano's comment 3 above). Also depends on the finder pane (bug 387746) to deal with setting columns.

	Asaf Romano (gone)
	Comment 5 • 17 years ago

Comment on attachment 273669 [details] [diff] [review]
reads out tag back-end

per IRC.

	Asaf Romano (gone)
	Comment 7 • 17 years ago

Comment on attachment 273855 [details] [diff] [review]
patch

>Index: treeView.js
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/places/content/treeView.js,v
>retrieving revision 1.13
>diff -u -8 -p -r1.13 treeView.js
>--- treeView.js	11 Jul 2007 09:58:00 -0000	1.13
>+++ treeView.js	25 Jul 2007 21:16:02 -0000
>@@ -420,16 +420,17 @@ PlacesTreeView.prototype = {
>   COLUMN_TYPE_TITLE: 1,
>   COLUMN_TYPE_URI: 2,
>   COLUMN_TYPE_DATE: 3,
>   COLUMN_TYPE_VISITCOUNT: 4,
>   COLUMN_TYPE_KEYWORD: 5,
>   COLUMN_TYPE_DESCRIPTION: 6,
>   COLUMN_TYPE_DATEADDED: 7,
>   COLUMN_TYPE_LASTMODIFIED: 8,
>+  COLUMN_TYPE_TAGS: 9,
> 
>   _getColumnType: function PTV__getColumnType(aColumn) {
>     switch (aColumn.id) {
>       case "title":
>         return this.COLUMN_TYPE_TITLE;
>       case "url":
>         return this.COLUMN_TYPE_URI;
>       case "date":
>@@ -439,16 +440,18 @@ PlacesTreeView.prototype = {
>       case "keyword":
>         return this.COLUMN_TYPE_KEYWORD;
>       case "description":
>         return this.COLUMN_TYPE_DESCRIPTION;
>       case "dateAdded":
>         return this.COLUMN_TYPE_DATEADDED;
>       case "lastModified":
>         return this.COLUMN_TYPE_LASTMODIFIED;
>+      case "tags":
>+        return this.COLUMN_TYPE_TAGS;
>     }
>     return this.COLUMN_TYPE_UNKNOWN;
>   },
> 
>   _sortTypeToColumnType: function PTV__sortTypeToColumnType(aSortType) {
>     switch(aSortType) {
>       case Ci.nsINavHistoryQueryOptions.SORT_BY_TITLE_ASCENDING:
>         return [this.COLUMN_TYPE_TITLE, false];
>@@ -1013,16 +1016,24 @@ PlacesTreeView.prototype = {
>       case this.COLUMN_TYPE_TITLE:
>         // normally, this is just the title, but we don't want empty items in
>         // the tree view so return a special string if the title is empty.
>         // Do it here so that callers can still get at the 0 length title
>         // if they go through the "result" API.
>         if (PlacesUtils.nodeIsSeparator(node))
>           return "";
>         return node.title || PlacesUtils.getString("noTitle");
>+      case this.COLUMN_TYPE_TAGS:
>+        if (PlacesUtils.nodeIsURI(node)) {
>+          var tagsvc = PlacesUtils.tagging;
>+          var uri = PlacesUtils._uri(node.uri);
>+          var tags = tagsvc.getTagsForURI(uri, {});
>+          return tags.join(", ");
>+        }
>+        return "";
>       case this.COLUMN_TYPE_URI:
>         if (PlacesUtils.nodeIsURI(node))
>           return node.uri;
>         return "";
>       case this.COLUMN_TYPE_DATE:
>         if (node.time == 0 || !PlacesUtils.nodeIsURI(node)) {
>           // hosts and days shouldn't have a value for the date column.
>           // Actually, you could argue this point, but looking at the
>@@ -1097,16 +1108,20 @@ PlacesTreeView.prototype = {
>     }
> 
>     node.containerOpen = !node.containerOpen;
>   },
> 
>   cycleHeader: function PTV_cycleHeader(aColumn) {
>     if (!this._result)
>       throw Cr.NS_ERROR_UNEXPECTED;
>+    

nit: trailing spaces

>+    // Currently - don't want to allow sorting by tags

eh, cannot ;)

r=mano otherwise, this should land only once the add-bookmarks does though.

	Alex Faaborg [:faaborg] (Firefox UX)
	Comment 8 • 17 years ago

Here is the latest iteration of the Places Organizer:

http://people.mozilla.com/~faaborg/files/granParadisoUI/placesOrganizer_i5WindowLayout.png

	Dietrich Ayala (:dietrich) Reporter
	Comment 9 • 17 years ago

Checking in browser/components/places/content/places.xul;
/cvsroot/mozilla/browser/components/places/content/places.xul,v  <--  places.xul
new revision: 1.78; previous revision: 1.77
done
Checking in browser/components/places/content/treeView.js;
/cvsroot/mozilla/browser/components/places/content/treeView.js,v  <--  treeView.js
new revision: 1.15; previous revision: 1.14
done
Checking in browser/locales/en-US/chrome/browser/places/places.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/places/places.dtd,v  <--  places.dtd
new revision: 1.27; previous revision: 1.26
done

	Peter van der Woude [:Peter6]
	Comment 11 • 17 years ago

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007091322 Minefield/3.0a8pre ID:2007091322

VERIFIED

	Tracy Walker [:tracy]
	Comment 12 • 17 years ago

verified with - Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a8pre) Gecko/2007091304 Minefield/3.0a8pre

	Gervase Markham [:gerv]
	Comment 13 • 15 years ago

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