1185845 - Fix localization issues in one-off searches: remove contatenations, "Change Search Settings" is hard-coded

Status: RESOLVED FIXED

(Firefox :: Search, defect)

Description

[Francesco Lodolo [:flod]]

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.

Comment 1

[Francesco Lodolo [:flod]]

Side note: escaped spaces are pretty confusing, and might confuse some tools too.

Comment 2

[(inactive) Nihanth Subramanya [:nhnt11]]

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:".

Comment 3

[Francesco Lodolo [:flod]]

(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).

Comment 4

[Francesco Lodolo [:flod]]

Also realized that "Change Search Settings" is hard-coded
https://hg.mozilla.org/mozilla-central/rev/b2a96402ed45#l7.744

Comment 5

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Patch

(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.

Comment 6

[Francesco Lodolo [:flod]]

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.

Comment 7

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Patch v1.1

Updated comment.

Comment 8

[(inactive) Nihanth Subramanya [:nhnt11]]

green try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6cf43d895bce

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/f389c9c1eafb

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f389c9c1eafb