aboutdebugging: Add localization comments to aboutdebugging.properties

RESOLVED FIXED in Firefox 50



Developer Tools: about:debugging
a year ago
a year ago


(Reporter: jdescottes, Assigned: dalimil, Mentored)


Firefox 50

Firefox Tracking Flags

(firefox49 affected, firefox50 fixed)


(Whiteboard: [good first bug])


(1 attachment, 1 obsolete attachment)

3.71 KB, patch
: review+
Details | Diff | Splinter Review


a year ago
At the moment all strings localized through aboutdebugging.properties are missing comments. This will make it harder for localization teams to handle the translation.

See https://dxr.mozilla.org/mozilla-central/source/devtools/client/locales/en-US/aboutdebugging.properties

Comment 1

a year ago
Jan: Just want to confirm we're on the same page here. Is there a reason why we don't have localization notes for now?
Flags: needinfo?(janx)
Julian, I agree that we should have localization comments if they're helpful to localizers.

They're absent from aboutdebugging.properties simply because we forgot them originally, and new contributions stayed consistent with that.

Pike, do you have any guidelines on when/how to write useful localization notes next to our strings?
Flags: needinfo?(janx) → needinfo?(l10n)


a year ago
Priority: -- → P3
You can find some information in this page (together with a lot of other useful information about l10n)

about:debugging is a very self-contained window, so it's quite easy to figure out where strings go through testing.

The only string that really needs a comment is doesNotExist, to explain what the variable is, and maybe the first three strings to explain that they are button labels.
Flags: needinfo?(l10n)
Thanks Francesco! Julian, are you interested in mentoring this bug e.g. as a [good first bug]?
Flags: needinfo?(jdescottes)

Comment 5

a year ago
(In reply to Jan Keromnes [:janx] from comment #4)
> Thanks Francesco! Julian, are you interested in mentoring this bug e.g. as a
> [good first bug]?

Sure! As explained in the comments above the goal of this bug will be to add localization notes to some of the keys defined in: devtools/client/locales/en-US/aboutdebugging.properties. 

Keys that should be documented in priority:
- doesNotExist: error message displayed when navigating to an invalid aboutdebugging page (the variable is the invalid page name) 
- debug: label of the button that starts debugging a service worker 
- push: label of the button that pushes a test payload to a service worker
- start: label of the button that starts a service worker

The usual syntax used for localization comments can be found in other properties files from the same folder, such as animationinspector.properties.

Documentation for contributors can be found at: https://wiki.mozilla.org/DevTools/Hacking
Mentor: jdescottes@mozilla.com
Flags: needinfo?(jdescottes)
Whiteboard: [good first bug]

Comment 6

a year ago
Created attachment 8769482 [details] [diff] [review]

I added the localization strings
Assignee: nobody → dalimilhajek
Attachment #8769482 - Flags: review?(jdescottes)

Comment 7

a year ago
Comment on attachment 8769482 [details] [diff] [review]

Review of attachment 8769482 [details] [diff] [review]:

Thanks a lot for working on this Dalimil! 

The changeset looks good, hopefully I can convince you to document two other strings in the file, which feel a bit ambiguous to me without any description.
Let me know if you need help with any of them!

Also an overall nit: can you add a dot at the end of each documentation sentence?

Finally when you amend your commit, can you update the message so that it ends with r=jdescottes (no need for "Julian Descottes [:jdescottes]").


::: devtools/client/locales/en-US/aboutdebugging.properties
@@ +2,5 @@
>  # 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/.
> +# This string is displayed as a label of the button that starts 

nit: remove the trailing space here.

@@ +50,2 @@
>  extensions = Extensions
>  selectAddonFromFile2 = Select Manifest File or Package (.xpi)

Let's also document this one: this is the title of the file picker displayed when the user clicks on "load temporary add on"

@@ +55,4 @@
>  reload = Reload
> +
> +# LOCALIZATION NOTE (reloadDisabledTooltip):
> +# This string is displayed in a tooltip that appears when hovering over a 

nit: remove the trailing space

@@ +72,3 @@
>  tabs = Tabs
>  pageNotFound = Page not found

Let's document this one as well since it's directly connected to the next one. This one is the main message displayed on any error/invalid page.
Attachment #8769482 - Flags: review?(jdescottes) → feedback+

Comment 8

a year ago
Created attachment 8770023 [details] [diff] [review]

OK, fixed.
Attachment #8769482 - Attachment is obsolete: true
Attachment #8770023 - Flags: review?(jdescottes)

Comment 9

a year ago
Comment on attachment 8770023 [details] [diff] [review]

Review of attachment 8770023 [details] [diff] [review]:

Thanks for addressing the nits, the patch is ready to go.

I'll add the checkin-needed flag so that it can be picked up by one of the sheriffs, who will land it on fx-team.
In case you want to find other devtools bugs, you can have a look at http://firefox-dev.tools/ :)
Attachment #8770023 - Flags: review?(jdescottes) → review+


a year ago
Keywords: checkin-needed

Comment 10

a year ago
Pushed by cbook@mozilla.com:
aboutdebugging: Add localization comments to aboutdebugging.properties. r=jdescottes
Keywords: checkin-needed

Comment 11

a year ago
Last Resolved: a year ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
You need to log in before you can comment on or make changes to this bug.