Make about:performance localizable

VERIFIED FIXED in Firefox 64

Status

()

defect
VERIFIED FIXED
4 years ago
8 months ago

People

(Reporter: flod, Assigned: florian, NeedInfo)

Tracking

unspecified
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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

Last year
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.
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → INACTIVE
Status: RESOLVED → REOPENED
Resolution: INACTIVE → ---
Assignee

Comment 2

8 months ago
Posted patch Patch (obsolete) — Splinter Review
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.
Attachment #9018028 - Flags: review?(felipc)
Assignee

Updated

8 months ago
Assignee: nobody → florian
Status: REOPENED → ASSIGNED
Assignee

Comment 3

8 months ago
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!
Flags: needinfo?(bbell)
Assignee

Updated

8 months ago
Attachment #9018028 - Flags: review?(francesco.lodolo)
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. :)
Attachment #9018028 - Flags: review+
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+)
Attachment #9018028 - Flags: review?(felipc) → feedback+
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
        }

```
Attachment #9018028 - Flags: review?(francesco.lodolo) → review-
Assignee

Comment 7

8 months ago
Posted patch Patch v2Splinter Review
The line break in the tooltip is intentional. I like your suggestion that doesn't require a plural form, thanks!
Attachment #9018314 - Flags: review?(francesco.lodolo)
Assignee

Updated

8 months ago
Attachment #9018028 - Attachment is obsolete: true
Comment on attachment 9018314 [details] [diff] [review]
Patch v2

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

Looks good, thanks.
Attachment #9018314 - Flags: review?(francesco.lodolo) → review+

Comment 9

8 months ago
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a6b1ba0f53b
Make about:performance localizable, r=gandalf,flod.

Comment 10

8 months ago
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6669d853e366
follow-up to fix eslint failure, rs=bustage-fix.

Comment 11

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9a6b1ba0f53b
https://hg.mozilla.org/mozilla-central/rev/6669d853e366
Status: ASSIGNED → RESOLVED
Closed: Last year8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Looks good (64.0a1 (2018-10-19) (64 bit))
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.