Closed
Bug 571129
Opened 14 years ago
Closed 14 years ago
contextMenuSearchText should have numbered arguments
Categories
(Firefox :: Search, defect)
Firefox
Search
Tracking
()
RESOLVED
FIXED
Firefox 4.0b1
People
(Reporter: tim.babych, Assigned: unghost)
Details
Attachments
(2 files, 2 obsolete files)
975 bytes,
patch
|
Pike
:
review+
|
Details | Diff | Splinter Review |
970 bytes,
patch
|
unghost
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•14 years ago
|
||
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
Assignee | ||
Comment 2•14 years ago
|
||
This patch works for me, though I have no idea if it's enough. Tim, can you give it a try?
Comment 3•14 years ago
|
||
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.)
Assignee | ||
Comment 4•14 years ago
|
||
(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.
Assignee | ||
Comment 5•14 years ago
|
||
With l10n note added.
Attachment #450439 -
Attachment is obsolete: true
Comment 6•14 years ago
|
||
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-
Comment 7•14 years ago
|
||
(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.
Assignee | ||
Comment 8•14 years ago
|
||
Without the key change.
Attachment #450475 -
Attachment is obsolete: true
Attachment #450641 -
Flags: review?(l10n)
Comment 9•14 years ago
|
||
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+
Assignee | ||
Comment 10•14 years ago
|
||
Patch for checkin with Pike's nits addressed.
Attachment #453432 -
Flags: review+
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → unghost
Status: NEW → ASSIGNED
Comment 11•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/bc91b4c4586f
Status: ASSIGNED → RESOLVED
Closed: 14 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.
Description
•