Closed Bug 1544013 Opened 5 years ago Closed 5 years ago

41 tests from toolkit/components/search/tests/xpcshell fail when run as part of TB - Default search engines disappeared on Trunk - Two failing Mozmill tests

Categories

(Thunderbird :: Search, defect)

Product:
Thunderbird
Component:
Search
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(thunderbird68+)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 68.0
Tracking Flags:
Tracking Status
thunderbird68 + ---

People

(Reporter: jorgk-bmo, Assigned: darktrojan)

References

Details

(Whiteboard: [Thunderbird-testfailure: XZ all][Thunderbird-disabled-test])

Attachments

(3 files, 2 obsolete files)

1544013-search-schema-2.diff
25.98 KB, patch
mkmelin
: review+
Details | Diff | Splinter Review
search-extensions.zip
257.80 KB, application/zip
Details
1544013-search-engine-conversion.patch
897.94 KB, patch
Details | Diff | Splinter Review

TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_json_cache_ignorelist.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_list_json_searchdefault.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_list_json_locale.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_list_json_locale.js | test_listJSONlocale - [test_listJSONlocale : 257] A promise chain failed to handle a rejection: 2147500037 - stack: _init@resource://gre/modules/SearchService.jsm:2658:27
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_pref.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_pref.js | test_pref - [test_pref : 257] A promise chain failed to handle a rejection: 2147500037 - stack: _init@resource://gre/modules/SearchService.jsm:2658:27
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_defaultEngine.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_defaultEngine.js | test_defaultEngine - [test_defaultEngine : 37] [object XPCWrappedNative_NoHelper] == null
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_addEngine_callback.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_addEngine_callback.js | simple_callback_test - [simple_callback_test : 37] "Test search engine" != "Test search engine"
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_async_distribution.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_async_distribution.js | test_async_distribution - [test_async_distribution : 257] A promise chain failed to handle a rejection: 2147500037 - stack: _init@resource://gre/modules/SearchService.jsm:2658:27
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_async_disthidden.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_async_disthidden.js | test_disthidden - [test_disthidden : 257] A promise chain failed to handle a rejection: 2147500037 - stack: _init@resource://gre/modules/SearchService.jsm:2658:27
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_selectedEngine.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_async.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_async.js | test_async - [test_async : 257] A promise chain failed to handle a rejection: 2147500037 - stack: _init@resource://gre/modules/SearchService.jsm:2658:27
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_currentEngine_fallback.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_currentEngine_fallback.js | test_fallbacks - [test_fallbacks : 9] false == true
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_hidden.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_hidden.js | async_init - [async_init : 257] A promise chain failed to handle a rejection: 2147500037 - stack: _init@resource://gre/modules/SearchService.jsm:2658:27
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_require_engines_in_cache.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_require_engines_in_cache.js | ignore_cache_files_without_engines - [ignore_cache_files_without_engines : 257] A promise chain failed to handle a rejection: 2147500037 - stack: _init@resource://gre/modules/SearchService.jsm:2658:27
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_engineUpdate.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_engineUpdate.js | test_engineUpdate - [test_engineUpdate : 36] Order not cleared by update - "undefined" == 1
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_validate_manifests.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_purpose.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_purpose.js | setup - [setup : 257] A promise chain failed to handle a rejection: 2147500037 - stack: _init@resource://gre/modules/SearchService.jsm:2658:27
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_webextensions_install.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_webextensions_install.js | basic_install_test - [basic_install_test : 257] A promise chain failed to handle a rejection: 2147500037 - stack: _init@resource://gre/modules/SearchService.jsm:2658:27
TEST-UNEXPECTED-FAIL | toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_async_addon.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_async_addon.js | test_async_addon - [test_async_addon : 257] A promise chain failed to handle a rejection: 2147500037 - stack: _init@resource://gre/modules/SearchService.jsm:2658:27
TEST-UNEXPECTED-TIMEOUT | toolkit/components/search/tests/xpcshell/test_nocache.js | Test timed out
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_json_cache.js | xpcshell return code: 0
TEST-UNEXPECTED-TIMEOUT | toolkit/components/search/tests/xpcshell/test_identifiers.js | Test timed out
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_list_json_searchdefault_distro.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_list_json_searchorder.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_list_json_searchorder.js | test_searchOrderJSON - [test_searchOrderJSON : 22] search initialized - false == true
TEST-UNEXPECTED-TIMEOUT | toolkit/components/search/tests/xpcshell/test_reloadEngines.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | toolkit/components/search/tests/xpcshell/test_geodefaults.js | Test timed out

