Closed Bug 1741028 Opened 3 years ago Closed 2 years ago

Deleting bookmark from tag view doesn't delete bookmark, deletes tag instead (Context menu option is misleading)

Categories

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

Product:
Firefox
Component:
Bookmarks & History
Version:
Firefox 94
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
108 Branch
Tracking Flags:
Tracking Status
firefox108 --- verified

People

(Reporter: maybeoneday+bugzilla, Assigned: jsudiaman, Mentored)

References

Details

(Whiteboard: [lang=js])

Attachments

(1 file)

Bug 1741028 - Deleting bookmark from tag view doesn't delete bookmark, deletes tag instead (Context menu option is misleading) r?mak
48 bytes, text/x-phabricator-request
Details | Review

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Firefox/91.0

Steps to reproduce:

Go to tag.
Highlight bookmark(s).
Press Delete or Right-click - Delete Bookmark.

Actual results:

Bookmark isn't deleted.
Instead, tag is deleted from bookmark.
Bookmark is still somewhere in the Bookmarks Menu.
Have to then fish for it there to delete it.

Huge mess.
I had no idea this was the behavior, now I have a bazillion rogue bookmarks which I intended to delete but turns out are still there, some have other tags, some have none.
Total mess.

Expected results:

Like the button says, delete the bookmark.

There are already two ways to delete the tag (uncheck from tag list or remove from the Tags form), but apparently there is no way to delete the bookmark, other than going back to Bookmark Menu and fishing for it from there, hoping you get the right one.

And then you decide to delete a few bookmarks, and now you have to either oscillate between the tag and Bookmark Menu's search results (redoing the search every time) deleting them one-by-one, or memorize all of them at once (or write them down somewhere) and hope you get it all right in one try.

The Bugbug bot thinks this bug should belong to the 'Firefox::Bookmarks & History' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Component: Untriaged → Bookmarks & History

The behavior is correct, removing a tag should remove a tag, not the bookmark, because the same bookmark can have multiple tags.
The tagging UI is not so nice unfortunately.
I agree that the context menu option is now misleading, we should at least revert that to the plain Delete.

Severity: -- → S4
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
Summary: Deleting bookmark from tag view doesn't delete bookmark, deletes tag instead → Deleting bookmark from tag view doesn't delete bookmark, deletes tag instead (Context menu option is misleading)
Mentor: mak
Whiteboard: [lang=js]

So you're saying that there are in fact currently 3 options to delete a tag - all three about equally convenient and accessible from the same location - but zero to delete a bookmark?
What's the logic behind such decision?
How do you expect the user to delete a bookmark from the tag menu, let alone multiple bookmarks, other than as I stated in my followup comment by writing it down on a piece of paper?

Flags: needinfo?(mak)

