Closed Bug 1043896 Opened 10 years ago Closed 9 years ago

Use proper plural form for highlightOutputConfirm (gclicommands.properties)

Categories

(DevTools :: Inspector, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 38

People

(Reporter: flod, Assigned: lviknesh, Mentored)

References

Details

(Whiteboard: [good first bug][lang=js])

Attachments

(1 file, 3 obsolete files)

Mentor: jwalker
Whiteboard: [good first bug][lang=js]
Could you please assign me this bug?
Assignee: nobody → sushrutbhalla
Attached patch plurals.patch (obsolete) — Splinter Review
Just started with this . i haven't tested the patch . Just want to know whether i am in right track :)
Assignee: sushrutbhalla → lviknesh
Status: NEW → ASSIGNED
Attachment #8498463 - Flags: feedback?(jwalker)
Comment on attachment 8498463 [details] [diff] [review]
plurals.patch

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

::: browser/locales/en-US/chrome/browser/devtools/gclicommands.properties
@@ +215,5 @@
>  # LOCALIZATION NOTE (highlightOutputConfirm) A confirmation message for the
>  # 'highlight' command, displayed to the user once the command has been entered,
>  # informing the user how many nodes have been highlighted successfully and how
>  # to turn highlighting off
> +highlightOutputConfirm=%1$S node highlighted;%1$S nodes highlighted

You need a new string ID

https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_best_practices#Changing_existing_strings
Comment on attachment 8498463 [details] [diff] [review]
plurals.patch

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

Thanks for the patch. Flod is right - r+ with that.
Attachment #8498463 - Flags: feedback?(jwalker)
Attached patch plural-form.patch (obsolete) — Splinter Review
Updated with new String ID :)
Attachment #8498463 - Attachment is obsolete: true
Attachment #8498916 - Flags: review?(jwalker)
Comment on attachment 8498916 [details] [diff] [review]
plural-form.patch

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

Thanks
Attachment #8498916 - Flags: review?(jwalker) → review+
Patrick, please can I put this on your queue to get this landed?
Flags: needinfo?(pbrosset)
I wasn't sure if a try run was necessary, anyway, here's one: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9208bb87d00b
I'll push this patch to fx-team in a bit.
Flags: needinfo?(pbrosset)
Turns out pushing to try was a good idea, a test is consistently failing on all platforms because of the changes in this patch.
Viknesh: see test browser/devtools/commandline/test/browser_cmd_highlight_01.js 
The fix for this test seems to be pretty straightforward, the expected output needs to be changed (e.g. "1 nodes highlighted" should be changed to "1 node highlighted").

Once fixed, you can run the test locally with './mach mochitest-devtools browser/devtools/commandline/test" to see if it now works.
Attached patch plurals.patch (obsolete) — Splinter Review
updated as per the previous comment . error http://pastebin.com/pf2izFfs
Attachment #8498916 - Attachment is obsolete: true
Patrick, looks like this ball's in your court.
Flags: needinfo?(pbrosset)
Comment on attachment 8506227 [details] [diff] [review]
plurals.patch

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

Sorry for the delay, the new patch was flagged for review, so I missed it.
Anyway, looks good to me.
Attachment #8506227 - Flags: review+
Attached patch plurals.patchSplinter Review
Rebased that patch and pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ba91e596cee
Attachment #8506227 - Attachment is obsolete: true
Flags: needinfo?(pbrosset)
Attachment #8556547 - Flags: review+
https://hg.mozilla.org/integration/fx-team/rev/0cbdc6892176
Keywords: checkin-needed
Whiteboard: [good first bug][lang=js] → [good first bug][lang=js][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/0cbdc6892176
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [good first bug][lang=js][fixed-in-fx-team] → [good first bug][lang=js]
Target Milestone: --- → Firefox 38
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: