Closed Bug 571129 Opened 11 years ago Closed 11 years ago

contextMenuSearchText should have numbered arguments

Categories

(Firefox :: Search, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 4.0b1

People

(Reporter: tim.babych, Assigned: unghost)

Details

Attachments

(2 files, 2 obsolete files)

No description provided.
Currently browser/chrome/browser/browser.properties has 
contextMenuSearchText=Search %S for "%S"

For proper localization into Ukrainian, Russian and surely some other languages, the position of search engine name and query should be swapped.

Please make it possible.
Summary: browser.properties → contextMenuSearchText should have numbered arguments
Attached patch Potential patch (obsolete) — Splinter Review
This patch works for me, though I have no idea if it's enough. Tim, can you give it a try?
Why does en-US string need to change? Can't you just use the positional arguments in your localization?

(Adding the positional arguments in the en-US makes it clearer which string is which when you're translating, so it may be worth doing anyways, but I don't think it's necessary to get the result you need.)
(In reply to comment #3)
> Why does en-US string need to change? Can't you just use the positional
> arguments in your localization?
Do you mean that en-US can use "%S" and "%S" and localizations can use "%1$S" and "%2$S" instead? I have no idea that it's supported and safe to use (is it documented somewhere?).
Besides, I'm not sure that compare-locales and l10n dashboard support it.
Attached patch Potential patch v.2 (obsolete) — Splinter Review
With l10n note added.
Attachment #450439 - Attachment is obsolete: true
Comment on attachment 450475 [details] [diff] [review]
Potential patch v.2

Adding the numbers is fine as a hint, and adding a localization note would be really good.

But, "%S foo %S" and "%1$S foo %2$S" are totally equivalent, and you can use positional arguments in your localization independently of whether en-US uses them or not.

Once compare-locales and the dashboard will support param checks, they will know about this equivalence and deal with it rightly. As in, the not yet ready code I have locally does deal with it OK.

I'd welcome a patch with just the note and the positions, but without the key change, personally.
Attachment #450475 - Flags: review-
(In reply to comment #6)
> Once compare-locales and the dashboard will support param checks, they will
> know about this equivalence and deal with it rightly. As in, the not yet ready
> code I have locally does deal with it OK.

For what it's worth, we (it, Italian) have been using positional arguments in this string for years without problems with compare-locales.
Attached patch Patch v.3Splinter Review
Without the key change.
Attachment #450475 - Attachment is obsolete: true
Attachment #450641 - Flags: review?(l10n)
Comment on attachment 450641 [details] [diff] [review]
Patch v.3

r=me with a nit, I'd reference %1$S instead of 1st string, %2$S instead of 2nd explicitly, as first and second become confusing for locales that chnage the order.
Attachment #450641 - Flags: review?(l10n) → review+
Patch for checkin with Pike's nits addressed.
Attachment #453432 - Flags: review+
Keywords: checkin-needed
Assignee: nobody → unghost
Status: NEW → ASSIGNED
http://hg.mozilla.org/mozilla-central/rev/bc91b4c4586f
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a6
You need to log in before you can comment on or make changes to this bug.