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

This is iteration 7 of the starring and tagging UI, mostly consisting of some visual tweaks to simplify the interface.  Also, the checkbox next to "place in folder" from i6 has now been removed.

Here are the notes in the mockup:

This dialog is displayed when the user double clicks on the star, or clicks a second time.  This dialog also replaces the new bookmarks dialog.

When nothing is entered in the tag field box the text "tag one, tag two, tag three," appears in a grey font.

The default location is based on how the UI was displayed:

Double click on star:  All Bookmarks

Bookmarks Menu>Bookmark this Page: Bookmarks Menu

Accel-D: Bookmarks Menu

The expanded list of all tags is sorted alphabetically.

	Ivan Ičin
	Comment 3 • 17 years ago

I think that "All Bookmarks" and "Bookmarks menu" would be confusing for new users. Two suggestions:

1) Starred pages (e.g. like in Picasa) and Bookmarks menu. And Other starred pages for non-menu/toolbar items.

2) All Bookmarks and Classic Bookmarks menu and Classic Bookmarks toolbar.

	Asaf Romano (gone) Assignee
	Comment 5 • 17 years ago

Applies after ndef-MOZ_PLACES_BOOKMARKS removal.

	Asaf Romano (gone) Assignee
	Comment 7 • 17 years ago

there's _a lot_ more to do once this lands

	Dietrich Ayala (:dietrich)
	Comment 8 • 17 years ago

Comment on attachment 276709 [details] [diff] [review]
something to land, v1

