The bookmark menu item should always say "Bookmark", not "Remove"

VERIFIED FIXED in Firefox 11

Status

()

Firefox for Android
General
P2
normal
VERIFIED FIXED
6 years ago
11 months ago

People

(Reporter: madhava, Assigned: mfinkle)

Tracking

unspecified
Firefox 12
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox11 fixed, firefox12 fixed, fennec11+)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Right now, when a page is bookmarked, we change the bookmark icon to a filled-in star and change the label to "Remove" so that you can unbookmark. On ICS, though, we don't do icons in the menu, and we have a checked checkbox in the row (because the page is bookmarked, meaning that we get this nonsensical thing:

Remove   [x]

with no context.

Instead, it should read as


Bookmark    [x]

and tapping should uncheck (unbookmark).
Would it be out of the question to use "Bookmark" all the time (ICS and other phones)? I worry that "Remove" is too vague.
Or maybe it could change between "Bookmark [ ]" and "Bookmarked [x]" -- but I don't know how localizable that is.

Updated

6 years ago
Duplicate of this bug: 712745
Hardware: x86 → ARM
Morphing this bug to apply to all phones, not just ICS. Let's just always say Bookmark.

On ICS, we have a checkbox next to it, and on earlier phones we have a hollow star and a filled in star, which should be enough.
Summary: [ICS] On ICS, bookmark menu item should always say "Bookmark" → The bookmark menu item should always say "Bookmark"
Summary: The bookmark menu item should always say "Bookmark" → The bookmark menu item should always say "Bookmark", not "Remove"

Updated

6 years ago
status-firefox11: --- → affected
status-firefox12: --- → affected
Created attachment 586416 [details] [diff] [review]
patch

This patch:
* Removes the "bookmark_remove" string
* Stops changing the title of the menu item. It's always "Bookmark" now
* Changes the entity name of "bookmark_add" to just "bookmark"
* Removes the unused "bookmarks_title" string

Tested on Gingerbread and ICS. Added bookmark, removed bookmark and switched between tabs. The menu and the popup toast worked correctly on both.
Assignee: nobody → mark.finkle
Attachment #586416 - Flags: review?(mbrubeck)
Comment on attachment 586416 [details] [diff] [review]
patch

If reducing l10n churn is a concern at this point, we could keep the ID as "bookmark_add".
Attachment #586416 - Flags: review?(mbrubeck) → review+
there was enough string churn here I decided to go with the change for correctness
https://hg.mozilla.org/integration/mozilla-inbound/rev/13d57ec07b6e

Comment 8

6 years ago
https://hg.mozilla.org/mozilla-central/rev/13d57ec07b6e
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12
tracking-fennec: --- → 11+

Comment 9

6 years ago
We can't reuse the bookmarks_add string, as that's a verb, while bookmark is a noun now. Same characters in English, but rather different words :-).

String freeze is in a week from now so that we can fix things like this on aurora still.
Verified fix on 1-10-2011 build.
Status: RESOLVED → VERIFIED
Comment on attachment 586416 [details] [diff] [review]
patch

[Approval Request Comment]
UX wanted string change
Attachment #586416 - Flags: approval-mozilla-aurora?
Comment on attachment 586416 [details] [diff] [review]
patch

[Triage Comment]
Mobile only - approved for Aurora.
Attachment #586416 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/ffb813d220fb
status-firefox11: affected → fixed
status-firefox12: affected → fixed
You need to log in before you can comment on or make changes to this bug.