M-C last good: c77962add953a48bdbcbbcd3d157d63d12
M-C first bad: a632dfa60af6edd4f7dd03d404d898b622
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c77962add953a48bdbcbbcd3d157d63d12&tochange=a632dfa60af6edd4f7dd03d404d898b622

That looks a lot like:
6534d80a3329 Dale Harvey — Bug 1496075 - Part 4: Delete searchplugins. r=mixedpuppy
9e0b9cbb8982 Dale Harvey — Bug 1496075 - Part 3: Use webextensions in SearchServices. r=mikedeboer,mixedpuppy
e07b4ceea077 Dale Harvey — Bug 1496075 - Part 2: Telemetry changes to support search extensions. r=chutten,raphael
7efb082cc25d Dale Harvey — Bug 1496075 - Part 1: Extensions changes to support search extensions. r=mixedpuppy,robwu
041d3ca2a427 Dale Harvey — Bug 1486820 - Part 2: Add tests to ensure valid manifest. r=rpl
111b88dd28d6 Dale Harvey — Bug 1486820 - Convert Opensearch files to WebExtensions. r=mixedpuppy

Note that rev 6534d80a3329 deleted all content from browser/components/search/searchplugins.

I noticed that in Daily all default search engines have disappeared, I only have the ones that I added manually with a TB version that allowed manual adding (like ESR and beta).

Magnus, that's hitting us by surprise, isn't it? Since this is going into the WE corner, I'm adding Geoff to the NI.

Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(geoff)
Summary: 41 tests from toolkit/components/search/tests/xpcshell fail when run as part of TB → 41 tests from toolkit/components/search/tests/xpcshell fail when run as part of TB - Default search engines disappeared on Trunk

Hmm, "Convert Opensearch files to WebExtensions" looks like some work. We mostly copied FF's files in bug 1427133, but there were a lot of tweaks. Looking at bug 1486820 comment #23, there is a script to do it.

There are also two failing Mozimill tests:
EST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1555067300/build/tests/mozmill/newmailaccount/test-newmailaccount.js | test-newmailaccount.js::test_get_an_account
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1555067300/build/tests/mozmill/newmailaccount/test-newmailaccount.js | test-newmailaccount.js::test_restored_ap_tab_works

https://taskcluster-artifacts.net/buF8_GvxSKiROIp9O_gCUg/0/public/logs/live_backing.log says:
EXCEPTION: a == b: '[xpconnect wrapped nsISearchEngine]' == '[xpconnect wrapped nsISearchEngine]'

So this is also coming from here.

Summary: 41 tests from toolkit/components/search/tests/xpcshell fail when run as part of TB - Default search engines disappeared on Trunk → 41 tests from toolkit/components/search/tests/xpcshell fail when run as part of TB - Default search engines disappeared on Trunk - Two failing Mozmill tests

Looking at the tweaks in bug 1427133, it might be more expedient to run the script from https://github.com/daleharvey/ffx-opensearch-to-webext. Dale, are there any instructions on how to run this?

Flags: needinfo?(dharvey)

Let's just see what happens if I just copy M-C's browser/components/search/extensions to TB's mail/components/search/extensions:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=835ec756865b15f1bdb39d557c7d4be75e234f82

I guess we will also need to port some of this: https://hg.mozilla.org/mozilla-central/rev/7efb082cc25d, but that's better answered by Geoff.

Whiteboard: [Thunderbird-testfailure: XZ all][Thunderbird-disabled-test]
Keywords: leave-open
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/826410daf770
temporarily disable failing parts of test-newmailaccount.js. rs=bustage-fix
Attached patch 1544013-search-schema-1.diff (obsolete) — — Splinter Review

I've copied the API and schema from mozilla-central and removed all references to changing the homepage, which it also does in Firefox. I've also used the JAR manifest to ship the default search extensions from mozilla-central.

I expect this try run will work, although I did forget to re-enable the mozmill tests before starting it. They work on my machine so it's probably fine.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&group_state=expanded&revision=2b4fc55495b8081555a6b4e2cf7ccbc203e01e52

Assignee: nobody → geoff
Status: NEW → ASSIGNED
Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(geoff)
Flags: needinfo?(dharvey)
Attachment #9058233 - Flags: review?(mkmelin+mozilla)

We could probably stop shipping the contents of mail/components/search/searchplugins now, and get rid of it, but I haven't done so in the patch.

