Closed Bug 1281630 Opened 8 years ago Closed 8 years ago

Create JSON files for all locales to replace list.txt

Categories

(Firefox :: Search, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
firefox50 --- affected

People

(Reporter: mkaply, Unassigned)

References

Details

(Whiteboard: [partner search] [fxsearch])

Attachments

(2 files)

As part of the conversion to JSON files for defining search plugins in bug 1276739, files need to be created for all locales that represent the search engines.

The plan going forward is that the order in these files will specify the order of the search engines in the UI (replacing browser.search.order.1), but initially the engines will still be sorted.

When I started investigating creating these files, I discovered that the UI search order is different between Windows (English) and Mac (English), in particular how latin characters are sorted against non Latin characters.

I went with the Windows sort order.

Ideally we would be explicit about the search order for all locales, but we need UX involvement to verify that it's ok to change the order for existing users.
Whiteboard: [partner search] [fxsearch]
Comment on attachment 8764404 [details] [diff] [review]
Initial pass at JSON files for all locales

Do we need to have all the translators look at this?

Basically for now, this is a simple conversion of list.txt to a JSON file.

The order is in there, but it won't be used.

So the key is "are the same engines in the JSON that were in the TXT"

Maybe you could write a script that checked my work? :)
Attachment #8764404 - Flags: review?(francesco.lodolo)
(In reply to Mike Kaply [:mkaply] from comment #1)
> Initial pass at JSON files for all locales
I was expecting one single file for all search settings across locales. 

Is there a specific reason (e.g. build system) or benefit to go with one file for each locale? I don't think the size of the single JSON file would become unmanageable, while I see a lot of benefits in having one file.

> Do we need to have all the translators look at this?
No, I think we should simply ask them to double check that everything looks good after it lands.

> Maybe you could write a script that checked my work? :)
I can definitely do that. Timing might be a bit of a challenge, since I'll be on vacation next week, but I'll try to come up with something.

From a first look at the list, I think it would be a good time to get rid of x-testing.
Comment on attachment 8764404 [details] [diff] [review]
Initial pass at JSON files for all locales

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

