1309273 - TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_json_cache.js - Port bug 1276739 to C-C

Status: RESOLVED DUPLICATE of bug 1300199

(Toolkit :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

Port bug 1276739 to C-C:
https://hg.mozilla.org/mozilla-central/rev/3cd0102f89e9

Looks like we need the browser/ files.

First seen:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=6d463070d845ed47088ee322e0bca4dc4bf12ae3

To run test locally:
$ mozilla/mach xpcshell-test toolkit/components/search/tests/xpcshell/test_json_cache.js

Error is:
PROCESS_OUTPUT: Thread-1 (pid:3784) "Chrome file doesn't exist: c:\\mozilla-source\\comm-central\\obj-i686-pc-mingw32\\dist\\bin\\chrome\\en-US\\locale\\en-US\\messenger\\searchplugins\\list.json"
LOG: Thread-1 ERROR NS_ERROR_FILE_NOT_FOUND: Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIChannel.open2]

Or is this another undesired dependency of toolkit/ on browser/?

Comment 1

[aleth [:aleth]]

Looks like list.txt has been replaced with list.json, and c-c apps have to make the equivalent changes made in the linked bug for browser/.

Comment 2

[Mike Kaply [:mkaply]]

You can still use list.txt for now. I check if it's there and use it.

Not sure why this test failure is happening.

Comment 3

[Mike Kaply [:mkaply]]

So I updated the tests to use list.json forgetting that comm-central is still running the tests.

What do you think the right solution is here?

TB and SM need to migrate, but it shouldn't be needed to get this test passing.

Comment 4

[Jorg K (CEST = GMT+2)]

I'll have a patch ready in a few minutes.

Comment 5

[Mike Kaply [:mkaply]]

Note that this is a bug in the test. xpcshell test should not be depending on browser list.json. I'll look into that separately.

Comment 6

[Jorg K (CEST = GMT+2)]

Attachment: mail/ part

This is a straight copy of what is in M-C.
However, I got
Missing searchplugin: yahoo-en-CA.xml
Missing searchplugin: google-nocodes.xml
Missing searchplugin: ddg.xml
so I removed those from list.json.

Test passes locally now.

Comment 7

[Mike Kaply [:mkaply]]

It's not that simple. You need to grab contents of the list.txt files from all of the locales in Thunderbird.

You can't use the browser version.

I'm working on fixing the test.

Comment 8

[Jorg K (CEST = GMT+2)]