Okay, scratch that last message, and the one about mozmill tests. The packager doesn't like having the old stuff around (it didn't complain on my local build, but that's not the one that counts).

Here's a new try run:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=36259fdf3fe45eac3681da3d1a10219b5b69e713

Comment on attachment 9058233 [details] [diff] [review]
1544013-search-schema-1.diff

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

Looks OK as a bustage fix to start with. We still need to migrate TB's search plugins.

::: mail/components/search/jar.mn
@@ +2,5 @@
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  messenger.jar:
> +    search-extensions/                        (../../../../browser/components/search/extensions/**)

That will be replaced when we migrate mail/components/search/searchplugins to mail/components/search/extensions, right?

@@ +7,1 @@
>      searchplugins/                            (searchplugins/**)

And that will go, I suppose? EDIT: Sorry, commented on the old patch, already gone in the new one.

Repeating comment #3 re. the migration of the search plugins:

Looking at the tweaks in bug 1427133, instead of copying FF's search extensions and adjusting them, it might be more expedient to run the script from https://github.com/daleharvey/ffx-opensearch-to-webext to convert TB's plugins. Dale, are there any instructions on how to run this?

Flags: needinfo?(dharvey)

Actually, Geoff's patch already deletes all of TB's search plugins. Not such a great idea landing this since we'll need to see how we convert them first.

Geoff's patch from try without deleting the search plugins.

Attachment #9058233 - Attachment is obsolete: true
Attachment #9058233 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9058246 [details] [diff] [review]
1544013-search-schema-2.diff

Magnus, please take a look at this one. I/we'll process our search plugins separately. Let's see that it still works:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3b91eab925805e49e9a4654f7c1be771f659cb14
Attachment #9058246 - Flags: review?(mkmelin+mozilla)
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/30a7914d1c64
Support search extensions and temporarily ship the default ones from mozilla-central. rs=bustage-fix,jorgk
Comment on attachment 9058246 [details] [diff] [review]
1544013-search-schema-2.diff

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

Looks reasonable to me. r=mkmlin

::: mail/components/extensions/schemas/chrome_settings_overrides.json
@@ +46,5 @@
> +                    "type": "string",
> +                    "optional": true,
> +                    "format": "url",
> +                    "preprocess": "localize",
> +                    "deprecated": "Unsupported on Firefox at this time."

There's a bunch of these talking about Firefox

::: mail/components/search/jar.mn
@@ +2,5 @@
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  messenger.jar:
> +    search-extensions/                        (../../../../browser/components/search/extensions/**)

for this add a comment with fixme bug
Attachment #9058246 - Flags: review?(mkmelin+mozilla) → review+
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/dd798dde8956
Follow-up: s/Firefox/Thunderbird/ in chrome_settings_overrides.json. r=me

Hey, there isnt any instructions on that repo sorry, I will update it once I am on the computer that has the latest changes, guessing its to be run on https://searchfox.org/comm-central/source/mail/components/search/searchplugins ? I will run against that and make sure that it works (tomorrow morning latest)

Leaving needinfo to remind me

Excellent. I guess you'll need a checkout of https://hg.mozilla.org/comm-central/? That's a small repository you can pull in about 2 minutes. Maybe you could zip up the resulting extensions/ directory, or even put it into a patch, whatever is easiest.

So what I want to know is: how do our search engines differ from Firefox's, and why?

Read the bug I quoted various times, last in comment 10. Sorry, can't paste on the damn phone.

Now that I'm back at a normal keyboard: The search engines differ in three things

  1. TB has no partner codes, see bug 1427133 comment #22 and above.
  2. FF has multiple Google variations, like google-2018.xml, see bug 1427133 comment #20.
  3. FF has some eBays we don't have, see bug 1427133 comment #33.

If the automatic conversion can be done easily, it will be faster than re-applying all the tweaks again.

Dale, thanks for the offer to help us. Is there any news?

Flags: needinfo?(dharvey)
Flags: needinfo?(dharvey)
tracking-thunderbird68: --- → +
Attached file search-extensions.zip (obsolete) —

Sorry for the delay, I wanted to get https://bugzilla.mozilla.org/show_bug.cgi?id=1545517 landed before doing anything here.

I pushed the latest code to https://github.com/daleharvey/ffx-opensearch-to-webext, as mentioned its not a very polished script, it kept moving as the requirements changed but since you are using the same SearchService code it should work

If you check it out, change the path to SEARCHPLUGINS_FOLDER you can run

$ rm -rf tmp/ && node index.js --gecko-path=/users/foo/comm-central

Then you will get what I have attached (in the folder tmp/)

Flags: needinfo?(dharvey)

Also as I mentioned in the last comment, https://bugzilla.mozilla.org/show_bug.cgi?id=1545517 deleted a whole bunch of engines that are no longer working, particularly as we require https now, so you will want to delete and not convert those engines (the script automatically rewrites http to https, but not all the engines support that)

Thanks for that. Looking at the ZIP file I noticed this:
Amazon, Google, Wikipedia and Yandex don't have any favicon.ico files since the the XML file didn't have icon data since the icons were stored separately here:
https://searchfox.org/comm-central/source/mail/components/search/searchplugins/images.
That can be fixed manually since there are only five icons (not six since we don't have eBay at all).

Your script does locales, but they end up in, for example, search-extensions.zip\search-extensions\yandex_locales\comm-central\mail\components\search\searchplugins\yandex-az\messages.json (Windows path) where they should be in yandex/_lozales/az. Would it be hard to correct that in the script?

Maybe not worth the effort, only 10 engines with locales:
Directory of C:\temp\search-extensions\amazon
Directory of C:\temp\search-extensions\amazondotcom
Directory of C:\temp\search-extensions\bolcom
Directory of C:\temp\search-extensions\leo_ende_de
Directory of C:\temp\search-extensions\marktplaats
Directory of C:\temp\search-extensions\mercadolibre
Directory of C:\temp\search-extensions\twitter
Directory of C:\temp\search-extensions\wikipedia
Directory of C:\temp\search-extensions\wiktionary
Directory of C:\temp\search-extensions\yandex
That could be fixed manually. Deleting the 22 engines from bug 1545517 doesn't appear too much fun either.

I've just noticed that some of the original XML files referenced FF and now the JSON files do:

.\atlas-sk.xml:  <Param name="sourceid" value="firefox"/>
.\coccoc.xml:  <Param name="utm_source" value="firefox"/>
.\danawa-kr.xml:  <Param name="from" value="firefox"/>
.\google.xml:<Url type="application/x-suggestions+json" method="GET" template="https://www.google.com/complete/search?client=firefox&amp;q={searchTerms}"/>
.\heureka-cz.xml:<Url type="application/x-suggestions+json" method="GET" template="https://www.heureka.cz/direct/firefox/autocompleter.php">
.\heureka-cz.xml:       <Param name="utm_source" value="firefox-search"/>
.\hoepli.xml:   <Param name="utm_source" value="mozilla-firefox" />
.\olx.xml:<Url type="application/x-suggestions+json" method="GET" template="http://www.olx.ba/sugestije/firefox_pojmovi">
.\oshiete-goo.xml:  <Param name="from" value="Firefox30"/>
.\oshiete-goo.xml:  <Param name="PT" value="Firefox30"/>
.\osta-ee.xml:<Url type="text/html" method="GET" template="http://www.osta.ee/firefox/" resultdomain="osta.ee">
.\ozonru.xml:  <Param name="from" value="firefox"/>
.\ozonru.xml:  <Param name="from" value="firefox"/>
.\portalbgdict.xml:  <Param name="translate" value="firefox" />
.\priberam.xml:<Url type="text/html" method="GET" template="http://www.priberam.pt/dlpo/firefox.aspx" resultdomain="priberam.pt">
.\salidzinilv.xml:  <Param name="utm_source" value="firefox-plugin"/>
.\salidzinilv.xml:  <Param name="utm_source" value="firefox-plugin"/>
.\seznam-cz.xml:        <Param name="sourceid" value="firefox"/>
.\slovnik-sk.xml:  <Param name="sourceid" value="firefox"/>
.\sslv.xml:  <Param name="from" value="firefox-plugin"/>

.\atlas-sk\manifest.json:      "search_url_get_params": "phrase={searchTerms}&sourceid=firefox"
.\coccoc\manifest.json:      "search_url_get_params": "query={searchTerms}&s=ff&utm_source=firefox",
.\danawa-kr\manifest.json:      "search_url_get_params": "k1={searchTerms}&from=firefox"
.\google\manifest.json:      "suggest_url": "https://www.google.com/complete/search?client=firefox&q={searchTerms}"
.\heureka-cz\manifest.json:      "search_url_get_params": "h[fraze]={searchTerms}&utm_source=firefox-search",
.\heureka-cz\manifest.json:      "suggest_url": "https://www.heureka.cz/direct/firefox/autocompleter.php",
.\hoepli\manifest.json:      "search_url_get_params": "idD=1&utm_source=mozilla-firefox&query={searchTerms}"
.\olx\manifest.json:      "suggest_url": "https://www.olx.ba/sugestije/firefox_pojmovi",xL
.\oshiete-goo\manifest.json:      "search_url_get_params": "MT={searchTerms}&from=Firefox30&PT=Firefox30"
.\osta-ee\manifest.json:      "search_url": "https://www.osta.ee/firefox/",
.\ozonru\manifest.json:      "search_url_get_params": "context=search&text={searchTerms}&from=firefox",
.\ozonru\manifest.json:      "suggest_url_get_params": "text={searchTerms}&from=firefox"
.\portalbgdict\manifest.json:      "search_url_get_params": "encin=windows-1251&encout=windows-1251&translate=firefox&word={searchTerms}"
.\priberam\manifest.json:      "search_url": "https://www.priberam.pt/dlpo/firefox.aspx",
.\salidzinilv\manifest.json:      "search_url_get_params": "q={searchTerms}&utm_source=firefox-plugin",
.\salidzinilv\manifest.json:      "suggest_url_get_params": "q={searchTerms}&utm_source=firefox-plugin"
.\seznam-cz\manifest.json:      "search_form": "https://search.seznam.cz/?q={searchTerms}&sourceid=firefox",
.\seznam-cz\manifest.json:      "search_url_get_params": "q={searchTerms}&sourceid=firefox",
.\slovnik-sk\manifest.json:      "search_form": "https://slovnik.azet.sk/?q={searchTerms}&l=en-sk&sourceid=firefox",
.\slovnik-sk\manifest.json:      "search_url_get_params": "q={searchTerms}&l=en-sk&sourceid=firefox"
.\sslv\manifest.json:      "search_url_post_params": "txt={searchTerms}&from=firefox-plugin"

I guess that should be removed? Looks like it slipped through the review in bug 1427133? Landed for example here:
https://hg.mozilla.org/comm-central/rev/1337b98ae249#l17.16

Mike?

Flags: needinfo?(mozilla)

I didn't work on this, dale harvey did.

Flags: needinfo?(mozilla) → needinfo?(dharvey)

Mike, is there a misunderstanding? You reviewed the patch in bug 1427133 and approved all the "firefox" strings, see:
https://hg.mozilla.org/comm-central/rev/1337b98ae249 - search for "firefox".

Attached file search-extensions.zip —

Fixed the path of of locales

Attachment #9061776 - Attachment is obsolete: true
Flags: needinfo?(dharvey)

As for changing the string firefox, I suspect it is probably best to but I am not definitive on that

Thanks, Dale. I see that Amazon, Google and Wikipedia also have icons now, Yandex doesn't, looks like there are two (yandex-en, yandex-ru), both referenced in the JSON file. No problem to move them manually.

I'm just wondering how to proceed best here. Take the ZIP file as a base or worry about the "firefox" replacement first and then migrate again. It will be easier to drop single lines from the XML files than to edit them out of the JSON files.

We have something like <Param name="sourceid" value="firefox"/> 15 times and then five other ones (see comment #27):

google.xml:<Url type="application/x-suggestions+json" method="GET" template="https://www.google.com/complete/search?client=firefox&amp;q={searchTerms}"/>
heureka-cz.xml:<Url type="application/x-suggestions+json" method="GET" template="https://www.heureka.cz/direct/firefox/autocompleter.php">
olx.xml:<Url type="application/x-suggestions+json" method="GET" template="http://www.olx.ba/sugestije/firefox_pojmovi">
osta-ee.xml:<Url type="text/html" method="GET" template="http://www.osta.ee/firefox/" resultdomain="osta.ee">
priberam.xml:<Url type="text/html" method="GET" template="http://www.priberam.pt/dlpo/firefox.aspx" resultdomain="priberam.pt">

where the "firefox" is part of a URL.

So let's phrase this quite clearly for Mike to answer:

Mike, in bug 1427133 we copied the search engine XML files from FF, made some edits (summarised in comment #21) and landed the whole thing here: https://hg.mozilla.org/comm-central/rev/1337b98ae249

As a result, we now have the string "firefox" 20 times in our XML files, 15 files as Param, and five times as part of the URL, see above in this comment. Before we migrate the XML files to WebExtension JSON files, should we clean up the "firefox" strings? The 15 Params can be easily dropped. The five other ones are not so easily treatable. Apart from client=firefox, the remaining four specify a path on the search engine server, so removing "firefox" there will break things.

EDIT: I tried it, the Google search stops working when removing client=firefox, so all I could do for the clean-up would be to remove the 15 occurrences of "firefox" in a Param.

Flags: needinfo?(mozilla)

A lot of those firefox are needed because they are related to search suggestions.

I don't see any reason to remove them. we removed the ones that were related to revenue.

Flags: needinfo?(mozilla)

Thanks, Mike. So I have what I need. I'll just take Dale's ZIP file and then remove the 22 engines from bug 1545517.

Thanks again to you both!

Keywords: leave-open
Target Milestone: --- → Thunderbird 68.0

OK, here we go:

  • Used files provided by Dale Harvey.
  • Moved the two Yandex icons manually.
  • Moved list.json.
  • Removed 22 engines (bug 1545517), also from list.json
  • fixed mail/components/search/jar.mn.

This should be it. Looks OK in local build, but better safe than sorry:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=b6582fe5cdb382e850bbae9e2e4c8ad205698da3

Wow, test failures :-( - Before we just packaged the engines from FF here

and all the test pass, now there are heaps of failures with our own engines:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=b6582fe5cdb382e850bbae9e2e4c8ad205698da3&selectedJob=244007057
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/searchconfigs/test_baidu.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/searchconfigs/test_baidu.js | test_searchConfig_baidu - [test_searchConfig_baidu : 128] Should have completely re-initialization, if it fails check logs for if reinit was successful - false == true
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/searchconfigs/test_yandex.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/searchconfigs/test_yandex.js | test_searchConfig_yandex - [test_searchConfig_yandex : 128] Should have completely re-initialization, if it fails check logs for if reinit was successful - false == true
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/searchconfigs/test_bing.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/searchconfigs/test_bing.js | test_searchConfig_bing - [test_searchConfig_bing : 128] Should have completely re-initialization, if it fails check logs for if reinit was successful - false == true
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/searchconfigs/test_google.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/searchconfigs/test_google.js | test_searchConfig_google - [test_searchConfig_google : 128] Should have completely re-initialization, if it fails check logs for if reinit was successful - false == true
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_validate_manifests.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_validate_manifests.js | test_validate_manifest - [test_validate_manifest : 49] FAIL manifest for undefined in locale nl failed Error: Error processing chrome_settings_overrides.search_provider.search_form: String "http://www.bol.com/" must match /^https:\/\/.*$/ :: getLocalizedManifest@resource://gre/modules/Extension.jsm:608:13
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_validate_engines.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_validate_engines.js | test_validate_engines - [test_validate_engines : 257] A promise chain failed to handle a rejection: 2147500037 - stack: _init@resource://gre/modules/SearchService.jsm:2750:27

So basically the tests for Baido, Yandex, Bing and Google fail, on top test_validate_manifests.js fails on an engine which has bol.com in it, that's the "bolcom" engine. Somehow our "bolcom" files have http:// instead of https://, so that should be easily fixed. The Baidu file differs a little from the FF version ... I'll look at the other ones in a minute.

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/bee760385e54
Convert builtin opensearch XML files to webextention, remove 22 engines from bug 1545517. r=me

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED

For the record: To make the tests pass I had to

  • change all URLs to https:, there was a bunch for http: URLs still
  • fix the references to the Yandex icon resources
  • fix Leo by copying it again from the M-C files. There was some funny malfunctioning leo_ende_de-rm version which FF doesn't have.

Hmm, following FF search engine activities appears to be a never ending story. We copied the whole lot in Jan 2019 and since then missed three actualisations:
https://hg.mozilla.org/mozilla-central/log/tip/browser/components/search/searchplugins/list.json
Francesco Lodolo (:flod) - Bug 1535288 - Set up productization for Bengali (bn) on Firefox desktop and Firefox for Android
Francesco Lodolo (:flod) - Bug 1527349 - Remove meta.ua searchplugin from Ukrainian (uk) locale r=mkaply
Francesco Lodolo (:flod) - Bug 1523243 - Update URLs in LEO searchplugin, remove Romansh variant r=mkaply

I guess TB won't support Bengali, and I already copied Leo again which took care of the -rm variant. So now I still need to remove meta.au in order not to get into trouble.

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/f5d9b79ff6c6
Follow-up: Port bug 1527349: Remove meta.ua searchplugin from Ukrainian (uk) locale. r=me DONTBUILD
Regressions: 1566200
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/3751d9701937
Follow-up: Add newline to end of json files. rs=white-space-only
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: