1300199 - Move list.txt over to JSON once bug 1276739 is in

Status: RESOLVED FIXED

(Thunderbird :: Search, defect)

Description

[Mike Kaply [:mkaply]]

In bug 1276739, we are moving to JSON files for search, replacing list.txt.

For browser, we will have one JSON file that is parsed at build time to create locale specific JSON files.

Once we get the code checked in, we'd like the change made in Thunderbird so we can remove the list.txt code from search service.

Comment 1

[Mike Kaply [:mkaply]]

Attachment: Move Thunderbird to list.json

Here's the patch that moves thunderbird to list.json.

It does not centralize the files. That's bug 1309717.

To be clear, there are two stages here (as there are in Firefox).

1. Move to list.json
2. Dedupe and merge search files.

1. Is straightforward.
2. Will be painful and complicated based on what I'm seeing in Thunderbird.

2 is also unnecessary to get everything working. IT's a "nice to have"

Comment 2

[Jorg K (CEST = GMT+2)]

Comment on attachment 8800440 [details] [diff] [review]
Move Thunderbird to list.json

Thanks.

That's basically what I had in attachment 8799848 [details] [diff] [review] with the list.json reworked.

Comment 3

[Mike Kaply [:mkaply]]

> That's basically what I had in attachment 8799848 [details] [diff] [review] [diff] [review] with the list.json reworked.

Yep. You had the exact right idea. Painful part is grabbing all the list.txt content and moving it into list.json.

Comment 4

[Jorg K (CEST = GMT+2)]

So this is ready to land? Or do you want that landed after bug 1309273 so we can first see that the tests work using the old list.txt?

Comment 5

[Mike Kaply [:mkaply]]

It's ready to land. I have it backed out locally to verify the test fix.

Comment 6

[Jorg K (CEST = GMT+2)]

Thanks again.
https://hg.mozilla.org/comm-central/rev/593964199b659fdb8b5b7d8ce765c1976a783c99

Comment 7

[Francesco Lodolo [:flod]]

Attachment: differences.txt

Mike, which l10n branch did you use to create this list.json? I have the feeling you used central and not aurora to get the list of locales, we should do the same thing we did for Firefox (use Aurora). That includes keeping a relevant order for future sanity (first searchengines explicitly listed in search.order, then others in alphabetical order).

This is the diff I'm getting with my scripts adapted from Firefox.

Comment 8

[Mike Kaply [:mkaply]]

I did use an aurora l10n, but clearly somehow my Aurora was out of date.

I guess I assumed the list.txt files hadn't changed recently.

I'll fix.

I'm still unclear as to where to get the list of locales to include in the list.json though. Shipping locales doesn't seem to reflect the shipping locales...

Comment 9

[Francesco Lodolo [:flod]]

In Firefox shipped-locales includes locales that ship in Beta and Release, all-locales has all locales shipping in aurora. 
Since search engines are set up when we add new locales to aurora, all-locales from mozilla-aurora is the best point of reference.

I assume the same is valid for comm-central/mail
http://hg.mozilla.org/releases/comm-aurora/file/default/mail/locales/all-locales

In any case, there must be something wrong also on my side of checks, since I don't see bn-IN anywhere (shipped-locales or all-locales).

Comment 10

[Mike Kaply [:mkaply]]

It doesn't look like Thunderbird ships bn-IN. They ship bn-BD.

Comment 11

[Francesco Lodolo [:flod]]