(In reply to Mike Kaply [:mkaply] from comment #7)
> It's not that simple. You need to grab contents of the list.txt files from
> all of the locales in Thunderbird.
Where would I get those?

Comment 9

[Jorg K (CEST = GMT+2)]

How is this meant to work? Every time a locale is added, you need to update that file? And every time a locale changes search engines as well? Who's coordinating that?

Comment 10

[Mike Kaply [:mkaply]]

> How is this meant to work? Every time a locale is added, you need to update that file? And every time a locale changes search engines as well? Who's coordinating that?

These are going to replace the search information in the locales. Everything will be centrally managed. Even the search engines themselves will live in mozilla-central.

We're moving towards centralized management of search engines.

>>> The bug here is that the test is broke. I'm working on that. <<<

Comment 11

[Jorg K (CEST = GMT+2)]

Comment on attachment 8799848 [details] [diff] [review]
mail/ part

Let's see how this pans out then.

Do we have plans to manage TB search engines for all locales centrally?

Comment 12

[Mike Kaply [:mkaply]]

> Do we have plans to manage TB search engines for all locales centrally?

Ideally this should happen for all products, but it is up to you.

But you should be switching to the list.json at some point.

Comment 13

[aleth [:aleth]]

(In reply to Mike Kaply [:mkaply] from comment #12)
> > Do we have plans to manage TB search engines for all locales centrally?
> 
> Ideally this should happen for all products, but it is up to you.
> 
> But you should be switching to the list.json at some point.

We should probably file a separate bug for this work, and make sure it is dependant on the relevant m-c bugs.

(In reply to Mike Kaply [:mkaply] from comment #10)
> These are going to replace the search information in the locales. Everything
> will be centrally managed. Even the search engines themselves will live in
> mozilla-central.

Is there already a bug open for this work?

Comment 14

[Mike Kaply [:mkaply]]

https://bugzilla.mozilla.org/show_bug.cgi?id=1300199

is the bug for moving to list.json.

> Is there already a bug open for this work?

For browser, yes:

https://bugzilla.mozilla.org/show_bug.cgi?id=1276740

Technically thunderbird and seamonkey don't need to centralize their engines, but it should make life easier in the long run.

We've pondered centralizing all engines in toolkit, but I think there are too many differences.

Comment 15

[Mike Kaply [:mkaply]]

Attachment: Use list.txt as a backup for xpcshell test

This is a temporary fix while we get the other platforms moved over to list.json.

It allows the test_json_cache test to fallback to list.txt.

Comment 16

[Kent James (:rkent)]

If I understand correctly, the search engine functionality was originally put into TB in the MoMo days as one of several attempts at monetization. That did not really work out financially, and at some point post-MoMo the code was changed so that any searches are now directed to the browser (it was previously open in a TB tab). There is no current monetization in TB directed toward search, and no plans to attempt that for the foreseeable future. The default Bing search in Thunderbird is leftover from the failed monetization attempt, but no longer makes any sense.

The FF search list is managed according to monetization strategies for Firefox IIUC, so does not really make any sense for Thunderbird. Personally I never use search UI from Thunderbird, and I suspect it is fairly rare for others to do so, so I cannot see us putting any but minimal effort into that. Is there any UI in Thunderbird to pull up any but the default search engine? If am not aware of any.

So no there are no plans around centralizing search engines in Thunderbird that I am aware of.

Comment 17

[Mike Kaply [:mkaply]]

> Is there any UI in Thunderbird to pull up any but the default search engine? If am not aware of any.

I'll double check.

> The default Bing search in Thunderbird is leftover from the failed monetization attempt, but no longer makes any sense.

I thought that monetization was still in place.

I'll see if I can figure out the easiest solution for this.

IT's probably just to be a default JSON file in with Bing and be done.

Comment 18

[Mike Kaply [:mkaply]]

>> Is there any UI in Thunderbird to pull up any but the default search engine? If am not aware of any.

> I'll double check.

You can set the default in preferences to whatever you want. That's the only UI. No idea if there is data as to how people use that.

You can use this site:

https://transvision.mozfr.org/productization/?locale=en-US&product=mail

To see how many engines there are for Thunderbird there are in all the different languages.

It's pretty straightforward for me to at least create the JSON file for Thundebird. Once that's done, it will actually be easier if you want to change search engines since it will be in comm-central instead of the locales.

Comment 19

[Kent James (:rkent)]

(In reply to Mike Kaply [:mkaply] from comment #18)

> It's pretty straightforward for me to at least create the JSON file for
> Thundebird. Once that's done, it will actually be easier if you want to
> change search engines since it will be in comm-central instead of the
> locales.

If it is easy for you to solve the problem, please do so.

Comment 20

[Jorg K (CEST = GMT+2)]

I had a patch for migrating to list.json, but my list.json file was apparently wrong.

Comment 21

[Mike Kaply [:mkaply]]

> I had a patch for migrating to list.json, but my list.json file was apparently wrong.

I'll have this soon.

Comment 22

[Jorg K (CEST = GMT+2)]

Thanks. Please attach it to bug 1309717 and I will merge it with a patch to transition to list.json, basically the patch (attachment 8799848 [details] [diff] [review]) I had proposed here.

Comment 23

[Florian Quèze [:florian]]

Comment on attachment 8799924 [details] [diff] [review]
Use list.txt as a backup for xpcshell test

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

rs=me.

::: toolkit/components/search/tests/xpcshell/test_json_cache.js
@@ +57,5 @@
>    let chan = NetUtil.newChannel({
>      uri: "resource://search-plugins/list.json",
>      loadUsingSystemPrincipal: true
>    });
>    let visibleDefaultEngines = [];

This variable is dead code, let's remove it.

@@ +68,4 @@
>  
> +    cacheTemplate.visibleDefaultEngines = searchSettings["default"]["visibleDefaultEngines"];
> +  } catch (e) {
> +    // Allow test to run for now on TB and SM which use list.txt

Can we mention here the bug number of where we expect TB to switch to list.json?

@@ +80,5 @@
> +      if (name.endsWith(":hidden"))
> +        continue;
> +      visibleDefaultEngines.push(name);
> +    }
> +    cacheTemplate.visibleDefaultEngines = visibleDefaultEngines;

It seems to me that this whole thing can be simplified to:
  cacheTemplate.visibleDefaultEngines =
    list.split("\n").filter(n => n && !n.endsWith(":hidden"));

Comment 24

[Mike Kaply [:mkaply]]

The JSON transition is actually an existing bug - https://bugzilla.mozilla.org/show_bug.cgi?id=1300199.

bug 1309717 should be used for actually centralizing the search files (IF thunderbird wants to do it - it's not mandatory).

Comment 25

[Mike Kaply [:mkaply]]

I'm going to mark this fixed.

Seamonkey doesn't run the tests and we updated Thunderbird.

I don't see the need to hack this tests when no one will hit this case.

Comment 26

[Jorg K (CEST = GMT+2)]

Don't you want to land the r+ patch?

If you abandon the whole thing, it shouldn't be marked resolved/fixed. Maybe in this case it should be a duplicate of bug 1300199.

Comment 27

[Mike Kaply [:mkaply]]

The patch was basically a hack to make the test pass until the switch to list.txt, and since Thunderbird switched, we're good.

So you're right. Dup.

This was fixed by the patch in bug 1300199.