mozilla-mobile / fenix

⚠️ Fenix (Firefox for Android) moved to a new repository. It is now developed and maintained as part of: https://github.com/mozilla-mobile/firefox-android
https://github.com/mozilla-mobile/firefox-android
Mozilla Public License 2.0
6.47k stars 1.28k forks source link

[Bug] Fenix not setting default search engine properly #3869

Closed boek closed 4 years ago

boek commented 5 years ago

Steps to reproduce

Set locale to RU or CN

Expected behavior

Default search engine is not Google

Actual behavior

Default search engine is Google

Device information

any

┆Issue is synchronized with this Jira Task

AndiAJ commented 5 years ago

Hi, I can still reproduce this issue on the latest Fenix Build #11850611 from 4/7 using a Google Pixel 3XL (Android 9) as it follows:

Russian ✅ OK - Yandex ► **Settings:** ![Screenshot_20190704-110149](https://user-images.githubusercontent.com/51314259/60652021-1184d100-9e50-11e9-8f02-26c25fdbf9f4.png) ► **Fenix default search engines:** ![Screenshot_20190704-110135](https://user-images.githubusercontent.com/51314259/60651930-e4d0b980-9e4f-11e9-9a22-f3fa5770c886.png)
Chinese Traditional 1st option ❌NOK - Google ► **Settings:** ![Screenshot_20190704-110829](https://user-images.githubusercontent.com/51314259/60652242-8952fb80-9e50-11e9-9e16-81c72ef338d7.png) ► **Fenix default search engines:** ![Screenshot_20190704-110913](https://user-images.githubusercontent.com/51314259/60652209-77715880-9e50-11e9-867f-4594d4835740.png)
Chinese Traditional 2nd option ❌NOK - Google ► **Settings:** ![Screenshot_20190704-111146](https://user-images.githubusercontent.com/51314259/60652605-3cbbf000-9e51-11e9-8352-960b02234727.png) ► **Fenix default search engines:** ![Screenshot_20190704-111218](https://user-images.githubusercontent.com/51314259/60652581-2f9f0100-9e51-11e9-9112-ddf1db5f8cd6.png)
Chinese Traditional 3rd option ❌NOK - Google ► **Settings:** ![Screenshot_20190704-111414](https://user-images.githubusercontent.com/51314259/60652717-6412bd00-9e51-11e9-80bf-add3968c4170.png) ► **Fenix default search engines:** ![Screenshot_20190704-111438](https://user-images.githubusercontent.com/51314259/60652764-742a9c80-9e51-11e9-8aeb-d5a7c9dba358.png)
Chinese Standard 1st option ✅ OK - Baidu ► **Settings:** ![Screenshot_20190704-111646](https://user-images.githubusercontent.com/51314259/60652856-9ae8d300-9e51-11e9-8d52-7d789880583f.png) ► **Fenix default search engines:** ![Screenshot_20190704-111732](https://user-images.githubusercontent.com/51314259/60652912-afc56680-9e51-11e9-96a0-c037b432e688.png)
Chinese Standard 2nd option ❌NOK - Google ► **Settings:** ![Screenshot_20190704-112025](https://user-images.githubusercontent.com/51314259/60652986-d2577f80-9e51-11e9-8e09-cba201b5d026.png) ► **Fenix default search engines:** ![Screenshot_20190704-112003](https://user-images.githubusercontent.com/51314259/60653017-e4d1b900-9e51-11e9-8e87-7bb52759b51e.png)
Chinese Standard 3rd option ❌NOK - Google ► **Settings:** ![Screenshot_20190704-112154](https://user-images.githubusercontent.com/51314259/60653075-003cc400-9e52-11e9-8d45-79e4a24e443f.png) ► **Fenix default search engines:** ![Screenshot_20190704-112228](https://user-images.githubusercontent.com/51314259/60653102-1054a380-9e52-11e9-8796-b7eab89e582b.png)
Chinese Standard 4th option ❌NOK - Google ► **Settings:** ![Screenshot_20190704-112426](https://user-images.githubusercontent.com/51314259/60653152-282c2780-9e52-11e9-8b89-3b44b2e28817.png) ► **Fenix default search engines:** ![Screenshot_20190704-112348](https://user-images.githubusercontent.com/51314259/60653195-3bd78e00-9e52-11e9-929c-bce084e2aec6.png)

@boek For each of the above mentioned languages you'll find attached pictures from the device settings as well as from Fenix. Also, I'm not sure if for each Chinese language (Traditional or Standard) and their options should be also covered?

boek commented 5 years ago

Pt 2 waiting on: https://github.com/mozilla-mobile/android-components/issues/3637

liuche commented 5 years ago

Half of this has been done and fixed, but waiting for more permanent location search-engine from AC.

Will remove from sprint.

k0st1x commented 5 years ago

How to use the eng language on device but use the Yandex search?

pocmo commented 5 years ago

In AC 10.0.0 we landed RegionSearchLocalizationProvider which does a GeoIP lookup to determine the region and is more accurate than locale. In order to use that we need to make sure that Fenix is only using "async" methods of SearchEngineManager since it may do a network request if no region is cached.

boek commented 4 years ago

Waiting on an MLS api key

severinrudie commented 4 years ago

@boek since this was moved into the backlog, does that mean that the key is now available? Where would I find it?

@pocmo if RegionSearchLocalizationProvider checks my current location, is there a way for me to verify RU and CN behavior without booking a flight?

@vesta0 to clarify requirements, is it expected that users who are currently in China use Baidu by default, or that users who have selected a Chinese locale use Baidu by default? Because these will require different approaches.

severinrudie commented 4 years ago

boek since this was moved into the backlog, does that mean that the key is now available? Where would I find it?

I spoke with boek, and it looks like we've requested the key but haven't heard back yet. We will refactor our SearchEngineManager calls to use the async API now, so once we get the key we can quickly drop it in.

vesta0 commented 4 years ago

@Baron-Severin to answer your question, we currently reference the system default locale so if a user's system default locale is zh-cn then their search engine default will be Baidu.

Once #220 is shipped, then user can change their locale but I believe search engine default will regardless follow the locale.

cc @mconnormoz in case further clarification needed

severinrudie commented 4 years ago

We expect that changes made in #5577 will fix this bug as well.

severinrudie commented 4 years ago

We expect that changes made in #5577 will fix this bug as well.

Correction, these changes did a lot of the prework that will enable this fix, but didn't actually fix the behavior. Moving this into the backlog, as it still needs to be done.

abodea commented 4 years ago

Hello, @boek @vesta0 is there a list where I can see what default search engine should be displayed for each language? I synced with Severin on slack but he couldn't help me. Note that for Chinese Traditional now is not Google displayed, but it's Bing. Before moving forward with the verification I wanted to make sure I have the right list for each language. I will remove the qa needed until further notice.

liuche commented 4 years ago

@flod do you know where the default search engine is listed per-locale?

vesta0 commented 4 years ago

Can we find out which devices or locales are impacted by this bug?

sv-ohorvath commented 4 years ago

@vesta0 Does this need QA investigation? If so, we could add the QA:needed label and have someone look at it.

liuche commented 4 years ago

This seems like the same STR as #12544 which was verified, so I'm going to close this as a dupe