1080690 - Strings for bug 1080406

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: update-badge-strings.patch

Here is a patch with just the string changes from bug 1080406. Hopefully we can get this in before uplift.

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Strings for an optional badge to the hamburger menu when updates are available v2

Incorporated your comment from bug 1080406. Is that OK and can you find someone who can review this in due time?

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8503590 [details] [diff] [review]
Strings for an optional badge to the hamburger menu when updates are available v2

>+# LOCALIZATION NOTE(appmenu.*.tooltip): these are used for the appmenu
>+# tooltip when an update is staged for installation or background update has
>+# failed and a manual download is required.
>+# %S is brandShortName
>+appmenu.restart.tooltip = Open menu\n(restart %S to apply updates)
>+appmenu.failed.tooltip = Open menu\n(backgroud update failed, please download update)

typo: backgroud

>+# LOCALIZATION NOTE(appmenu.*.label): these are used for the appmenu
>+# button labels that carry out the actions above.
>+# %S is brandShortName
>+appmenu.restart.label = Restart %S
>+appmenu.failed.label = Download update

1. Picking self-explanatory string names is preferable over adding localization notes. appmenu.failed.tooltip and appmenu.failed.label don't even make sense, if anything they suggest that the menu failed.

2. If the menu itself provides the details and the response to the badge (as it should), then I don't think the tooltip needs to change. Except that your proposed tooltip is actually more informative than the thing you want to add in the menu, which seems backwards.

3. Generally, can we please land this when it's ready and not rush in strings for an UI that's yet to be defined?

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Strings for an optional badge to the hamburger menu when updates are available v3

(In reply to Dão Gottwald [:dao] from comment #2)

> typo: backgroud

Thanks, fixed.

> 1. Picking self-explanatory string names is preferable over adding
> localization notes. appmenu.failed.tooltip and appmenu.failed.label don't
> even make sense, if anything they suggest that the menu failed.

Done.

> 2. If the menu itself provides the details and the response to the badge (as
> it should), then I don't think the tooltip needs to change. Except that your
> proposed tooltip is actually more informative than the thing you want to add
> in the menu, which seems backwards.

Done.

> 3. Generally, can we please land this when it's ready and not rush in
> strings for an UI that's yet to be defined?

I wouldn't mind that, as it would spare me working past midnight and at weekends, but sometimes we need to make some hard choices. If you really feel that way, that's a discussion you need to have with dcamp.

Comment 4

[Joe Walker [:jwalker]]

Comment on attachment 8502607 [details] [diff] [review]
update-badge-strings.patch

Still happy for ux input but right now doing the best we can for l10n with tight deadlines is the right thing to do.

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 8503596 [details] [diff] [review]
Strings for an optional badge to the hamburger menu when updates are available v3

If (some of?) these labels are for buttons, they need to use title capitalization. Which of these labels are for buttons isn't clear to me, though, as your l10n note doesn't clarify that and since it's generally nebulous how the UI will eventually look...

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Strings for an optional badge to the hamburger menu when updates are available v4

OK, better now?

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 8503609 [details] [diff] [review]
Strings for an optional badge to the hamburger menu when updates are available v4

>+# LOCALIZATION NOTE(appmenu.*.label): these are used for the appmenu labels and
>+# buttons that appear when an update is staged for installation or a background
>+# update has failed and a manual download is required.
>+# %S is brandShortName
>+appmenu.restartNeededLabel.label = Restart %S to apply updates
>+appmenu.updateFailedLabel.label = Background update failed, please download update

Better:

appmenu.restartNeeded.description
appmenu.updateFailed.description

You'll need to update the l10n note for that, specifically the "appmenu.*.label" reference.

>+appmenu.restartBrowserButton.label = Restart %S

Hard to say without a UI spec, but I suspect that if the description already says "Restart %S ...", we might want to use "Restart Now" or just "Restart" for the button? Though then again if you don't read the description carefully, you could think clicking the button would restart your computer...

Comment 8

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Strings for an optional badge to the hamburger menu when updates are available v5

Thanks for the review, I had the exact same thoughts about a plain "Restart" label!

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 8503610 [details] [diff] [review]
Strings for an optional badge to the hamburger menu when updates are available v5

>+appmenu.restartNeededLabel.description = Restart %S to apply updates
>+appmenu.updateFailedLabel.description = Background update failed, please download update

remove "Label" from these names

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Strings for an optional badge to the hamburger menu when updates are available v6

Oops!

Comment 11

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/integration/fx-team/rev/f216320d8538

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f216320d8538