(In reply to Mike Kaply [:mkaply] from comment #10)
> It doesn't look like Thunderbird ships bn-IN. They ship bn-BD.

Exactly, that's why I said something is wrong on my side: my script reports bn-IN as missing in list.json but available in my checks, but it shouldn't be there.
The problem is that I'm checking all locales that are enabled in Firefox, and if they have a list.txt, I consider them as available in Thunderbird.

The proper way is to look at all-locales. On a side note, I'd strongly suggest someone in Thunderbird to take a look at the status of some of those (be, ta-LK come to mind).

Comment 12

[Mike Kaply [:mkaply]]

I think in the end, it doesn't matter if we overship locales in the list.json, right?

And since there's a default, even if we forget a locale, that locale will still work properly.

So it's a win win.

I'll take a look.

Comment 13

[Francesco Lodolo [:flod]]

(In reply to Mike Kaply [:mkaply] from comment #12)
> I think in the end, it doesn't matter if we overship locales in the
> list.json, right?
> 
> And since there's a default, even if we forget a locale, that locale will
> still work properly.

Given that we fallback to a default, I think it would be safer to cover the minimum (all-locales), no need to maintain unused XML files.

Comment 14

[Mike Kaply [:mkaply]]

What's especially confusing is that af, bn-BD, gl, sr and ta-LK are in all-locales but they don't have mail/searchplugins/list.txt files...

Comment 15

[Mike Kaply [:mkaply]]

flod:

Your comparison doesn't look correct.

The values you have for the various locales are browser, not mail. For instance ar is:

          "bing", "yahoo", "twitter", "wikipedia-ar", "amazondotcom"

It doesn't have google:

https://dxr.mozilla.org/l10n-mozilla-aurora/source/ar/mail/searchplugins/list.txt

Comment 16

[Francesco Lodolo [:flod]]

I clearly forgot to update some pieces of the script still referencing 'browser'. Let me fix it and re-run the script.

Comment 17

[Francesco Lodolo [:flod]]

Attachment: differences.txt

Hopefully this makes more sense.

This is the script generating it
https://gist.github.com/flodolo/d969c1f7f24dd6b9b646cfaf8f38200c

Comment 18

[Jorg K (CEST = GMT+2)]

Hi Mike and Francesco, you're busily discussing in a bug that was closed in comment #6 ;-)

I'm only following the discussion from a corner of my eye. Is this about improving list.json for Thunderbird since we're in a Thunderbird bug here? Or for Firefox?

Please let me know. I'm happy to land any patch you deem fit ;-)
There is also bug 1309717 which was meant to provide an optimised list.json for Thunderbird.

Comment 19

[Francesco Lodolo [:flod]]

I think we're basically trying to agree on a follow-up bug :-)

Bug 1309717 can probably be used for that, otherwise it would pretty much be a dupe of this one.

Comment 20

[Francesco Lodolo [:flod]]

Adding 
https://l10n.mozilla-community.org/~flod/p12n/errors/?channel=trunk&product=mail

Things to note:
* en-GB has a reference to eBay that should be removed
* You use wikipediapt for pt-PT, but it's still wikipediaptpt.xml in the l10n repository. Is there an open bug to centralize .xml files as well? I can help fixing that at the beginning of the next Aurora cycle, would just need a bug on file to avoid forgetting
* fbc-pl.xml is missing for pl

All errors tagged as "SP" should be fixed in list.json, those marked as p12n come from region.properties

Comment 21

[Mike Kaply [:mkaply]]

> * en-GB has a reference to eBay that should be removed

Fixed

> * You use wikipediapt for pt-PT, but it's still wikipediaptpt.xml in the l10n repository. Is there an open bug to centralize .xml files as well? I can help fixing that at the beginning of the next Aurora cycle, would just need a bug on file to avoid forgetting

Not yet. I gues we can use bug 1309717. I think this is going to be more painful than Firefox though.

> * fbc-pl.xml is missing for pl

It's not in list.txt: https://dxr.mozilla.org/l10n-mozilla-aurora/source/pl/mail/searchplugins/list.txt

even though it's in the directory.

I'll fix the rest and put a patch in bug 1309717.

Comment 22

[Francesco Lodolo [:flod]]

(In reply to Mike Kaply [:mkaply] from comment #21)
> Not yet. I gues we can use bug 1309717. I think this is going to be more
> painful than Firefox though.

I would definitely keep the move for Firefox and Thunderbird separate. Also, moving them into a centralized folder is not a blocker, and requires a lot of checks for duplicates.

You're correct about pl, I misread my own error report…