Closed
Bug 1185845
Opened 9 years ago
Closed 9 years ago
Fix localization issues in one-off searches: remove contatenations, "Change Search Settings" is hard-coded
Categories
(Firefox :: Search, defect)
Firefox
Search
Tracking
()
RESOLVED
FIXED
Firefox 42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: flod, Assigned: nhnt11)
References
Details
Attachments
(1 file, 1 obsolete file)
5.61 KB,
patch
|
adw
:
review+
flod
:
feedback+
|
Details | Diff | Splinter Review |
This is a landing from bug 1171344 http://hg.mozilla.org/mozilla-central/diff/b2a96402ed45/browser/locales/en-US/chrome/browser/search.properties It's really bad from a localization point of view. > searchFor=Search for > searchWith= with: 1) Unless I'm mistaken, whitespaces before and after the separator are simply ignored unless they're in escaped form. https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Tutorial/Property_Files#Escape_non-ASCII_Characters 2) Concatenations are bad. https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Avoid_concatenations_use_placeholders_instead Unless the are huge technical limitations, we should avoid them at all costs and use something like "Search for #1 with:", and replace #1 with whatever needed. > searchWithHeader=Search with: Is this concatenated with the name of the searchengine? Having said that concatenations are bad, if you decide to take that road you always need to provide a "before" and "after" string. Not having that forces a grammar structure on other languages.
Reporter | ||
Comment 1•9 years ago
|
||
Side note: escaped spaces are pretty confusing, and might confuse some tools too.
Assignee | ||
Comment 2•9 years ago
|
||
Some context on why it was done this way in bug 1171344: The design specified in bug 1106057 has the query text styled differently than the rest of the string. It is probably possible to format the string by replacing the "#1" or whatever with HTML code (rather unclean but it would work), but there was precedent in the main searchbox code so we went the concatenation route. As for the searchWithHeader - no, there is no concatenation going on there. It stays as "Search with:".
Reporter | ||
Comment 3•9 years ago
|
||
(In reply to Nihanth Subramanya [:nhnt11] from comment #2) > Some context on why it was done this way in bug 1171344: > The design specified in bug 1106057 has the query text styled differently > than the rest of the string. It is probably possible to format the string by > replacing the "#1" or whatever with HTML code (rather unclean but it would > work), but there was precedent in the main searchbox code so we went the > concatenation route. This is the involved code, isn't it? https://hg.mozilla.org/mozilla-central/rev/b2a96402ed45#l7.561 I don't think replacing the placeholder with the <span> HTML code would be that awful. > As for the searchWithHeader - no, there is no concatenation going on there. > It stays as "Search with:". Thanks (missed the UX design in bug 1106057, so couldn't figure out how this was actually used).
Reporter | ||
Comment 4•9 years ago
|
||
Also realized that "Change Search Settings" is hard-coded https://hg.mozilla.org/mozilla-central/rev/b2a96402ed45#l7.744
Summary: Fix localization issues in one-off searches → Fix localization issues in one-off searches: remove contatenations, "Change Search Settings" is hard-coded
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #4) > Also realized that "Change Search Settings" is hard-coded > https://hg.mozilla.org/mozilla-central/rev/b2a96402ed45#l7.744 That's embarrassing.
Attachment #8637104 -
Flags: review?(adw)
Reporter | ||
Comment 6•9 years ago
|
||
Comment on attachment 8637104 [details] [diff] [review] Patch Review of attachment 8637104 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! String-wise it looks good, only one localization comment to update. ::: browser/locales/en-US/chrome/browser/search.properties @@ +40,5 @@ > > # LOCALIZATION NOTE (searchWithHeader): > # The wording of this string should be as close as possible to > # searchFor and searchWith. This string will be used instead of > # them when the user has not typed any keyword. Need to update this comment too.
Attachment #8637104 -
Flags: feedback+
Assignee | ||
Comment 7•9 years ago
|
||
Updated comment.
Attachment #8637104 -
Attachment is obsolete: true
Attachment #8637104 -
Flags: review?(adw)
Attachment #8637112 -
Flags: review?(adw)
Reporter | ||
Updated•9 years ago
|
Attachment #8637112 -
Flags: feedback+
Updated•9 years ago
|
Attachment #8637112 -
Flags: review?(adw) → review+
Assignee | ||
Comment 8•9 years ago
|
||
green try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6cf43d895bce
Keywords: checkin-needed
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f389c9c1eafb
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
You need to log in
before you can comment on or make changes to this bug.
Description
•