1232438 - Show edit bookmark dialog when user clicks on bookmark star for bookmarked page

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, defect)

Description

[:Margaret Leibovic]

Instead of just un-bookmarking the page, we should show the edit bookmark dialog. This is similar to how desktop Firefox behaves.

However, this means we'll also want to add a "Remove" button of some sort to the edit bookmark dialog.

Comment 1

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_bookmarks_bookmarkmgmt_L2_4.png

Attaching mock

Comment 2

[Richard Newman [:rnewman]]

It's worth remembering that the same URL can be bookmarked multiple times with the same or different title and the same or different parent folder.

We have quietly ignored this so far, defining the star's behavior as:

* Filled if this URL is bookmarked at least once, empty otherwise.
* On tap:
  * If not bookmarked, create one in Mobile Bookmarks.
  * If bookmarked, delete all bookmarks with this URL.

If we introduce editing, we can no longer ignore this.

We now have to address *which bookmark* you're editing:

- Which title should we take?
- Should we delete one or all when you hit Remove?
- If you change the URL, does it change the URL for all of them?
- If you move the bookmark, do we move all of the duplicates into the same place?

It's not enough to declare that we'll only allow editing mobile bookmarks: they can still be duplicated, because desktop allows it and Sync can screw it up.

It's probably not enough to declare that we'll edit the first: that's not really deterministic, and it can result in surprising behavior (e.g., you hit Remove, and the star still shows that it's bookmarked! *sinking feeling*).


The easiest approach is to not allow you to edit from the star: you have to go to the bookmarks panel and pick a single bookmark.

Next simplest is to show a stacked edit dialog for every instance of the bookmark, assuming that we'll have two or three in most cases:

|---------------------------------|
| ✉                               |
| /in Browser Toolbar/     [move] |
| https://mail.google.com/        |
|                      [ Remove ] |
===================================
|---------------------------------|
| Gmail                           |
| /in Unsorted Bookmarks/  [move] |
| https://mail.google.com/        |
|                      [ Remove ] |
-----------------------------------

More complicated is to support searching bookmarks, and to have the star dump you into a search results pane showing all matching bookmarks. From there you can edit as you wish.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

What does desktop do?

Comment 4

[Richard Newman [:rnewman]]

It shows "Remove 2 bookmarks" as a button at the top, and picks one of them to edit, showing that instance's title and folder.

Comment 5

[Richard Newman [:rnewman]]

(IMO the desktop version would benefit from <|> arrows to step through, or a "Show me" option.)

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Richard Newman [:rnewman] from comment #4)
> It shows "Remove 2 bookmarks" as a button at the top, and picks one of them
> to edit, showing that instance's title and folder.

So unbookmarking a multi-instance URL will remove all occurrences of the bookmarked URL. The edit dialog appears to be editing the most recently bookmarked entry.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Richard Newman [:rnewman] from comment #5)
> (IMO the desktop version would benefit from <|> arrows to step through, or a
> "Show me" option.)

This also has the potential to be complicated without the context of the folder locations.

Comment 8

[Richard Newman [:rnewman]]

> This also has the potential to be complicated without the context of the folder locations.

The edit dialog on desktop (and in Anthony's mock) has a dropdown for the folder, so there's context there. I agree that the context is important.

Bookmarks are complex, and there's only so much we can do to paper over that. In some places the sharp bits poke out, and ignoring it doesn't help.


> So unbookmarking a multi-instance URL will remove all occurrences of the
> bookmarked URL.

Yup.

Labeling the button "Remove N" is better than ignoring the situation altogether, but good UX offers reversible actions, and that's not the case here: you added this page to Mobile Bookmarks on your phone, and now it's not obvious how to remove it without, say, also removing it from your desktop toolbar.

It helps that the removal button gives a clue that there's more than one bookmark, because at least a user is less likely to remove more than one bookmark by accident.

However, unless the user's task is to totally remove that site from their bookmarks, desktop's dialog doesn't really move them closer to completing their task -- which here can be categorization and renaming, not just deletion.

I expect that the justification for this simple approach is that the main way to create dupes is drag-and-drop on desktop; if your only interaction with bookmarks is via the star, you'll never have dupes anyway, so why bother addressing that?

I take a different viewpoint for three reasons:

1. Desktop lets you search your bookmarks, and has desktop-level management. We don't really have a management UI, so the star is the main entry point for management.

2. If you're seeing this edge-case UX at all, you're not a desktop star-only user -- you've definitely created a bookmark some other way -- and if we go for the blunt approach we'll probably disappoint you. We don't need to go crazy, but I think it's reasonable to offer a way to manage multiple bookmarks in the multiple-bookmark case, even if that way is an arrow pointing somewhere else in the app.

3. If you have dupes of a bookmark, it seems to me like the thing you're most likely to want to do is delete _the dupes_, not _all copies_. It feels really scary to hit "Remove all" then re-bookmark, but that's the only way to do it.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Richard Newman [:rnewman] from comment #8)
> > This also has the potential to be complicated without the context of the folder locations.
> 
> The edit dialog on desktop (and in Anthony's mock) has a dropdown for the
> folder, so there's context there. I agree that the context is important.
> 
> Bookmarks are complex, and there's only so much we can do to paper over
> that. In some places the sharp bits poke out, and ignoring it doesn't help.

Adding complex UI doesn't help either. I don't think we have a clear idea of who we are designing this system for, so I would lean to the less-advanced set of users as a start. We should do some UR to figure out what additional edge cases we want to smooth out.

> > So unbookmarking a multi-instance URL will remove all occurrences of the
> > bookmarked URL.
> 
> Yup.
> 
> Labeling the button "Remove N" is better than ignoring the situation
> altogether, but good UX offers reversible actions, and that's not the case
> here: you added this page to Mobile Bookmarks on your phone, and now it's
> not obvious how to remove it without, say, also removing it from your
> desktop toolbar.

Going against my own point of keeping it simple...

I don't think we have a good way to Undo the removal action, so we should get a confirmation prompt. When getting the confirmation prompt, we could tell the user when more than one bookmark will be removed. In those cases, we could show a checkbox list of the parent folder names where the bookmark URL lives. The prompt would also have text/copy to let the user know that they are potentially removing more than one bookmark.

This could be a followup.

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

Part of my reasoning in comment 9 is to keep the UX simple for the common cases where there is not more than one bookmark for a given URL. Only add complexity for people who need it.

Comment 11

[Richard Newman [:rnewman]]

(In reply to Mark Finkle (:mfinkle) from comment #10)
> Part of my reasoning in comment 9 is to keep the UX simple for the common
> cases where there is not more than one bookmark for a given URL. Only add
> complexity for people who need it.

Yes, exactly. In those cases I'd just show the simple case: Anthony's mocks.

For the complex case, I'd be inclined to do something like desktop, but keep the editing greyed out and have a [Show] button alongside [Remove N]. Presumably we'll be adding bookmark search at some point -- it's not difficult -- and dropping them into a search by URL lets them work with the individual bookmarks that match with little chance of screwing up off the bat.

Comment 15

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.