Closed Bug 1313196 Opened 3 years ago Closed 3 years ago

Add L10N support for the new Debugger

Categories

(DevTools :: Debugger, defect, P1)

defect

Tracking

(firefox52 fixed)

RESOLVED FIXED
Firefox 52
Tracking Status
firefox52 --- fixed

People

(Reporter: jlast, Assigned: jlast)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Assignee: nobody → jlaster
Blocks: 1300861
Priority: -- → P1
Attached patch l10n2.patch (obsolete) — Splinter Review
Attachment #8805179 - Flags: review?(jlong)
devtools/client/debugger/new/test/mochitest/browser_dbg-scopes.js
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
Attachment #8805179 - Flags: review?(jlong) → review+
> 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
Attached patch l10n3.patchSplinter Review
Attachment #8805179 - Attachment is obsolete: true
Pushed by jlong@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/182f24158aad
Add L10N support for the new Debugger. r=jlongster
Pushed by archaeopteryx@coole-files.de:
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
https://hg.mozilla.org/mozilla-central/rev/182f24158aad
https://hg.mozilla.org/mozilla-central/rev/a89bd4876d4d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
debugger.properties had about 90 strings before these landings. Are the old strings still used somehow?
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?
Flags: needinfo?(jlaster)
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?
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
Flags: needinfo?(jlaster)
(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).
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?
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.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.