(In reply to maybeoneday+bugzilla from comment #4)

So you're saying that there are in fact currently 3 options to delete a tag - all three about equally convenient and accessible from the same location - but zero to delete a bookmark?

There's multiple ways to delete a bookmark in Firefox, but not from a UI that is intended to manage tags.

How do you expect the user to delete a bookmark from the tag menu, let alone multiple bookmarks, other than as I stated in my followup comment by writing it down on a piece of paper?

The fact is, I don't expect users to remove bookmarks from a tagging ui. The current tags management ui is bad imho, it has been built like that because it was simpler and faster to just make tags look like folders of bookmarks.
I'd be ok with evaluating a further option to also Delete Bookmark, if we see users requesting it. So far in many years this is the first request I saw, I'm sure there's more users that would like it, but at the same time that option may cause unexpected dataloss: "I was managing tags and the bookmark disappeared". I would suggest to open an enhancement bug asking to add an additional Delete Bookmark option to bookmarks inside tag containers.

Flags: needinfo?(mak)

There's multiple ways to delete a bookmark in Firefox, but not from a UI that is intended to manage tags.

Yes that is what I meant. The "from the tag UI" part was implied. Leaving the tag UI and trying to do it elsewhere is, as I said, cumbersome at best, but sometimes nigh-on impossible.

The fact is, I don't expect users to remove bookmarks from a tagging ui.

But removing a bookmark is such a basic feature.
Like I said, there already are 2 convenient ways to delete a tag from a bookmark from that same screen.
What's the point of having yet another way while not having fundamental functionality like deleting the bookmark itself?

So far in many years this is the first request I saw

That could be because most people don't even know of tags' existence in Firefox (due to, as you noted, the tagging system being not so great) or what tags are in general.

that option may cause unexpected dataloss: "I was managing tags and the bookmark disappeared".

Do you mind elaborating? What happens between the managing tags and the bookmark disappearance parts? Like a bug?

I would suggest to open an enhancement bug asking to add an additional Delete Bookmark option to bookmarks inside tag containers.

Okay! Will do.

Flags: needinfo?(mak)

Also what about the Delete key? I would argue pressing Delete when a bookmark is selected shouldn't delete a tag when it deletes the bookmark everywhere else in the UI. That is unintuitive at best.

(In reply to maybeoneday+bugzilla from comment #6)

Like I said, there already are 2 convenient ways to delete a tag from a bookmark from that same screen.

Accessibility must be considered, for a set of users the context menu may be very important.

What's the point of having yet another way while not having fundamental functionality like deleting the bookmark itself?

Yes, I think we should have both options, but the default should be to untag, because we are inside a tag, and dataloss of a bookmark is worse.

that option may cause unexpected dataloss: "I was managing tags and the bookmark disappeared".

Do you mind elaborating? What happens between the managing tags and the bookmark disappearance parts? Like a bug?

No, it's expectations. You have the expectation that removing a bookmark from the tag container removes the bookmark, another user may have the opposite expectation "I'm inside a tag, I'm removing the tag". So when the bookmark goes away instead, we'll break their expectation and cause dataloss. Dataloss of a bookmark is worse than dataloss of a tag.

(In reply to maybeoneday+bugzilla from comment #7)

Also what about the Delete key? I would argue pressing Delete when a bookmark is selected shouldn't delete a tag when it deletes the bookmark everywhere else in the UI. That is unintuitive at best.

Delete should just do the default delete operation based on the type of the list, e.g. removing from history, from bookmarks, or from tags. Would you expect that removing a page from an history list also removes a bookmark, if that page is bookmarked? Most probably don't, and this is very similar.

Overall, the situation is just made worse by the ui using a listing that looks like bookmarks to show tagged urls. It is confusing.

Romain, my suggestion here would be to have a Delete Tag option (Untag may sound better but doesn't translate well), that is the default for Delete key too. We can also have Remove Bookmark as a separate option... but if this tag list UI would look different, it would probably not be necessary.

Flags: needinfo?(mak) → needinfo?(rtestard)

I agree that the current label is really misleading and that it would make sense to replace the "Delete Bookmark" label with something that clearly signals that the tag will be removed from the Bookmark ("Delete Tag", "Untag", "Remove tag from bookmark").
Meridel, this may be a good first bug on your side to confirm the label we should be using here, this is not urgent but something to add to your backlog.

Flags: needinfo?(rtestard) → needinfo?(mwalkington)

Thanks, Romain. New team members have hands full right now so let's just make an immediate improvement here. Anything we do on bookmarks is a bandaid – as we know the entire experience needs work.

I recommend we update this label to "Remove tag", and here's why:

"Delete tag" is inaccurate. The tag remains. You just remove the tag from the bookmark.
"Remove tag from bookmark" is long and will be very long once localized.
"Untag": I like how concise this is but the current nomenclature in our context menus is removing/delete things so this feels like it sticks out.

Flags: needinfo?(mwalkington)

Hi there this is Kumar Kalyan I am new to this community; can anyone help me out by letting me know about how can I get started with this bug ?
(Reach me out ) https://linktr.ee/kumarkalyan

The context menu items are in https://searchfox.org/mozilla-central/source/browser/components/places/content/placesContextMenu.inc.xhtml I think we should add a new "placesContext_removeTag" item, similar to placesContext_deleteBookmark.
Then we must modify the rules so that on Tags deleteBookmark is not shown, while the new removeTag is shown. We use attributes here to establish if a menuitem should be visible... we have an "hide-if-node-type" attribute, we can probably add "link_bookmark_tag" to deleteBookmark and that should hide the menu for these, and then add node-type="link_bookmark_tag" for removeTag.
That should hide/show depending on whether the node is a bookmark tag... if that doesn't work there may be more to modify to ensure detection is correct.

Bugs are assigned when a patch is posted to Phabricator, so feel free to work on this.
See
https://firefox-source-docs.mozilla.org/setup/index.html
https://firefox-source-docs.mozilla.org/contributing/index.html

(In reply to Marco Bonardo [:mak] from comment #8)

(In reply to maybeoneday+bugzilla from comment #6)

Like I said, there already are 2 convenient ways to delete a tag from a bookmark from that same screen.

Accessibility must be considered, for a set of users the context menu may be very important.

That goes both ways.
For the same set of users context menus may be important for deleting a bookmark.
And how is this an accessibility question anyway?
Someone who can right-click to open a context menu can also left click to tick a tag.

that option may cause unexpected dataloss: "I was managing tags and the bookmark disappeared".

Do you mind elaborating? What happens between the managing tags and the bookmark disappearance parts? Like a bug?

No, it's expectations. You have the expectation that removing a bookmark from the tag container removes the bookmark, another user may have the opposite expectation "I'm inside a tag, I'm removing the tag". So when the bookmark goes away instead, we'll break their expectation and cause dataloss. Dataloss of a bookmark is worse than dataloss of a tag.

Who would expect hitting Delete while having a bookmark highlighted would delete a tag from that bookmark?
This is such a far reach it almost sounds like an excuse to keep it the way it is.
And you can't really account for every user's mess-up; that's a losing game.
Especially if it's hurting other users.

(In reply to maybeoneday+bugzilla from comment #7)

Also what about the Delete key? I would argue pressing Delete when a bookmark is selected shouldn't delete a tag when it deletes the bookmark everywhere else in the UI. That is unintuitive at best.

Delete should just do the default delete operation based on the type of the list, e.g. removing from history, from bookmarks, or from tags. Would you expect that removing a page from an history list also removes a bookmark, if that page is bookmarked? Most probably don't, and this is very similar.

The difference is History is a completely independent entity from either tags or bookmarks, while tags and bookmarks are innately interconnected, and you can only add tags to bookmarks.
It's also why I never understood having the Tags entry alongside the History and All Bookmarks entry.

Furthermore, if a rework is incoming, then I would argue tags should be made into something more than glorified folders which they currently are.

From the moment I started using tags for bookmark organization, I've wanted a way to filter by more than a single tag.
E.g. I want bookmarks tagged both "deals" AND "shopping" AND "fashion," or "videos" but NOT "funny."

And that's where your proposal of having an option to "Delete/Remove Tag" would fall apart, because which of the 2, 3, 4 tags would that remove?

Here's an example of a well implemented tagging UI:
https://i.imgur.com/pMWTzAJ.png
This is from Google Keep, but really most tagging solutions I've seen work this way.
They do not care about what happens if this user expects this and that, removing a note removes the note (by pressing Delete or the Bin icon on the note), and removing a tag (by pressing the X icon next to each tag) removes the tag.
If you wanna filter multiple tags based on AND, OR and NOT rules, you do it with the search field or by selecting multiple tags in the side bar.
That's it.
There's no need to get into psychology of who expects what or make up accessibility problems, it's very simple and logical.

Flags: needinfo?(mak)

And if you really wanna get into psychology, then choice overload isn't a good thing either.
Having 3 very similar ways to remove a tag from the exact same screen is exhausting.

(In reply to maybeoneday+bugzilla from comment #13)

Someone who can right-click to open a context menu can also left click to tick a tag.

Well, this is not true, for various reasons. The user may have mobility or visual problems for which clicking on a checkbox is really hard, but they can use the context menu key on their keyboard, or have helpers to use the context menu.

Who would expect hitting Delete while having a bookmark highlighted would delete a tag from that bookmark?
This is such a far reach it almost sounds like an excuse to keep it the way it is.

You would be surprised how many times it happened.
As for the excuse part, I already said our tagging ui is horrible, and should be changed. But I also said we don't have the resources to do it now.

Here's an example of a well implemented tagging UI:
https://i.imgur.com/pMWTzAJ.png

Yes, this is a lot better and closer to what we should have.

Flags: needinfo?(mak)

How about two actions on the context menu:

  • Delete Bookmark
  • Remove Tag

With the caveat that if there are no bookmarks with the tag, that the tag will be removed from Tags.

In general, an action in a UI that affects more than one item, the user should be warned. Replace item with tag, bookmark, url.

Another direction...

I never looked at Tags as an editing mechanism (add, change, delete), but rather a list mechanism. Therefore, delete bookmark makes sense, really delete the bookmark, and the only way to delete the tag from a selected bookmark would be to edit the Tags field in the form (right, bottom).

If the tag is deleted (removed??) under Tags (left) it is removed from all bookmarks.

(In reply to prusselltechgroup from comment #17)

With the caveat that if there are no bookmarks with the tag, that the tag will be removed from Tags.

AFAIK if there are no bookmarks with a tag, the tag gets auto-deleted, so there's no need to worry about this scenario.

I do like your suggestion about having two actions.
It would compensate for this lack of functionality (a band aid as Meridel put it) until the tagging system rework referenced above takes place.
Still not sure what the semantic difference between "delete" and "remove" is.
To me they sound synonymous.

I'm not sure I follow your last two paragraphs.
What is meant by an editing and list mechanism?

Still not sure what the semantic difference between "delete" and "remove" is.

delete
delete the record; get rid of the whole
delete a bookmark from the places database

remove
remove an atrribute; take something away
remove a tag from a bookmark

By "mechanism" I'm just referring to a simple UI for adding, changing, deleting and listing/viewing any database data.

When I clicked a tag on the left side, I expected the bookmark list/view on the right to be filtered by the tag. Any action I could take on that list/view, I expected it to apply to a specific bookmark. The "Delete Bookmark" on the list/view context menu makes sense, if it really deleted the bookmark. The surprise was that it removed all tags from the bookmark.

If a user needs to delete a tag, use the context menu on the left side with the label "Delete Tag".
If a user only needs to remove a specific tag (or all) from a specific bookmark, use the form (bottom right).

The form on the bottom right is a little vague. There is nothing that indicate that the user can add, change, or delete just by typing in the field and clicking away. Sometimes I am not certain if this is the intended action or just happenstance.

Definitely renaming the context menu text to match the action ("Remove Tags") will clear things up. Adding a new menu item, which deletes the specific bookmark ("Delete Bookmark"), will improve bookmark management in the Library. Side Note: The Delete button on the keyboard will also delete the bookmark.

(In reply to prusselltechgroup from comment #19)

The surprise was that it removed all tags from the bookmark.

Yes that was pretty much my experience.
Didn't help that I only realized it many months into my tag usage, so it was all a mess, and I had to more or less start over.

Definitely renaming the context menu text to match the action ("Remove Tags") will clear things up. Adding a new menu item, which deletes the specific bookmark ("Delete Bookmark"), will improve bookmark management in the Library. Side Note: The Delete button on the keyboard will also delete the bookmark.

This sounds ideal.
Really if this were the way M handled this, then the only thing missing from the tagging system would be filtering by multiple tags (e.g. show me bookmarks with tags A, B, but not C).

Assignee: nobody → jsudiaman
Status: NEW → ASSIGNED
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/910edb644f13
Deleting bookmark from tag view doesn't delete bookmark, deletes tag instead (Context menu option is misleading) r=mak,fluent-reviewers

https://hg.mozilla.org/mozilla-central/rev/910edb644f13

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
status-firefox108: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 108 Branch
QA Whiteboard: [qa-108b-p2]

Reproduced this issue on Windows 10 using the STR from comment 0. The tag is deleted instead of the bookmark when clicking the Delete Bookmark option from the context menu.

Verified this issue as fixed on Firefox 108.0b5 (20221122190120) and 109.0a1 (20221123213526) across the following platforms: Windows 10 x64, macOS 10.15 and Ubuntu 22.04. The Delete Bookmark label was replaced with the Remove Tag label.

Status: RESOLVED → VERIFIED
status-firefox108: fixed → verified
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: