1313196 - Add L10N support for the new Debugger

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P1)

Comment 1

[Jason Laster [:jlast]]

Attachment: l10n2.patch

Comment 2

[Jason Laster [:jlast]]

devtools/client/debugger/new/test/mochitest/browser_dbg-scopes.js

Comment 3

[James Long (:jlongster)]

Comment on attachment 8805179 [details] [diff] [review]
l10n2.patch

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

Awesome! A few nits, but will go ahead and r+.

::: devtools/client/debugger/new/panel.js
@@ +11,4 @@
>  
>  function DebuggerPanel(iframeWindow, toolbox) {
>    this.panelWin = iframeWindow;
> +  this.panelWin.L10N = L10N;

Are we 100% sure that we don't try to use the L10N APIs until the app is bootstrapped? Just making sure. I think that's right, but wanted to point out that the iframe could have already loaded by this point and JS executed. Just need to make sure the JS that could be executed by this point doesn't use L10N. If it's only our UI, we don't render that until we call `bootstrap`, so I think it should be OK.

::: devtools/client/locales/en-US/debugger.properties
@@ +203,5 @@
> +
> +# LOCALIZATION NOTE (sources.search): Sources left sidebar prompt
> +# e.g. Cmd+P to search. On a mac, we use the command unicode character.
> +# On windows, it's ctrl.
> +sources.search=%S+P to search

Not sure we want to encode the key in here. Could we just do `%S to search` instead? Then we can embed the right keycode elsewhere and it'll be separate. (updating the key shouldn't require re-localization)

@@ +212,5 @@
> +
> +# LOCALIZATION NOTE (welcome.search): The center pane welcome panel's
> +# search prompt. e.g. cmd+p to search for files. On windows, it's ctrl, on
> +# a mac we use the unicode character.
> +welcome.search=%S+P to search for files

ditto

Comment 4

[Jason Laster [:jlast]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b22146164cf1e1b4c5e7b65c757ce60b54bbdba

Comment 5

[Jason Laster [:jlast]]

> Are we 100% sure that we don't try to use the L10N APIs until the app is bootstrapped? 

I believe this is a good assumption for now. We might need to change this if an edge case emerges.

Good call on the search command

Comment 6

[Jason Laster [:jlast]]

Attachment: l10n3.patch

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/182f24158aad
Add L10N support for the new Debugger. r=jlongster

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/a89bd4876d4d
Add L10N support for the new Debugger: use ellipsis instead of three full stops. r=maketesthappy

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/182f24158aad
https://hg.mozilla.org/mozilla-central/rev/a89bd4876d4d

Comment 10

[Francesco Lodolo [:flod]]

debugger.properties had about 90 strings before these landings. Are the old strings still used somehow?

Comment 11

[Francesco Lodolo [:flod]]

I should have written the whole comment before sending, sorry for the double email.

editor.searchResults = %d of %d results

Do we have plural support? It's also the first time I see %d used in a string. Can the order be safely swapped?

Comment 12

[Francesco Lodolo [:flod]]

More notes after a quick test:
* There are hard-coded strings: "Loading..." when clicking a file, "No Breakpoints" in the sidebar.
* Several tooltips in the sidebar show %S, and there are hard-coded strings ("Disable breakpoints").

Are there follow-up discussions?

Comment 13

[Jason Laster [:jlast]]

Hi Francesco, thanks for the comments.

>  debugger.properties had about 90 strings before these landings

They're still used in the old debugger

> editor.searchResults = %d of %d results

I don't think they can be swapped. An example of this is "1 of 5 results", we don't show "1 or 1 results".

> There are hard-coded strings

Thanks will fix here https://github.com/devtools-html/debugger.html/issues/1143

Comment 14

[Francesco Lodolo [:flod]]

(In reply to Jason Laster [:jlast] from comment #13)
> > editor.searchResults = %d of %d results
> 
> I don't think they can be swapped. An example of this is "1 of 5 results",
> we don't show "1 or 1 results".

Let me rephrase the question: is it possible for a locale to invert them? Because they might just need to.

In Mozilla's .properties files you would have "%S of %S results", and you could implicitly switch to "Total results %2$S, showing number %2$S" (made up example, but just to explain the logic).

Comment 15

[Jason Laster [:jlast]]

Hmm, I can switch to %S.

I'm not sure what this logic is:
Total results %2$S, showing number %2$S"

Are there docs for this?

Comment 16

[Francesco Lodolo [:flod]]

I don't know how you parse strings at the moment, but that's the standard with stringbundles
https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIStringBundle
https://dxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/stringbundle.xml#31

Comment 17

[Axel Hecht [:Pike]]

I dug into this a bit a while ago, there's sprintf.js in the source, the readme at https://github.com/juliandescottes/sprintf.js has the supported syntax. Julian forked that to explicitly support capital S as formatter to be more inline with what we do in the platform.

And yes, they're supporting positioned params, too.