MB 213: Remove Leta from the built-in search engines

Merge Info

Related Issues

• #213 (closed)

Backporting

Timeline

• Immediate: patchset needed as soon as possible
• Next Minor Stable Release: patchset that needs to be verified in nightly before backport
• Eventually: patchset that needs to be verified in alpha before backport
• No Backport (preferred): patchset for the next major stable

(Optional) Justification

• Emergency security update: patchset fixes CVEs, 0-days, etc
• Critical bug-fix: patchset fixes a bug in core-functionality
• Consistency: patchset which would make development easier if it were in both the alpha and release branches; developer tools, build system changes, etc
• Sponsor required: patchset required for sponsor
• Other: please explain

Merging

• Merge to mullvad-browser - !fixups to mullvad-browser-specific commits, new features, security backports
• Merge to base-browser -!fixups to base-browser-specific commits, new features to be shared with tor-browser
  • NOTE: if your changeset includes patches to both base-browser and mullvad-browser please clearly label in the change description which commits should be cherry-picked to base-browser after merging

Issue Tracking

• Link resolved issues with appropriate Release Prep issue for changelog generation

Review

Request Reviewer

• Request review from an applications developer depending on modified system:
  • NOTE: if the MR modifies multiple areas, please /cc all the relevant reviewers (since gitlab only allows 1 reviewer)
  • accessibility : henry
  • android : dan
  • build system : boklm
  • extensions : ma1
  • firefox internals (XUL/JS/XPCOM) : ma1
  • fonts : pierov
  • frontend (implementation) : henry
  • frontend (review) : donuts, richard
  • localization : henry, pierov
  • nightly builds : boklm
  • rebases/release-prep : dan_b, ma1, pierov, richard
  • security : ma1
  • signing : boklm, richard
  • updater : pierov
  • misc/other : pierov, richard

/cc @ruihildt

Change Description

In the previous MR for #213 (closed), we added built-in search engines to Mullvad Browser, and it included Leta.

However, my dev build didn't have the Mullvad Extension, so I didn't catch a problem you get with a similar configuration: extensions cannot add providers with the same name as a built-in one (maybe to avoid phishing).

The easiest solution is to remove the built-in Leta, and Rui wrote in the issue he's okay with that.

However, I think we could also patch the code so that arriving to a broken state of the whole search system just because of an extension becomes impossible.

I've already written a POC, in which we catch the exception when installing the extension's search engines, instead of having the caller handle it:

diff --git a/toolkit/components/search/SearchService.sys.mjs b/toolkit/components/search/SearchService.sys.mjs
index 84ba5c40377e..62ff0a4e2b56 100644
--- a/toolkit/components/search/SearchService.sys.mjs
+++ b/toolkit/components/search/SearchService.sys.mjs
@@ -2700,7 +2700,14 @@ export class SearchService {
         ":",
         locale
       );
-      engines.push(await installLocale(locale));
+      try {
+        engines.push(await installLocale(locale));
+      } catch (err) {
+        lazy.logConsole.error(
+          `Could not install the search engine of ${extension.id}`,
+          err
+        );
+      }
     }
     return engines;
   }

We should decide if we want to break the installation of all the locales when the first one fails and maybe even uninstalling any successfully installed one.

Let me know if you want me to include it in this MR 🙂 .