1360902 - Improve localization of netmonitor waterfall tooltip

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement)

Description

[Vangelis Katsikaros]

As mentioned in https://reviewboard.mozilla.org/r/130856/#review137874 joining localized strings with a hardcoded latin comma is not a good practive localization-wise.

Comment 1

[Francesco Lodolo [:flod]]

Thanks for filing the bug.

Is the tooltip always the same, i.e. does it always contain the same elements or they can change depending on the request/context? If it's the latter, it might become tricky to fix.

Comment 2

[Vangelis Katsikaros]

Attachment: Bug 1360902 - Improve localization of netmonitor waterfall tooltip.

Review commit: https://reviewboard.mozilla.org/r/135020/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/135020/

Comment 3

[Vangelis Katsikaros]

Comment on attachment 8863216 [details]
Bug 1360902 - Improve localization of netmonitor waterfall tooltip.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135020/diff/1-2/

Comment 4

[Vangelis Katsikaros]

Comment on attachment 8863216 [details]
Bug 1360902 - Improve localization of netmonitor waterfall tooltip.

https://reviewboard.mozilla.org/r/135020/#review137932

::: devtools/client/locales/en-US/netmonitor.properties:222
(Diff revision 2)
>  
>  # LOCALIZATION NOTE (networkMenu.totalMS): This is the label displayed
>  # in the network menu specifying the time for a request to finish (in milliseconds).
>  networkMenu.totalMS=→ %S ms
>  
> +# LOCALIZATION NOTE (netmonitor.waterfall.tooltip.separator): This is part of the tooltip

Added to give some context to translators

::: devtools/client/netmonitor/src/components/request-list-column-waterfall.js:101
(Diff revision 2)
>      tooltip.push(L10N.getFormatStr("netmonitor.waterfall.tooltip.total", totalTime));
>    }
>  
> -  return { boxes, tooltip: tooltip.join(", ") };
> +  return {
> +    boxes,
> +    tooltip: tooltip.join(L10N.getStr("netmonitor.waterfall.tooltip.separator"))

I imagine this will work well for RTL languages?

Example: changing netmonitor.waterfall.tooltip.separator to 'ARABIC COMMA' U+060C and a whitespace?

Comment 5

[Vangelis Katsikaros]

Yep, it's the later :( I tried to workaround the problem with the above patch.

Comment 6

[Francesco Lodolo [:flod]]

Comment on attachment 8863216 [details]
Bug 1360902 - Improve localization of netmonitor waterfall tooltip.

https://reviewboard.mozilla.org/r/135020/#review137934

Feel free to land with the updated comment

::: devtools/client/locales/en-US/netmonitor.properties:222
(Diff revision 2)
>  
>  # LOCALIZATION NOTE (networkMenu.totalMS): This is the label displayed
>  # in the network menu specifying the time for a request to finish (in milliseconds).
>  networkMenu.totalMS=→ %S ms
>  
> +# LOCALIZATION NOTE (netmonitor.waterfall.tooltip.separator): This is part of the tooltip

Thanks for the quick turn-around. If the tooltip itself is dynamic we don't have a better solution.

I would rephrase the comment like this, but the approach is good.

# This string is used to concatenate tooltips (netmonitor.waterfall.tooltip.*)
# in the requests waterfall for total time (in milliseconds). \\u0020 represents
# a whitespace. You can replace this with a different character, e.g. an hyphen
# or a period, if a comma doesn't work for your language.

Comment 7

[Vangelis Katsikaros]

Comment on attachment 8863216 [details]
Bug 1360902 - Improve localization of netmonitor waterfall tooltip.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135020/diff/2-3/

Comment 8

[Vangelis Katsikaros]

Thanks for the comments Francesco. I don't think I can land commits, only push to try. I pushed an update and just triggered a try.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a8322bcff72e
Improve localization of netmonitor waterfall tooltip. r=flod

Comment 10

[Francesco Lodolo [:flod]]

In case, you can set the checkin-need keyword in the bug, a sheriff will land it from mozreview. The try build seems green enough though.

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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