Last Comment Bug 710433 - The bookmark menu item should always say "Bookmark", not "Remove"
: The bookmark menu item should always say "Bookmark", not "Remove"
Status: VERIFIED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: P2 normal (vote)
: Firefox 12
Assigned To: Mark Finkle (:mfinkle) (use needinfo?)
:
: Sebastian Kaspari (:sebastian)
Mentors:
: 712745 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-13 15:13 PST by Madhava Enros [:madhava]
Modified: 2016-07-29 14:21 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
11+


Attachments
patch (5.21 KB, patch)
2012-01-06 07:09 PST, Mark Finkle (:mfinkle) (use needinfo?)
mbrubeck: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Madhava Enros [:madhava] 2011-12-13 15:13:55 PST
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).
Comment 1 Mark Finkle (:mfinkle) (use needinfo?) 2011-12-13 18:38:50 PST
Would it be out of the question to use "Bookmark" all the time (ICS and other phones)? I worry that "Remove" is too vague.
Comment 2 Matt Brubeck (:mbrubeck) 2011-12-13 21:09:20 PST
Or maybe it could change between "Bookmark [ ]" and "Bookmarked [x]" -- but I don't know how localizable that is.
Comment 3 Ian Barlow (:ibarlow) 2011-12-21 12:47:52 PST
*** Bug 712745 has been marked as a duplicate of this bug. ***
Comment 4 Ian Barlow (:ibarlow) 2012-01-04 11:11:24 PST
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.
Comment 5 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-06 07:09:50 PST
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.
Comment 6 Matt Brubeck (:mbrubeck) 2012-01-06 09:43:40 PST
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".
Comment 7 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-06 10:19:00 PST
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 Ed Morley [:emorley] 2012-01-06 15:46:39 PST
https://hg.mozilla.org/mozilla-central/rev/13d57ec07b6e
Comment 9 Axel Hecht [:Pike] 2012-01-10 04:20:25 PST
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.
Comment 10 Tony Chung [:tchung] 2012-01-10 11:36:10 PST
Verified fix on 1-10-2011 build.
Comment 11 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-10 12:53:16 PST
Comment on attachment 586416 [details] [diff] [review]
patch

[Approval Request Comment]
UX wanted string change
Comment 12 Alex Keybl [:akeybl] 2012-01-11 13:34:20 PST
Comment on attachment 586416 [details] [diff] [review]
patch

[Triage Comment]
Mobile only - approved for Aurora.
Comment 13 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-11 21:02:25 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/ffb813d220fb

Note You need to log in before you can comment on or make changes to this bug.