Fix Omnibox for TBB/52ESR
The Omnibox code different in Firefox 52ESR vs 45ESR. We need to rewrite our patch for Omnibox.
Activity
Are you referring to the following two patches?
Replying to mcs:
Are you referring to the following two patches?
Sorry, I somehow missed this question. Yes, the implementation patch needs to be fixed. I think the testing patch may still work.
Mozilla changed their search engine list format so I am currently grappling with that.
Here's my patch. Unfortunately it only applies currently to the US English locale. I will need to investigate how to update our tor-browser-bundle.git code (a la #18915 (moved)). But if this patch looks OK I will add it to our #20680 (moved) branch:
https://github.com/arthuredelstein/tor-browser/commit/21309
Trac:
Status: new to needs_review-
Do you know if the
browser.search.*prefs are set from the values that are included inbrowser/locales/search/list.json?Also, do we need to change the en-US
USandCAentries in that file? I assume the browser uses theUSentries if it detects that the user is located in the USA and CA if in Canada (although maybe we disabled that kind of location detection? I cannot remember).For non-en-US locales, will it work to just patch
browser/locales/search/list.jsonfor each locale that we care to change? That won't work if the language packs override those settings though. Replying to mcs:
Do you know if the
browser.search.*prefs are set from the values that are included inbrowser/locales/search/list.json?Yes, they appear to be.
Also, do we need to change the en-US
USandCAentries in that file? I assume the browser uses theUSentries if it detects that the user is located in the USA and CA if in Canada (although maybe we disabled that kind of location detection? I cannot remember).I believe that location detection is disabled, because we have browser.search.geoSpecificDefaults set to false.
For non-en-US locales, will it work to just patch
browser/locales/search/list.jsonfor each locale that we care to change? That won't work if the language packs override those settings though.Unfortunately it doesn't work -- the language packs each contains a list.json file that overrides these settings. So we'll need to update the #18915 (moved) patch as I mentioned in comment:7.
-
Replying to arthuredelstein:
Unfortunately it doesn't work -- the language packs each contains a list.json file that overrides these settings. So we'll need to update the #18915 (moved) patch as I mentioned in comment:7.
Here is a radical idea: modify browser/locales/search/list.json to only contain a "default" entry and remove the list.json file from each language pack during our packaging phase. I don't know if everything would work correctly if we did that, but it might.
Replying to mcs:
Replying to arthuredelstein:
Unfortunately it doesn't work -- the language packs each contains a list.json file that overrides these settings. So we'll need to update the #18915 (moved) patch as I mentioned in comment:7.
Here is a radical idea: modify browser/locales/search/list.json to only contain a "default" entry and remove the list.json file from each language pack during our packaging phase. I don't know if everything would work correctly if we did that, but it might.
I think that's a good suggestion and it might work. I will give it a try.
Replying to mcs:
Replying to arthuredelstein:
Unfortunately it doesn't work -- the language packs each contains a list.json file that overrides these settings. So we'll need to update the #18915 (moved) patch as I mentioned in comment:7.
Here is a radical idea: modify browser/locales/search/list.json to only contain a "default" entry and remove the list.json file from each language pack during our packaging phase. I don't know if everything would work correctly if we did that, but it might.
For some reason removing all but the default entry doesn't work, even for US English. nsSearchService.js is pretty convoluted so I'm not sure why. My current approach is to change all locales we deploy to include the same search engine list, and then rely on our #18915 (moved) patches for tor-browser-bundle.git to copy these search engines and the list to each locale xpi. It's awkward, but it seems like it should work. Unfortunately I'm having trouble getting tor-browser-bundle.git to build right now, so I haven't fully tested this. But here is the patch in any case:
https://github.com/arthuredelstein/tor-browser/commit/21309+1
The reason the Omnibox was redesigned is the Omnibox API riding the trains to Firefox 52 which has privacy implications and may be worth investigating. https://bugzilla.mozilla.org/show_bug.cgi?id=1166831
-
Replying to cypherpunks:
The reason the Omnibox was redesigned is the Omnibox API riding the trains to Firefox 52 which has privacy implications and may be worth investigating. https://bugzilla.mozilla.org/show_bug.cgi?id=1166831
This API is for WebExtensions, which means there should be no privacy implications unless someone installs an add-on that they do not trust (something we definitely do not recommend).
-
Replying to arthuredelstein:
For some reason removing all but the default entry doesn't work, even for US English. nsSearchService.js is pretty convoluted so I'm not sure why. My current approach is to change all locales we deploy to include the same search engine list, and then rely on our #18915 (moved) patches for tor-browser-bundle.git to copy these search engines and the list to each locale xpi. It's awkward, but it seems like it should work. Unfortunately I'm having trouble getting tor-browser-bundle.git to build right now, so I haven't fully tested this. But here is the patch in any case:
https://github.com/arthuredelstein/tor-browser/commit/21309+1
This patch looks okay, except should the URLs in ddg-onion.xml be https instead of http?
Regarding the changes needed for packaging (builders/tor-browser-bundle.git), Kathy and I wonder if it will work to simply remove
browser/chrome/$LOCALE/locale/browser/searchpluginsfrom each of the language packs. But we have not tried it. Replying to mcs:
Replying to arthuredelstein:
For some reason removing all but the default entry doesn't work, even for US English. nsSearchService.js is pretty convoluted so I'm not sure why. My current approach is to change all locales we deploy to include the same search engine list, and then rely on our #18915 (moved) patches for tor-browser-bundle.git to copy these search engines and the list to each locale xpi. It's awkward, but it seems like it should work. Unfortunately I'm having trouble getting tor-browser-bundle.git to build right now, so I haven't fully tested this. But here is the patch in any case:
https://github.com/arthuredelstein/tor-browser/commit/21309+1
This patch looks okay, except should the URLs in ddg-onion.xml be https instead of http?
Good point. I changed that.
Regarding the changes needed for packaging (builders/tor-browser-bundle.git), Kathy and I wonder if it will work to simply remove
browser/chrome/$LOCALE/locale/browser/searchpluginsfrom each of the language packs. But we have not tried it.I tried this and couldn't get it to work. It seems Firefox doesn't fall back to the default plugins list as I would have expected. But I could be missing something. In any case I added the patch to my 20680 branch.
Trac:
Status: needs_review to closed
Resolution: N/A to fixedmentioned in issue #21851 (moved)
mentioned in issue tpo/applications/tor-browser#21851 (closed)
