1241024 - Make about:performance localizable

Status: VERIFIED FIXED

(Toolkit :: Performance Monitoring, defect)

Description

[Francesco Lodolo [:flod]]

I think there's value for tech-savvy users in this page, especially if it rides the train and it's exposed outside of the Nightly channel.

We should make this page localizable before it starts riding the trains.

Comment 1

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.

Comment 2

[Florian Quèze [:florian]]

Attachment: Patch

This localizes the strings of the new about:performance (which is now enabled by default). I didn't try to localize the strings of the old about:performance as we intend to remove it soon.

Comment 3

[Florian Quèze [:florian]]

Hey Bryan, could you please check that all the strings included in the aboutPerformance.ftl file in the patch here look correct for Firefox 64? Thanks!

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 9018028 [details] [diff] [review]
Patch

Review of attachment 9018028 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/aboutperformance/content/aboutPerformance.js
@@ +1208,5 @@
>          }
> +
> +        // If there's an l10n id on our <td> node, any image we add will be
> +        // removed during localization, so move the l10n id to a <span>
> +        let l10nId = elt.getAttribute("data-l10n-id");

We provide `document.l10n.getAttributes` which gets you id/args and is a bit more forward compatible. Not a big deal if you prefer your current code. :)

Comment 5

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 9018028 [details] [diff] [review]
Patch

Review of attachment 9018028 [details] [diff] [review]:
-----------------------------------------------------------------

(I don't think I need to do a full review, so I'll limit myself to feedback+)

Comment 6

[Francesco Lodolo [:flod]]

Comment on attachment 9018028 [details] [diff] [review]
Patch

Review of attachment 9018028 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/locales/en-US/toolkit/about/aboutPerformance.ftl
@@ +1,5 @@
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +## Page title

This comment refers to a single string, so it should be a string level comment (one #).

@@ +37,5 @@
> +## Tooltips for the action buttons
> +close-tab =
> +    .title = Close tab
> +show-addon =
> +    .title = Show in add-on manager

I believe it's usually called `Add-ons Manager`.

@@ +39,5 @@
> +    .title = Close tab
> +show-addon =
> +    .title = Show in add-on manager
> +
> +## Tooltip when hovering an item of the about:performance table

Same here, one #

@@ +47,5 @@
> +#   $dispatchesSincePrevious (Number) - how many dispatches occured in the last 2 seconds
> +#   $durationSincePrevious (Number) - how much CPU time was used in the last 2 seconds
> +item =
> +    .title =
> +        { $totalDispatches } ({ $totalDuration }ms) dispatches since load

This requires a plural form, since you have "dispatches". Also, is the new line meaningful?

You can reword this to avoid a plural form (which seems clear enough)

```
item =
    .title =
        Dispatches since load: { $totalDispatches } ({ $totalDuration }ms)
        Dispatches in the last seconds: { $dispatchesSincePrevious } ({ $durationSincePrevious }ms)
```

Or use a plural form (without repeating "dispatches" in the second sentence, because the plural depends on a different number, and it's currently poorly supported in Pontoon).

```
item =
    .title =
        { $totalDispatches ->
            [one] { $totalDispatches } ({ $totalDuration }ms) dispatch since load
                  { $dispatchesSincePrevious } ({ $durationSincePrevious }ms) in the last seconds
           *[other] { $totalDispatches } ({ $totalDuration }ms) dispatches since load
                  { $dispatchesSincePrevious } ({ $durationSincePrevious }ms) in the last seconds
        }

```

Comment 7

[Florian Quèze [:florian]]

Attachment: Patch v2

The line break in the tooltip is intentional. I like your suggestion that doesn't require a plural form, thanks!

Comment 8

[Francesco Lodolo [:flod]]

Comment on attachment 9018314 [details] [diff] [review]
Patch v2

Review of attachment 9018314 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a6b1ba0f53b
Make about:performance localizable, r=gandalf,flod.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6669d853e366
follow-up to fix eslint failure, rs=bustage-fix.

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/9a6b1ba0f53b
https://hg.mozilla.org/mozilla-central/rev/6669d853e366

Comment 12

[Francesco Lodolo [:flod]]

Looks good (64.0a1 (2018-10-19) (64 bit))