Omnibox in a non-english Tor Browser has no Disconnect.me as search engine in 6.0a5

added GeorgKoppen201605 TorBrowserTeam201605R component::applications/tor browser ff45-esr owner::gk priority::very high resolution::fixed severity::critical status::closed type::defect labels

Trac:
Keywords: TorBrowserTeam201604 deleted, GeorgKoppen201604 TorBrowserTeam201604 added
Owner: tbb-team to gk
Status: new to assigned

I noticed that in our "Omnibox patch" we add various *.xml files (search engine definitions) to the directory browser/locales/en-US/searchplugins/. So it would seem that this patch is specifically adding search engines for the US English locale only. I'm not sure why this approach would have provided the correct search engines in non-English in TBB/ESR38, but it at least seems like a plausible explanation for the problem we see in TBB/ESR45.

There's another directory, browser/locales/generic/, that at least superficially looks like it may provide its contents to every locale. So I added a matching searchplugins directory and added disconnect.xml and youtube.xml therein. I'm currently building the full tor-browser-bundle.git with this change, and I'll report the results soon.

Thanks to fqueze for debugging help! It seems that https://bugzilla.mozilla.org/show_bug.cgi?id=1162569 is the likely culprit.

We have a bunch of options here

We override re[/search-plugins/](/search-plugins/`) to point to a custom location
We ship our search plugins in a distribution folder
We replace the lang pack search engines and list.txt with the ones we want to get used during the bundling step.

I think I prefer 3) for at least two reasons: a) We can be sure that only our plugins get shipped. No other code path can use search plugins we don't want to get used. b) we avoid another possible Tor Browser patch.

I forgot to mention that fqueze thinks all three approaches have some kind of fragility involved, thus that argument does not count here. :)

Moving tickets

Trac:
Keywords: GeorgKoppen201604 TorBrowserTeam201604 deleted, GeorgKoppen201604, TorBrowserTeam201605 added

Moving things for me to May.

Trac:
Keywords: GeorgKoppen201604 deleted, GeorgKoppen201605 added

bug_18915_v2 (https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/commit/?h=bug_18915_v2&id=9ec08e87c4ce181fa79f8d5355a471ba8939788e) is up for review. I tested this with a hardened-nightly and on all platforms we support by rebundling an alpha.

Adding anonym, as I don't know how Tails is doing the localization and whether they are using the lang packs we ship or not.

Trac:
Cc: mcs, brade, arthuredelstein to mcs, brade, arthuredelstein, anonym
Keywords: TorBrowserTeam201605 deleted, TorBrowserTeam201605R added
Status: assigned to needs_review

Replying to gk:

Adding anonym, as I don't know how Tails is doing the localization and whether they are using the lang packs we ship or not.

Thanks for thinking about us! :)

In Tails we do localization essentially like this:

We install all your langpacks in the extensions directory, then Tor Browser picks the right one according to the LANG env var. Furthermore, it will pick localized search plugins from distribution/searchplugins/locale/$LANGCODE/ (used below).
We dynamically generate localized search plugins for Disconnect, Startpage and Wikipedia, replacing your variants of these. Since we provide the English Wikipedia plugin for all locales, we generate localized icons (using imagemagick) for Wikipedia, by putting the lang code in the bottom-right corner.
We also install all search plugins from Debian's ìceweasel-l10n-* for which we have langpacks. (Personally I'd happily drop these since they mainly seem to add commercial crap.)

So, we are not affected by this bug since we generate our own thing. However, I worked on integrating 6.0a5 into Tails yesterday and noticed a related issue (for us), namely that you now put your search engine plugins inside distribution/omni.ja, which makes it harder for us to replace them. I guess the best, touch, would be if we settled on the same search plugins, and we did nothing regarding this on Tails' side. I also realize that this is not the right ticket for this -- if you think there's an interest in harmonizing this I can open a new ticket.

Replying to anonym:

Replying to gk:

Adding anonym, as I don't know how Tails is doing the localization and whether they are using the lang packs we ship or not.

Thanks for thinking about us! :)

In Tails we do localization essentially like this:

We install all your langpacks in the extensions directory, then Tor Browser picks the right one according to the LANG env var. Furthermore, it will pick localized search plugins from distribution/searchplugins/locale/$LANGCODE/ (used below).

Okay, you should be fine then I think.

We dynamically generate localized search plugins for Disconnect, Startpage and Wikipedia, replacing your variants of these. Since we provide the English Wikipedia plugin for all locales, we generate localized icons (using imagemagick) for Wikipedia, by putting the lang code in the bottom-right corner.

We also install all search plugins from Debian's ìceweasel-l10n-* for which we have langpacks. (Personally I'd happily drop these since they mainly seem to add commercial crap.)

So, we are not affected by this bug since we generate our own thing. However, I worked on integrating 6.0a5 into Tails yesterday and noticed a related issue (for us), namely that you now put your search engine plugins inside distribution/omni.ja, which makes it harder for us to replace them. I guess the best, touch, would be if we settled on the same search plugins, and we did nothing regarding this on Tails' side. I also realize that this is not the right ticket for this -- if you think there's an interest in harmonizing this I can open a new ticket.

If that helps you then let's look at it. However, I am not sure I understand you. We actually did not change anything with our switch from ESR38 to ESR45 which is why we have this bug :). Specifically, we don't put our search engines into distribution/omni.ja. If I extract e.g. the Linux 32bit en-US bundle, the only omni.ja files I get are Browser/omni.ja, Browser/browser/omni.ja and Browser/webapprt/omni.ja. The search engines are still in the second one.

Kathy and I reviewed these changes (although we did not run a bundling step with them). The changes look fine except we think that the chrome directory should be cleaned up after extracting the search plugin files from omni.ja (as is done for defaults), e.g., after: mv chrome/en-US/locale/browser/searchplugins ~/build add: rm -rf chrome

Trac:
Status: needs_review to needs_revision

Good catch. I changed the patched and merged the result to master (commit 94c6a1c73291f8f456249d9719de57ff002fe6fe) and hardened-builds (commit 467796a7b7901fc22f26716860ed6ed8d4102416).

Trac:
Status: needs_revision to closed
Resolution: N/A to fixed