Closed Bug 394252 Opened 15 years ago Closed 14 years ago

Unable to create a bookmark folder with Star menu

Categories

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

defect

Tracking

()

VERIFIED FIXED
Firefox 3 beta5

People

(Reporter: tchung, Assigned: mano)

References

Details

Attachments

(3 files, 3 obsolete files)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; es-ES; rv:1.9a8pre) Gecko/2007082904 Minefield/3.0a8pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; es-ES; rv:1.9a8pre) Gecko/2007082904 Minefield/3.0a8pre

I am unable to figure out how to create a new bookmark folder if i simply go to bookmarks > Bookmark this Page...   It pops up the drop down menu from the Star in location bar, but there is no option to create a new folder.  

Reproducible: Always

Steps to Reproduce:
1. Install latest trunk
2. open any page, and go to Bookmarks > bookmark this page...
3. A popup screen will appear under the Star in location bar
4. Verify there is no way to create a new Bookmark Folder
Actual Results:  
No Option to create a bookmark folder in bookmark menu

Expected Results:  
Able to create a bookmark folder.
Attached image No Bookmarks Folder
Adding a screenshot
Flags: blocking-firefox3?
Adding Litmus flag. We need to have cases for folder creation from standard entry points.
Flags: in-litmus?
Flags: blocking-firefox3? → blocking-firefox3+
Keywords: uiwanted
Target Milestone: --- → Firefox 3 M10
Yes, this was a rather moronic omission on my part, and will be fixed in the next iteration of the design (bug #393509)
Depends on: 393509
Version: unspecified → Trunk
Target Milestone: Firefox 3 M10 → Firefox 3 Mx
Target Milestone: Firefox 3 Mx → Firefox 3 M11
Priority: -- → P2
Priority: P2 → P3
Duplicate of this bug: 404684
(In reply to comment #3)
> Yes, this was a rather moronic omission on my part, and will be fixed in the
> next iteration of the design (bug #393509)

The New Folder button is included in the latest mockup <http://people.mozilla.com/~faaborg/files/granParadisoUI/places_NewBookmarkDialog_i9.png>.  I'm switching the dependencies here.
Blocks: 393509
No longer depends on: 393509
Keywords: uiwanted
Assignee: nobody → mano
Priority: P3 → P2
Target Milestone: Firefox 3 beta3 → Firefox 3 beta4
Attached patch patch (obsolete) — Splinter Review
Attachment #302001 - Flags: review?(dietrich)
Attached patch patch (obsolete) — Splinter Review
Attachment #302001 - Attachment is obsolete: true
Attachment #302008 - Flags: review?(dietrich)
Attachment #302001 - Flags: review?(dietrich)
Comment on attachment 302008 [details] [diff] [review]
patch

minusing because i was getting all kinds of funky tree behavior when clicking the button:

- multiple rows selected, instead of starting edit mode
- made the menu folder disappear
- opened pre-existing folders for editing

>+          // In edit mode, if we're not editing a folder, the ESC key is mapped
>+          // to the cancel button
>+          if (!this._element("editBookmarkPanelContent").hidden) {
>+            var elt = aEvent.target;
>+            if (elt.localName != "tree" ||
>+                (elt.localName == "tree" && !elt.hasAttribute("editing")))
>+              this.cancelButtonOnCommand();
>+          }
>+        }
>+        else if (aEvent.keyCode == KeyEvent.DOM_VK_RETURN) {
>+          // hide the panel if the unless the folder tree is focused
>+          if (aEvent.target.localName != "tree")
>+            this.panel.hidePopup(); 

nit: s/if the//

>@@ -260,24 +262,33 @@ var StarUI = {
>       this.panel.popupBoxObject
>           .setConsumeRollupEvent(Ci.nsIPopupBoxObject.ROLLUP_CONSUME);
>       this.panel.openPopup(aAnchorElement, aPosition, -1, -1);
>     }
>     else
>       this.panel.focus();
>   },
> 
>+  quitEditMode: function SU_quitEditMode() {
>+    this._element("editBookmarkPanelContent").hidden = true;
>+    this._element("editBookmarkPanelBottomButtons").hidden = true;
>+    gEditItemOverlay.uninitPanel(true);
>+  },
>+
>   editButtonCommand: function SU_editButtonCommand() {
>     this.showEditBookmarkPopup();
>   },
> 
>   cancelButtonOnCommand: function SU_cancelButtonOnCommand() {
>+    // The order is is important! We have to hide the panel first, otherwise

s/is is/is/

>+    // changes done as part on Undo may change the panel contents and by

s/on/of/

>+  isEditable: function PTV_isEditable(aRow, aColumn) {
>+    // At this point we only support editing the title field.
>+    if (aColumn.index != 0)
>+      return false;
>+
>+    var node = this.nodeForTreeIndex(aRow);
>+    if (PlacesUtils.nodeIsFolder(node) ||
>+        (PlacesUtils.nodeIsBookmark(node) &&
>+         !PlacesUtils.nodeIsLivemarkItem(node)))
>+      return true;
>+

&& !nodeIsReadOnly()
Attachment #302008 - Flags: review?(dietrich) → review-
Whiteboard: [needs new patch]
Duplicate of this bug: 417289
Duplicate of this bug: 417162
Duplicate of this bug: 415920
Duplicate of this bug: 417846
Duplicate of this bug: 419223
Target Milestone: Firefox 3 beta4 → Firefox 3
This is still bad in beta 4.
>  >+

>  && !nodeIsReadOnly()

This cannot happen in this tree, given the query specified.
Status: NEW → ASSIGNED
Whiteboard: [needs new patch]
Attached patch patch (obsolete) — Splinter Review
Attachment #302008 - Attachment is obsolete: true
Attachment #309080 - Flags: review?(dietrich)
Comment on attachment 309080 [details] [diff] [review]
patch

r=me
Attachment #309080 - Flags: review?(dietrich) → review+
Attached patch as checked inSplinter Review
mozilla/browser/base/content/browser-places.js 1.117
mozilla/browser/components/places/content/editBookmarkOverlay.js 1.36;
mozilla/browser/components/places/content/editBookmarkOverlay.xul 1.15
mozilla/browser/components/places/content/tree.xml 1.111;
mozilla/browser/components/places/content/treeView.js 1.58
Attachment #309080 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 3 → Firefox 3 beta5
Clicking the 'New Folder button' does nothing.  No folder created. 

In the console2: 
Error: PlacesUtils.ptm is undefined
Source file: chrome://browser/content/places/editBookmarkOverlay.js
Line: 786

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b5pre) Gecko/2008031402 Minefield/3.0b5pre Firefox/3.0 ID:2008031402
(In reply to comment #19)
> Clicking the 'New Folder button' does nothing.  No folder created. 
> 
> In the console2: 
> Error: PlacesUtils.ptm is undefined
> Source file: chrome://browser/content/places/editBookmarkOverlay.js
> Line: 786

one more in Console2,

Warning: reference to undefined property PlacesUtils.ptm
Source file: chrome://browser/content/places/editBookmarkOverlay.js
Line: 786 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008031410 Minefield/3.0b5pre ID:2008031410

"Ne Folder" button is fixed.
Duplicate of this bug: 423202
No, the errors still appear within the Error Console and I'm not able to give the new folder a name. Following exception/errors are shown:

Error: PlacesUtils.ptm is undefined
Source File: chrome://browser/content/places/treeView.js
Line: 1367

Error: uncaught exception: [Exception... "'[JavaScript Error: "PlacesUtils.ptm is undefined" {file: "chrome://browser/content/places/treeView.js" line: 1367}]' when calling method: [nsITreeView::setCellText]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: chrome://global/content/bindings/tree.xml :: stopEditing :: line 371"  data: yes]

You have to close the bookmarks panel. After you reopen it the dialog looks destroyed. I'll attach a screenshot. Tested under Windows and OS X with the similar builds: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b5pre) Gecko/2008031604 Minefield/3.0b5pre ID:2008031604
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
There are 2 other things that I think should be addressed:

1. 'New Folder' will only add a folder to a folder that already contain other folders. If you try to add a new folder to a folder that does not contain any folders, if will be added to the folder's parent.

(gah, too many 'folders'!)

2. Double-click to expand a folder also makes the folder's name editable. This is undesirable behavior because it can lead to accidental folder renaming (I suppose that could be considered data loss). A better solution would to have a context menu that contains actions like 'rename folder' 'add folder' and 'delete folder'
For renaming, pressing F2 should be supported as it's standard in Windows.
Whiteboard: [needs new patch]
Henrik: i'm checking in a fix for the console error (which effectively broke this patch :-/). If you still see the oddness on windows, please file a follow up (I cannot reproduce it on mac)

Roman R: please file a follow up.
mozilla/browser/components/places/content/treeView.js 1.60
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
Whiteboard: [needs new patch]
verified with: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b5pre) Gecko/2008031704 Minefield/3.0b5pre and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008031704 Minefield/3.0b5pre

though bug 423267 seems to be blocking verification with a new profile.
Status: RESOLVED → VERIFIED
Asaf, thanks for the follow-up patch. No exception can be seen anymore.

Matthew, both should be filed as new bugs and be marked as blocking this one. I cannot find already existing bugs about. Will you file them or shall I do that?
Depends on: 423747
If a custom name is specified for the new folder, the custom name doesn't make it to the combobox above the folders listbox. Filed bug 423747.
Duplicate of this bug: 425416
edited the existing add folder test case to include the Star dialogs
Flags: in-litmus? → in-litmus+
Duplicate of this bug: 429563
(In reply to comment #25)
> 1. 'New Folder' will only add a folder to a folder that already contain other
> folders. If you try to add a new folder to a folder that does not contain any
> folders, if will be added to the folder's parent.

That should be fine. I cannot see this behavior anymore.

> 2. Double-click to expand a folder also makes the folder's name editable. This
> is undesirable behavior because it can lead to accidental folder renaming (I
> suppose that could be considered data loss). A better solution would to have a
> context menu that contains actions like 'rename folder' 'add folder' and
> 'delete folder'

Marco, the current behavior is intended, right?
Depends on: 428403
No longer depends on: 426237
editing on double click is intended, not on roots, but there's another bug covering that.
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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.