::: browser/locales/search/bn-IN.json
@@ +1,5 @@
> +{
> +  "settings": {
> +    "default": {
> +      "visibleDefaultEngines": [
> +        "google", "yahoo-in", "amazondotcom", "bing", "ddg", "ebay-in", "rediff", "wikipedia-bn"

eBay-in (different case)

::: browser/locales/search/en-ZA.json
@@ +4,5 @@
> +      "visibleDefaultEngines": [
> +        "google", "bing", "amazondotcom", "ddg", "twitter", "wikipedia"
> +      ]
> +    }
> +}

JSON is broken, missing one }

::: browser/locales/search/gd.json
@@ +1,5 @@
> +{
> +  "settings": {
> +    "default": {
> +      "visibleDefaultEngines": [
> +        "google", "yahoo-en-GB", "faclair-beag", "amazon-en-GB", "bbc-alba", "ddg", "wikipedia-gd"

Nice, found a bug. gd shouldn't have ddg explicitly listed
http://hg.mozilla.org/releases/l10n/mozilla-aurora/gd/file/3511bf1a4e7b/browser/searchplugins/list.txt

::: browser/locales/search/hi-IN.json
@@ +1,5 @@
> +{
> +  "settings": {
> +    "default": {
> +      "visibleDefaultEngines": [
> +        "google", "yahoo-in", "bing", "ddg", "eBay-in", "wikipedia-gu"

wikipedia-gu -> wikipedia-hi

::: browser/locales/search/kok.json
@@ +1,5 @@
> +{
> +  "settings": {
> +    "default": {
> +      "visibleDefaultEngines": [
> +        "google", "ddg", "naver-kr", "danawa-kr", "daum-kr", "wikipedia-kr"

Completely wrong, this is the Korean(ko) one, not Konkani (kok). 

"google, yahoo-in, bing, ddg, eBay-in, wikipedia-hi"

::: browser/locales/search/or.json
@@ +1,5 @@
> +{
> +  "settings": {
> +    "default": {
> +      "visibleDefaultEngines": [
> +        "google", "yahoo-in", "amazondotcom", "bing", "ddg", "eBay-in", "wikipedia-or"

Should be: "google, yahoo-in, bing, amazondotcom, ddg, eBay-in, wikipedia-or"

Bing is 3rd in search order
http://hg.mozilla.org/releases/l10n/mozilla-aurora/or/file/tip/browser/chrome/browser-region/region.properties#l11

::: browser/locales/search/sk.json
@@ +1,5 @@
> +{
> +  "settings": {
> +    "default": {
> +      "visibleDefaultEngines": [
> +        "google", "azet-sk", "atlas-sk", "ddg", "dunaj-sk", "eBay", "slovnik-sk", "wikipedia", "zoznam-sk"

wikipedia -> wikipedia-sk

::: browser/locales/search/th.json
@@ +1,5 @@
> +{
> +  "settings": {
> +    "default": {
> +      "visibleDefaultEngines": [
> +        "google", "yahoo", "amazondotcom", "bing", "ddg", "eBay", "longdo", "wikipedia"

wikipedia -> wikipedia-th

::: browser/locales/search/uk.json
@@ +1,5 @@
> +{
> +  "settings": {
> +    "default": {
> +      "visibleDefaultEngines": [
> +        "google", "yandex", "meta-ua", "ddg", "wikipedia-wo", "metamarket"

wikipedia-wo -> wikipedia-uk

::: browser/locales/search/vi.json
@@ +1,5 @@
> +{
> +  "settings": {
> +    "default": {
> +      "visibleDefaultEngines": [
> +        "google", "5giay", "baambootratuav", "ddg", "muare", "wikipedia-vi", "zing-mp3"

"google, ddg, wikipedia-vi, zing-mp3"

The list was heavily changed in aurora recently, but locale is falling behind
http://hg.mozilla.org/releases/l10n/mozilla-aurora/vi/log/default/browser/searchplugins/list.txt
Attachment #8764404 - Flags: review?(francesco.lodolo) → review-
If you want to hurt your eyes, here's the script that I've used.
https://gist.github.com/flodolo/34b0d4f8b93b7e8058a01815a944b8b6

The sorting part is very brittle, and clearly fails for some unicode names.

One more thought: you have a few locales that I don't have, because we have repositories but they're not actually shipping.

Locales: ak, csb, ku, lg, mn, nr, nso, oc, rw, sah, ss, st, sw, ta-LK, tn, ts, ve, x-testing, zu

I'm tempted to start blank with these (use en-US defaults). If they ever decide to ship, we know that we need to re-approve the list of productization settings.

As said above, I think we should drop x-testing completely from repositories.

@Pike
Thoughts on the last two parts?
Flags: needinfo?(l10n)
Yes, we shouldn't touch stuff that's not building, including x-testing.

I'd also prefer a single file, I guess. That requires some modification to the patch in bug 1276739, but I don't think it's too bad.

If we should remove the repo is a good question, I'll need to run some checks.
Flags: needinfo?(l10n)
flod: Great catches, thanks. I'll fix.

> I'd also prefer a single file, I guess. That requires some modification to the patch in bug 1276739, but I don't think it's too bad.

On further thought, you're right. I thought it would be easier to maintain multiple files, but one file will make the build easier and make it so that new locales are handled by default.
Attached file Unified JSON file
I took in all the comments and corrected.

I removed all the unused locales and unified the file.
Attachment #8765005 - Flags: review?(francesco.lodolo)
Attachment #8765005 - Flags: feedback?(l10n)
Comment on attachment 8765005 [details]
Unified JSON file

kok has the wrong format

> "google, yahoo-in, bing, ddg, eBay-in, wikipedia-hi"

should be 

> "google", "yahoo-in", "bing", "ddg", "eBay-in", "wikipedia-hi"

The only other differences I see with my script output are be, sat, zh-CN where the ordering differs. With 'kok' fixed it looks definitely good.
Attachment #8765005 - Flags: review?(francesco.lodolo) → review+
Also took the change to remove ddg from Scottish Gaelic
https://hg.mozilla.org/releases/l10n/mozilla-aurora/gd/rev/2eecc56284fb
Comment on attachment 8765005 [details]
Unified JSON file

Don't think I have additional comments to what flod said.
Attachment #8765005 - Attachment is patch: false
Attachment #8765005 - Flags: feedback?(l10n)
This became part of the patch in 1276739
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: