Add "New Bookmark" to the bookmarks context menu

RESOLVED FIXED

Status

()

Firefox
Bookmarks & History
--
enhancement
RESOLVED FIXED
14 years ago
12 years ago

People

(Reporter: Dean Tessman, Assigned: Dean Tessman)

Tracking

({fixed-aviary1.0})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

14 years ago
I thought I'd seen a bug about this before, but I can't find it now.  I don't
see any sense in having a New Folder context menu item in the Bookmarks Sidebar
and Manage Bookmarks and not having New Bookmark.  I create many more bookmarks
than bookmark folders, as I'm sure does the average user.  Creating a bookmark
is also very context-sensitive as the position of the right-click dictates where
to create the bookmark, either in relation to the currently-selected bookmark or
in the selected folder.
(Assignee)

Updated

14 years ago
Flags: blocking1.0?
(Assignee)

Updated

14 years ago
Summary: Add "New Bookmark" to the bookmarks sidebar → Add "New Bookmark" to the bookmarks context menu

Comment 1

14 years ago
Would be nice to have, but not blocking 1.0. Dean, I bet you could easily make a
patch for this and drive it in ;)
Flags: blocking1.0? → blocking1.0-
(Assignee)

Comment 2

14 years ago
Created attachment 152082 [details] [diff] [review]
cvs diff -u mozilla/browser/components/bookmarks

This patch:

1. Adds "New Bookmark" and "New Separator" items to the bookmarks context menu
2. Adds an ellipsis to "New Separator" (don't ask me why I needed to do that in
two places)
3. Fixes bug 204406 by changing the access key for New Folder from 'F' to 'L'.
(Assignee)

Updated

14 years ago
Assignee: p_ch → dean_tessman
Status: NEW → ASSIGNED
(Assignee)

Comment 3

14 years ago
Comment on attachment 152082 [details] [diff] [review]
cvs diff -u mozilla/browser/components/bookmarks

Here it is, Blake.  How about a review?
Attachment #152082 - Flags: review?(firefox)
(Assignee)

Comment 4

14 years ago
Requesting blocking-aviary1.0 again now that there's a patch.
Flags: blocking-aviary1.0- → blocking-aviary1.0?
Comment on attachment 152082 [details] [diff] [review]
cvs diff -u mozilla/browser/components/bookmarks

The one thing I'm not sure I like about this patch is exposing the New Separtor
(and its dialog with the label) even more.

Unless separator labels are going to work in the BTF and bookmarks menus, I'm
thinking we should hide/disable the UI to add the labels.

Otherwise this looks fine.  r=me, for now, and I'll have to talk to ben about
the separator label stuff.
(Assignee)

Comment 6

14 years ago
Is it the New Separator menu item or just the actual dialog that you don't like?
 For now, while the label UI is in there, at least this patch indicates that an
intermediate dialog is going to appear, via the added ellipsis.
Comment on attachment 152082 [details] [diff] [review]
cvs diff -u mozilla/browser/components/bookmarks

go ahead and land this as-is, I'll kill the ellipsis when I fix bug 230370
(once someone fixes bug 236696, we can revisit that)
Attachment #152082 - Flags: review?(firefox) → review+
(Assignee)

Comment 8

14 years ago
Mike, I won't be around my build machine for the next few days.  Any chance you
could check this in for me?
sure, no problem.

I think I'll drop the ellipsis stuff before I check in though.
(Assignee)

Comment 10

14 years ago
Only if you drop the dialog at the same time.  Otherwise the ellipsis should be
there to indicate to the user that there's an additional step before the
separator is created.
that's the plan, of course :)
(Assignee)

Comment 12

14 years ago
Created attachment 153115 [details] [diff] [review]
patch without ellipsis

Fixed on branch alongside bug 230370.  This is what was checked in.  Still need
to fix this on the trunk when it opens.
(Assignee)

Updated

14 years ago
Attachment #152082 - Attachment is obsolete: true

Comment 13

14 years ago
Adding fixed-aviary1.0 keyword per comment #12
Keywords: fixed-aviary1.0
(Assignee)

Comment 14

14 years ago
I missed one case.  Mike, can you give me the go-ahead to check in this change
tonight?

-    case "BookmarkSeparator":
-      commands = ["bm_newfolder", "bm_separator", 
+      commands = ["bm_newbookmark", "bm_newfolder", "bm_newseparator",
"bm_separator",
r/a=me on that added bit.
(Assignee)

Comment 16

14 years ago
Fixed on trunk and branch.  (The trunk was open when I checked in - honest!)
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Updated

14 years ago
Flags: blocking-aviary1.0?
sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs, filter on "beltznerLovesGoats" to get rid of this mass change
QA Contact: mconnor → bookmarks
You need to log in before you can comment on or make changes to this bug.