412682 - Organize/Context menu for bookmarks listed under Tags should be reworked

Status: VERIFIED FIXED

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

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008011304 Minefield/3.0b3pre ID:2008011304

If you click on a tag under Tags and open the context menu of any bookmark there are some confusing menuentries:

1. New Bookmark: The creation of a new bookmark will place it under this tag. It also gets automatically tagged but it isn't stored under Unfiled Bookmarks. It only exists within the selected tag.

2. New Folder: A new folder is created under that tag and is listed in the tree view.

3. New separator: Doesn't make sense here IMO.

4. Cut/Copy/Paste: Nice feature to re-tag a bookmark

5. Delete: Only the tag gets deleted. The bookmark still remains within its folder. Should it be labeled as "Remove Tag"?

6. Sort by Name: Why is this grayed out? It's still possible to sort by name

Asking for blocking-firefox3.

Comment 1

[Tracy Walker [:tracy]]

For item 5. Delete: Only the tag gets deleted. The bookmark still remains within its
folder. Should it be labeled as "Remove Tag"?


Make the label dynamic  "Remove Tag - 'selected tag'.  For example, same as Library > Tags > select a tag, then select main menu Edit.  Look at - Find In 'selected tag'

Also should "Delete" remain and actually delete the selected bookmark?

Comment 2

[Alex Faaborg [:faaborg] (Firefox UX)]

>Also should "Delete" remain and actually delete the selected bookmark?

Yeah, if the user is only using tags, they still need an easy way of deleting bookmarks.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #0)
> 1. New Bookmark: The creation of a new bookmark will place it under this tag.
> It also gets automatically tagged but it isn't stored under Unfiled Bookmarks.
> It only exists within the selected tag.

Should also be added under Unfiled Bookmarks, yes. This might need a separate bug, feel free to spin it off.

> 2. New Folder: A new folder is created under that tag and is listed in the tree
> view.

This should be removed. Doesn't make sense here.

> 3. New separator: Doesn't make sense here IMO.

Agree.

> 4. Cut/Copy/Paste: Nice feature to re-tag a bookmark

Agree.

> 5. Delete: Only the tag gets deleted. The bookmark still remains within its
> folder. Should it be labeled as "Remove Tag"?

I think "Remove tag" would be a separate menu item, as indicated in the previous comments. Deleting an entry should do the same thing everywhere, and actually delete that entry.

> 6. Sort by Name: Why is this grayed out? It's still possible to sort by name

Agree.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #3)
> Should also be added under Unfiled Bookmarks, yes. This might need a separate
> bug, feel free to spin it off.

Filed as bug 416310.

Nearly all mentioned issues on this bug also applies to the Organize menu within the toolbar.

Comment 5

[Alex Faaborg [:faaborg] (Firefox UX)]

Summarizing the decisions above:

==Disabled==

When viewing the contents of a tag, the following menu items need to be disabled in the context menu and the Organize menu (similar to when viewing history):

New Folder...
New Separator

These items should also be disabled when right clicking directly on a tag.

Cut/Copy/Paste

Ideally we could use these for adding and removing tags on bookmarks, but if that is out of scope let's just disabled them similar to viewing history.

==Enabled==

Sort by Name...

==Different functionality==

New Bookmark... creates a bookmark with that tag associated with it, and the bookmark is added to unsorted bookmarks.

Remove tag: added after delete

I'm removing the uiwanted keyword, but please add back if more information is needed.

Comment 6

[Mike Connor [:mconnor]]

Mano, what's needed here?

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #5)

Users with an existing profile which was used for beta versions of Firefox will be mostly affected by this bug!

> ==Disabled==
> 
> When viewing the contents of a tag, the following menu items need to be
> disabled in the context menu and the Organize menu (similar to when viewing
> history):
> 
> New Folder...
> New Separator

Both are still enabled and gives you the possibility to create a new folder under a tag. This folder cannot be deleted! Even the tag persists after removing all bookmarks which are using this tag.

> These items should also be disabled when right clicking directly on a tag.
> 
> Cut/Copy/Paste

Cut/Copy are still enabled even for a fresh profile.

> ==Enabled==
> 
> Sort by Name...

This one is disabled for existing and new profiles.

> ==Different functionality==
> 
> New Bookmark... creates a bookmark with that tag associated with it, and the
> bookmark is added to unsorted bookmarks.

For fresh profiles this menu item is disabled. Using an existing profile places this bookmark into the tag container but doesn't add it to Unsorted Bookmarks.

> Remove tag: added after delete

Not available at the moment.

Comment 8

[Marco Bonardo [:mak]]

(In reply to comment #7)
> (In reply to comment #5)
> 
> Users with an existing profile which was used for beta versions of Firefox will
> be mostly affected by this bug!

we need to wait for dietrich's change of the left pane to unbitrot leftpane versioning and upgrade them, but should be done before final, so old profiles problems should not be a blocker.

> > ==Disabled==
> > 
> > New Folder...
> > New Separator
> 
> Both are still enabled

only for old profiles

> > Cut/Copy/Paste
> 
> Cut/Copy are still enabled even for a fresh profile.

copy is ok is the default action, when dragging we are forcing a copy, cut should be fixed, i've seen cut also active in other views so probably has some underlying bug

> > ==Enabled==
> > 
> > Sort by Name...
> 
> This one is disabled for existing and new profiles.

Known issue, not fixable for now (perf, it would need a quite complex and slower query with dupes filtering inside, or a revise of tagging backend)

> > ==Different functionality==
> > 
> > New Bookmark... creates a bookmark with that tag associated with it, and the
> > bookmark is added to unsorted bookmarks.
> 
> For fresh profiles this menu item is disabled. Using an existing profile places
> this bookmark into the tag container but doesn't add it to Unsorted Bookmarks.

New bookmark need to be fixed, drag & drop works fine though

> > Remove tag: added after delete
> 
> Not available at the moment.

enhancement.
probably need to be fixed also context menu "delete" over a tag container, even if i can see users clicking it by error and loosing their tags.

Comment 9

[Mike Connor [:mconnor]]

I think at this point, we can't add new behaviour, and we can't add strings anyway, so I think we should be brutally simple:

Organizer Menu, when tag is selected:

Everything is disabled.

Organizer Menu, when item is selected:

Only Delete is enabled, it deletes the bookmark.  If it is the last bookmark with that tag, delete the tag (we do that already, I hope)

Context menu for items in tag view:

Open
Open in Tab
Open in Window
--------------
Copy
--------------
Delete
--------------
Properties

(Cut and Paste, and the New * items have complexity and advanced expected behaviour, Sort by Name implies an permanence that is not supported.  Its a context menu, so we should just hide the options in this case)

Comment 10

[Asaf Romano (gone)]

Attachment: per irc

Comment 11

[Mike Connor [:mconnor]]

Comment on attachment 316749 [details] [diff] [review]
per irc

r=me with the comment fix mentioned over IRC

a=me for checkin

Comment 12

[Asaf Romano (gone)]

mozilla/browser/components/places/content/controller.js 1.229
mozilla/browser/components/places/content/history-panel.js 1.40
mozilla/browser/components/places/content/places.js 1.158
mozilla/browser/components/places/content/placesOverlay.xul 1.35

Comment 13

[Asaf Romano (gone)]

Attachment: add missing check

mozilla/browser/components/places/content/controller.js, 1.230

Comment 14

[Thomas Stache]

Is that a typo in the patch that the attribute is called
"hideifnoinsetionpoint"?
           ----

Comment 15

[Recall]

I think this bug has caused a regression, see bug 430075

Comment 16

[Tracy Walker [:tracy]]

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

Comment 17

[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