>+  // nsIDOMEventListener
>+  handleEvent: function PSB_handleEvent(aEvent) {
>+    if (aEvent.originalTarget != this.panel)
>+      return;
>+
>+    // This only happens for auto-hide. When the panel is closed from within
>+    // itself, doneCallback removes the listener and only then hides the popup
>+    gAddBookmarksPanel.saveItem();
>+    gAddBookmarksPanel.uninitPanel();

in the auto-hide case, do you need to remove the listener here?

>+  },
>+
>+  showBookmarkPagePopup: function PSB_showBookmarkPagePopup(aURL) {
>+    const bms = PlacesUtils.bookmarks;
>+
>+    var starIcon = document.getElementById("star-icon");
>+    var panel = this.panel;
>+    panel.showPopup(starIcon, -1, -1, "popup", "bottomright", "topright");
>+
>+    var itemId = -1;
>+    var bmkIds = bms.getBookmarkIdsForURI(this._url, {});
>+    for each (var bk in bmkIds) {
>+      // Find the first folder which isn't a tag container
>+      var folder = bms.getFolderIdForItem(bk);
>+      if (folder == PlacesUtils.placesRoot ||
>+          bms.getFolderIdForItem(folder) != PlacesUtils.tagRootId) {
>+        itemId = bk;
>+        break;
>+      }
>+    }
>+    if (itemId == -1) {
>+      // if the remaining items for this url are under tag containers,
>+      // star the page first
>+      itemId = this._star();
>+    }

for clarification, is this correct?

1. items are starred if they're not already bookmarked
2. bookmarked items are considered starred
3. starred but not filed items are stored in the places root

>+  // nsINavBookmarkObserver  
>+  onItemAdded: function(aItemId, aFolder, aIndex) {
>+    this.updateState(this._url);
>+  },
>+
>+  onItemRemoved: function(aItemId, aFolder, aIndex) {
>+    this.updateState(this._url);
>+  },
>+
>+  onItemChanged: function(aItemId, aProperty, aIsAnnotationProperty, aValue) {
>+    if (aProperty == "uri")
>+      this.updateState(this._url);
>+  },

these 3 preceding methods could all be called when some random extension starts doing it's
bookmark import, etc. maybe should compare against this._url first?

>+  // nsIMicrosummaryObserver
>+  onContentLoaded: function ABP_onContentLoaded(aMicrosummary) {
>+    var namePicker = this._element("editBMPanel_namePicker");
>+    var childNodes = namePicker.menupopup.childNodes;
>+
>+    // 0: user-entered item; 1: separator
>+    for (var i = 2; i < childNodes.length; i++) {
>+      if (childNodes[i].microsummary == aMicrosummary) {
>+        var newLabel = aMicrosummary.content;
>+        // XXXmano: non-editable menulist would do this for us, see bug 360220
>+        // We should fix editable-menulsits to set the DOMAttrModified handler
>+        // as well.

s/menulsits/menulists/

>+        //
>+        // Also note the order importance: if the label of the menu-item is
>+        // set the something different than the menulist's current value,

s/the/to/

>+      if (tagsToAdd.length > 0)
>+        PlacesUtils.tagging.tagURI(this._uri, tagsToAdd, tagsToAdd.length);
>+      if (tagsToRemove.length > 0)
>+        PlacesUtils.tagging.untagURI(this._uri, tagsToRemove, tagsToRemove.length);

no longer need the length parameters

>+    }
>+
>+    if (txns.length > 0)
>+      PlacesUtils.ptm.commitTransaction(PlacesUtils.ptm.aggregateTransactions("", txns));

why no aggregate txn name?

up to here, will continue in the AM.

	Asaf Romano (gone) Assignee
	Comment 9 • 17 years ago

(In reply to comment #8)
> (From update of attachment 276709 [details] [diff] [review])
> >+  // nsIDOMEventListener
> >+  handleEvent: function PSB_handleEvent(aEvent) {
> >+    if (aEvent.originalTarget != this.panel)
> >+      return;
> >+
> >+    // This only happens for auto-hide. When the panel is closed from within
> >+    // itself, doneCallback removes the listener and only then hides the popup
> >+    gAddBookmarksPanel.saveItem();
> >+    gAddBookmarksPanel.uninitPanel();
> 
> in the auto-hide case, do you need to remove the listener here?

Not for any good reason I could think of.

> >+  },
> >+
> >+  showBookmarkPagePopup: function PSB_showBookmarkPagePopup(aURL) {
> >+    const bms = PlacesUtils.bookmarks;
> >+
> >+    var starIcon = document.getElementById("star-icon");
> >+    var panel = this.panel;
> >+    panel.showPopup(starIcon, -1, -1, "popup", "bottomright", "topright");
> >+
> >+    var itemId = -1;
> >+    var bmkIds = bms.getBookmarkIdsForURI(this._url, {});
> >+    for each (var bk in bmkIds) {
> >+      // Find the first folder which isn't a tag container
> >+      var folder = bms.getFolderIdForItem(bk);
> >+      if (folder == PlacesUtils.placesRoot ||
> >+          bms.getFolderIdForItem(folder) != PlacesUtils.tagRootId) {
> >+        itemId = bk;
> >+        break;
> >+      }
> >+    }
> >+    if (itemId == -1) {
> >+      // if the remaining items for this url are under tag containers,
> >+      // star the page first
> >+      itemId = this._star();
> >+    }
> 
> for clarification, is this correct?
> 
> 1. items are starred if they're not already bookmarked
> 2. bookmarked items are considered starred
> 3. starred but not filed items are stored in the places root

right.

> >+  // nsINavBookmarkObserver  
> >+  onItemAdded: function(aItemId, aFolder, aIndex) {
> >+    this.updateState(this._url);
> >+  },
> >+
> >+  onItemRemoved: function(aItemId, aFolder, aIndex) {
> >+    this.updateState(this._url);
> >+  },
> >+
> >+  onItemChanged: function(aItemId, aProperty, aIsAnnotationProperty, aValue) {
> >+    if (aProperty == "uri")
> >+      this.updateState(this._url);
> >+  },
> 
> these 3 preceding methods could all be called when some random extension starts
> doing it's
> bookmark import, etc. maybe should compare against this._url first?
> 

hrm, onItemAdded/Removed don't take a uri parameter... I could optimize this listener for the batching case though.

	Dietrich Ayala (:dietrich)
	Comment 11 • 17 years ago

Comment on attachment 276789 [details] [diff] [review]
more...


some problems i noticed:

- folder popup appears behind the panel sometimes
- after autohide, the panel is sometimes partially visible behind the toolbar
- some of the back-end interaction is out of sync somehow: i saw some "removing item that doesn't exist" errors. don't know if this is tag or bookmark related.

however, all the actions that i tested worked ok: bookmarks added and removed, tags, descriptions, etc were all added and removed properly, etc.

clearly more work to do, but r=me given fixes for the comments inline.

can you please follow up by filing bugs for major known items that could be spun off for others could work on, or at the least list them here to minimize the level of dupes filed.

>@@ -1363,18 +1373,21 @@ var PlacesUtils = {
>     var annosvc = this.annotations;
>     aAnnos.forEach(function(anno) {
>       if (anno.type == annosvc.TYPE_BINARY) {
>         annosvc.setItemAnnotationBinary(aItemId, anno.name, anno.value,
>                                         anno.value.length, anno.mimeType,
>                                         anno.flags, anno.expires);
>       }
>       else {
>+        var flags = ("flags" in anno) ? anno.flags : 0;
>+        var expires = ("expires" in anno) ?
>+          anno.expires : Ci.nsIAnnotationService.EXPIRE_NEVER
>         annosvc.setItemAnnotation(aItemId, anno.name, anno.value,
>-                                  anno.flags, anno.expires);
>+                                  flags, expires);
>       }
>     });
>   },
> 

is this change also needed for setAnnotationsForURI?

> 
>   // nsITaggingService
>-  untagURI: function TS_untagURI(aURI, aTags, aCount) {
>+  untagURI: function TS_untagURI(aURI, aTags) {
>     if (!aURI)
>       throw Cr.NS_ERROR_INVALID_ARG;
> 
>+    if (!aTags) {
>+      // see IDL.
>+      // XXXmano: write a perf-sensitive version of this code path...
>+      var tags = this.getTagsForURI(aURI);
>+      if (tags.length > 0)
>+        this.untagURI(aURI, tags);
>+      return;

couldn't you set aTags = this.getTagsForURI(), return early if the uri has zero tags, and continue otherwise? that wouldn't be more performant, but might be less weird.

> 
>   // nsITaggingService
>   getTagsForURI: function TS_getTagsForURI(aURI) {
>     if (!aURI)
>       throw Cr.NS_ERROR_INVALID_ARG;
> 
>     var tags = [];
>-
>     var bookmarkIds = this._bms.getBookmarkIdsForURI(aURI, {});
>+    var root = this._tagsResult.root;
>+    var cc = root.childCount;
>     for (var i=0; i < bookmarkIds.length; i++) {
>       var parent = this._bms.getFolderIdForItem(bookmarkIds[i]);
>-      for (var j=0; j < this._tags.length; j++) {
>-        if (this._tags[j].itemId == parent)
>-          tags.push(this._tags[j].name);
>+      for (var j=0; j < cc; j++) {
>+        var child = root.getChild(j);
>+        if (child.itemId == parent)
>+          tags.push(child.title);
>       }
>     }

seems like this could be optimized by caching the tag folder ids the first time through, minimizing the xpconnect boundary crossings.

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

Mano: here are some temporary icons to use

	Asaf Romano (gone) Assignee
	Comment 16 • 17 years ago

mozilla/browser/base/content/browser-places.js 1.38
mozilla/browser/base/content/browser.js 1.821
mozilla/browser/base/content/browser.xul 1.355
mozilla/browser/components/places/jar.mn 1.38
mozilla/browser/components/places/content/editBookmarkOverlay.js initial revision: 1.1
mozilla/browser/components/places/content/editBookmarkOverlay.xul 
initial revision: 1.1
mozilla/browser/components/places/content/treeView.js 1.14
mozilla/browser/components/places/content/utils.js 1.55
mozilla/browser/locales/jar.mn 1.69
mozilla/browser/locales/en-US/chrome/browser/places/editBookmarkOverlay.dtd initial revision: 1.1
mozilla/browser/themes/pinstripe/browser/browser.css 1.62
mozilla/browser/themes/pinstripe/browser/jar.mn 1.45
mozilla/browser/themes/pinstripe/browser/places/editBookmarkOverlay.css initial revision: 1.1
mozilla/browser/themes/pinstripe/browser/places/pageStarred.png
initial revision: 1.1
mozilla/browser/themes/pinstripe/browser/places/starPage.png 
initial revision: 1.1
mozilla/browser/themes/winstripe/browser/browser.css 1.73
mozilla/browser/themes/winstripe/browser/jar.mn 1.40
mozilla/browser/themes/winstripe/browser/places/editBookmarkOverlay.css 
initial revision: 1.1
mozilla/browser/themes/winstripe/browser/places/pageStarred.png 
initial revision: 1.1
mozilla/browser/themes/winstripe/browser/places/starPage.png 
initial revision: 1.1
mozilla/toolkit/components/places/public/nsITaggingService.idl 1.3
mozilla/toolkit/components/places/src/nsTaggingService.js 1.3
mozilla/toolkit/components/places/tests/unit/test_tagging.js 1.4

	Jesse Maloney (jtmal0723 [at] gmail for email)
	Comment 17 • 17 years ago

With the new star implementation, the icon is stretched

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

	Dietrich Ayala (:dietrich)
	Comment 19 • 17 years ago

(In reply to comment #18)
> Created an attachment (id=277022) [details]
> more...
> 

testing this patch on osx, when bookmarking via "bookmark this page..." or "bookmark this link...", the popup loads in the top-left corner of the viewport.

	Dietrich Ayala (:dietrich)
	Comment 20 • 17 years ago

Comment on attachment 277022 [details] [diff] [review]
more...

everything looks good except the placement issue. r=me otherwise.

	Asaf Romano (gone) Assignee
	Comment 21 • 17 years ago

[10:02]	<Mano>	dietrich: for "boomark this link/frame" that is by design
[10:03]	<dietrich>	really?
[10:03]	<Mano>	dietrich: yeah, we don't want to attach the popup to the location bar when it bookmarks something else
[10:04]	<Mano>	might change, but this is what mconnor and I came up with, for now.
[10:04]	<dietrich>	centered like the old dialog would be preferable to the corner, i think
[10:04]	<dietrich>	ok, tweakable later
[10:04]	<Mano>	dietrich: well, it's border-less so
[10:05]	<Mano>	dietrich: i will fix context-menu bookmark-this-page later
[10:05]	<Mano>	dietrich: we also use the content area when the location bar isn't visible

	Asaf Romano (gone) Assignee
	Comment 22 • 17 years ago

mozilla/browser/base/content/browser-places.js 1.41
mozilla/browser/base/content/browser-sets.inc 1.100
mozilla/browser/base/content/browser.js 1.822
mozilla/browser/base/content/browser.xul 1.358
mozilla/browser/base/content/nsContextMenu.js 1.22
mozilla/browser/components/places/content/utils.js 1.56
mozilla/browser/themes/pinstripe/browser/browser.css 1.63
mozilla/browser/themes/winstripe/browser/browser.css 1.74

	Asaf Romano (gone) Assignee
	Comment 23 • 17 years ago

- changes to the item being edited are are applied as they're done.
 - restore microsummaries support
 - various fixes to make the overlay reusable by the organizer.
 - partial work to make the overlay code item-type-agnostic.

	Asaf Romano (gone) Assignee
	Comment 24 • 17 years ago

see comment 23.

	Alfred Kayser
	Comment 25 • 17 years ago

When using 'Bookmark This Link', we popup as buttons 'Delete' and 'Ok'. As the bookmark is being created with the panel, the 'Delete' is confusing. Normal behaviour is then to 'Cancel' the action. 
So, can we when this dialog is called from Bookmark This Link from the context-menu on a page, have the normal buttons: Cancel and OK?

	Alfred Kayser
	Comment 26 • 17 years ago

More issues:
the two buttons don't have an ID, so one cannot style them (giving them each specific button icon).
The panel has no title, so it can be confusing what it is really doing...
Furthermore the panel has fixed width, so with long titles, it becomes awkward.
Name and folder labels have a ':', but Keywords is missing it.
Tooltips are completely missing.
In the UI mockup (see http://wiki.mozilla.org/Places:User_Interface/I6
there is an text describing that keywords need ','. This is also missing from the panel.

Question: is there some attribute set to the 'star-icon' item, when the panel is shown? I want to restyle the icon when the panel is open.

	Dietrich Ayala (:dietrich)
	Comment 27 • 17 years ago

Comment on attachment 277317 [details] [diff] [review]
another round

>   /**
>    * Initialize the panel
>    */
>-  initPanel: function ABP_initPanel(aItemId, aTm, aDoneCallback, aInfo) {
>+  initPanel: function EIO_initPanel(aItemId, aInfo) {
>     this._folderMenuList = this._element("folderMenuList");
>     this._folderTree = this._element("folderTree");
>-    this._tm = aTm;
>     this._itemId = aItemId;
>-    this._uri = PlacesUtils.bookmarks.getBookmarkURI(this._itemId);
>-    this._doneCallback = aDoneCallback;
>+    this._itemType = PlacesUtils.bookmarks.getItemType(this._itemId);
>     this._determineInfo(aInfo);
> 
>+    if (this._itemType == Ci.nsINavBookmarksService.TYPE_BOOKMARK) {
>+      this._uri = PlacesUtils.bookmarks.getBookmarkURI(this._itemId);
>+      // tags field
>+      this._element("tagsField").value =
>+        PlacesUtils.tagging.getTagsForURI(this._uri).join(", ");
>+
>+      this._element("locationField").value = this._uri.spec;
>+    }
>+
>     // folder picker
>     this._initFolderMenuList();
>-    
>+
>     // name picker
>     this._initNamePicker();
>-
>-    // tags field
>-    this._element("tagsField").value = this._currentTags.join(", ");
>-
>+    

nit: whitespace

>-    if (txns.length > 0) {
>-      // Mark the containing folder as recently-used if it isn't the
>-      // "All Bookmarks" root
>-      if (container != PlacesUtils.placesRootId)
>-        this._markFolderAsRecentlyUsed(container);
>+  onNamePickerInput: function EIO_onNamePickerInput() {
>+    var title = this._element("namePicker").value;
>+    this._element("userEnteredName").label = title;
>+  },
>+
>+  onNamePickerChange: function EIO_onNamePickerBlur() {

s/Blur/Change/

>-  toggleFolderTreeVisibility: function ABP_toggleFolderTreeVisibility() {
>+  onLocationFieldBlur: function EIO_onLocationFieldBlur() {
>+    // XXXmano: uri fixup
>+    var uri;
>+    try {
>+      uri = IO.newURI(this._element("locationField").value);
>+    }
>+    catch(ex) { return; }

hrm, currently the bookmark is not able to be saved w/o a valid URI.

in this scenario we should at least give feedback to the user that this edit did not actually get saved. followup is fine.

>-    this._folderTree.selectFolders([this._getFolderIdFromMenuList()]);
>+    // Update folder-tree selection
>+    if (isElementVisible(this._folderTree)) {
>+      var selectedNode = this._folderTree.selectedNode;
>+      if (selectedNode.itemId != container)
>+        this._folderTree.selectFolders(container);

selectFolders takes an array

>+  // nsINavBookmarkObserver
>+  onItemChanged: function EIO_onItemChanged(aItemId, aProperty,
>+                                            aIsAnnotationProperty, aValue) {
>+    if (this._itemId != aItemId)
>+      return;
>+
>+    switch (aProperty) {
>+    case "title":
>+      if (PlacesUtils.annotations.itemHasAnnotation(this._itemId,
>+                                                    STATIC_TITLE_ANNO))
>+        return;  // onContentLoaded updates microsummary-items
>+
>+      var userEnteredNameField = this._element("userEnteredName");
>+      if (userEnteredNameField.value != aValue) {
>+          userEnteredNameField.value = aValue;
>+        var namePicker = this._element("namePicker");
>+        if (namePicker.selectedItem == userEnteredNameField)
>+            namePicker.label = aValue;

nit: some messed up indenting on a couple of lines here

>+      }
>+      break;
>+    case "uri":
>+      var locationField = this._element("locationField");
>+      if (locationField.value != aValue) {
>+        locationField.value = aValue;
>+        this._uri = IO.newURI(aValue);
>+        this._initNamePicker(); // for microsummaries
>+        this._element("tagsField").value =
>+          PlacesUtils.tagging.getTagsForURI(this._uri).join(", ");
>+        this._rebuildTagsSelectorList();
>+      }
>+      break;
>+    case DESCRIPTION_ANNO:
>+      this._element("descriptionField").value =
>+        PlacesUtils.annotations.getItemDescription(this._itemId);
>+      break;
>+    }
>+  },

what's the use-case for this? the user leaves the panel open, and then makes edits via the organizer, or extensions changing bookmark properties?

i'm mildly concerned about a user making some edits, and then a sync-like extension runs in the background and overwrites the local edits as the user is making them.


r=me given the changes above.

	Asaf Romano (gone) Assignee
	Comment 28 • 17 years ago

> what's the use-case for this? the user leaves the panel open, and then makes
> edits via the organizer, or extensions changing bookmark properties?

the user leaves the the item-detail pane in the organizer open and changes the bookmark through another panel/dialog.

	Asaf Romano (gone) Assignee
	Comment 29 • 17 years ago

mozilla/browser/base/content/browser-places.js 1.42
mozilla/browser/base/content/browser.xul 1.359
mozilla/browser/components/places/content/editBookmarkOverlay.j 1.2
mozilla/browser/components/places/content/editBookmarkOverlay.xul 1.2
mozilla/browser/components/places/content/places.js 1.96
mozilla/browser/locales/en-US/chrome/browser/browser.dtd 1.63
mozilla/browser/locales/en-US/chrome/browser/places/editBookmarkOverlay.dtd 1.2

	Michael Kraft [:morac]
	Comment 30 • 17 years ago

(In reply to comment #2)
> Double click on star:  All Bookmarks
> 
> Bookmarks Menu>Bookmark this Page: Bookmarks Menu
> 
> Accel-D: Bookmarks Menu
> 

Has this been implemented this way?  All 3 methods above default to "All Bookmarks" in Gecko/2007082505 Minefield/3.0a8pre (Windows).

	Asaf Romano (gone) Assignee
	Comment 31 • 17 years ago

Thanks, good catch; filed bug 393710.

	u286998
	Comment 32 • 17 years ago

dismissing the dialog while the tag selector is expanded does not update the visual appearance of the collapse/expand button

>+  uninitPanel: function EIO_uninitPanel(aHideCollapsibleElements) {
>+    if (aHideCollapsibleElements) {
>+      // hide the folder tree if it was previously visible
>+      if (!this._folderTree.collapsed)
>+        this.toggleFolderTreeVisibility();
>+
>+      // hide the tag selector if it was previously visible
>+      var tagsSelector = this._element("tagsSelector");
>+      if (!tagsSelector.collapsed)
>+        tagsSelector.collapsed = true;

the last line should probably be this.toggleTagsSelector()

	Asaf Romano (gone) Assignee
	Comment 33 • 17 years ago

* comment 32
 * bug 393710
 * move the star icon out of the location bar
 * remove workarounds to bug 392512
 * remove workaround to bug 392397
 * fix some js errors in the overlay.
 * bug 393887

	Dietrich Ayala (:dietrich)
	Comment 34 • 17 years ago

Comment on attachment 278477 [details] [diff] [review]
comment 33

>@@ -542,34 +542,29 @@ var gEditItemOverlay = {
>       // "All Bookmarks" root
>       if (container != PlacesUtils.placesRootId)
>         this._markFolderAsRecentlyUsed(container);
>     }
> 
>     // Update folder-tree selection
>     if (isElementVisible(this._folderTree)) {
>       var selectedNode = this._folderTree.selectedNode;
>-      if (selectedNode.itemId != container)
>+      //alert("selectedNode.itemId command: " + selectedNode.itemId);
>+      if (!selectedNode || selectedNode.itemId != container)
>         this._folderTree.selectFolders([container]);
>     }

remove the debugging line please

r=me otherwise.

	Asaf Romano (gone) Assignee
	Comment 36 • 17 years ago

mozilla/browser/base/content/browser-places.js 1.43
mozilla/browser/base/content/browser-sets.inc 1.101
mozilla/browser/base/content/browser.js 1.827
mozilla/browser/base/content/browser.xul 1.362
mozilla/browser/base/content/nsContextMenu.js 1.24
mozilla/browser/components/places/content/editBookmarkOverlay.js 1.4
mozilla/browser/components/places/content/editBookmarkOverlay.xul 1.3
mozilla/browser/components/places/content/utils.js 1.60
mozilla/browser/themes/pinstripe/browser/browser.css 1.67
mozilla/browser/themes/winstripe/browser/browser.css 1.82

	Asaf Romano (gone) Assignee
	Comment 37 • 17 years ago

Resolving, further work tbd. in follow-ups.

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

Mano: the star needs to be placed inside the location bar, (as opposed to a separate button next to go) so that it lines up with the stars in auto-complete:

https://bugzilla.mozilla.org/attachment.cgi?id=277820

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

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

	Aronnax
	Comment 40 • 17 years ago

(In reply to comment #38)
> Mano: the star needs to be placed inside the location bar, (as opposed to a
> separate button next to go) so that it lines up with the stars in
> auto-complete:
> 
> https://bugzilla.mozilla.org/attachment.cgi?id=277820
> 

Hi, as far as i know is it decided to move the Star back inside of the location bar. Or is this changed? Would be nice to know, in particular some theme design options depends on it.
 
Cheers  

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

>move the Star back inside of the location bar

We will pick up that change with the next iteration, but it doesn't need to hit M9.

	u286998
	Comment 42 • 16 years ago

>the star needs to be placed inside the location bar, so that it lines up with the stars in auto-complete
Just noticed that the stars don't line up if there is no scrollbar, is that a problem/bug?

	Gervase Markham [:gerv]
	Comment 44 • 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