Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Bug] Search engine issues on non-EN locales #12544

Closed
sv-ohorvath opened this issue Jul 14, 2020 · 8 comments
Closed

[Bug] Search engine issues on non-EN locales #12544

sv-ohorvath opened this issue Jul 14, 2020 · 8 comments
Labels
🐞 bug Crashes, Something isn't working, .. E3 Estimation Point: average, 2 - 3 days eng:qa:verified QA Verified Feature:HomeScreen Feature:Search S1 Blocks development/testing, may impact more than 25% of users, causes data loss, potential chemspill
Projects
Hershey's 🍫

Comments

@sv-ohorvath
Copy link
Contributor

sv-ohorvath commented Jul 14, 2020 •
edited by data-sync-user
Loading

Steps to reproduce

Scenario 1:

  1. Set the device language to Russian.
  2. Clean app install, open the app.
  3. Observe the default search engine on the first run.
  4. Restart the app. Observe the search engine.
    1. Expected behavior
      For some non-EN locales, the default engine should be Yandex or Baidu(?).
    2. Actual behavior
      On first launch, the engine is set right as Yandex. After a restart, it got changed to Google.

Scenario 2: From #11875 (comment)

Device information

  • Android device: Pixel 4 (Android 10)
  • Fenix version: 79.0-beta6.

┆Issue is synchronized with this Jira Task
@sv-ohorvath sv-ohorvath added 🐞 bug Crashes, Something isn't working, .. Feature:HomeScreen Feature:Search S1 Blocks development/testing, may impact more than 25% of users, causes data loss, potential chemspill labels Jul 14, 2020
@github-actions github-actions bot added the needs:triage Issue needs triage label Jul 14, 2020
@sv-ohorvath sv-ohorvath mentioned this issue Jul 14, 2020
Closed
@eliserichards eliserichards removed the needs:triage Issue needs triage label Jul 14, 2020
@s-ankur
Copy link
Contributor

s-ankur commented Jul 14, 2020

Related #12236

Mugurell added a commit that referenced this issue Jul 16, 2020
@Mugurell
For #12544 - Prevent search engines mixup
d449184 
Stricter synchronization by always using the same "loadedSearchEngines"
variable.
With "loadedSearchEngines" calling "refreshAsync()" we also get the fallback
engines to contain reddit and youtube (which are programatically added) and
also now we properly remember and display the engines added by user.
@sv-ohorvath
Copy link
Contributor Author

sv-ohorvath commented Jul 17, 2020 •
edited
Loading

We've verified the fix on Nightly 7.17 and both scenarios work now as expected.
LE: Also verified on Fenix 79.0-beta8 & beta 9 builds.

@sv-ohorvath sv-ohorvath added the eng:qa:verified QA Verified label Jul 17, 2020
pocmo pushed a commit that referenced this issue Jul 17, 2020
@Mugurell @pocmo
For #12544 - Prevent search engines mixup
0a4a170 
Stricter synchronization by always using the same "loadedSearchEngines"
variable.
With "loadedSearchEngines" calling "refreshAsync()" we also get the fallback
engines to contain reddit and youtube (which are programatically added) and
also now we properly remember and display the engines added by user.
@Mugurell Mugurell added this to Done in Hershey's 🍫 Jul 17, 2020
This was referenced Jul 17, 2020
Closed
Closed
@BranescuMihai BranescuMihai added the E3 Estimation Point: average, 2 - 3 days label Jul 20, 2020
@liuche liuche mentioned this issue Jul 20, 2020
Closed
9 tasks
@data-sync-user data-sync-user changed the title [Bug] Search engine issues on non-EN locales FNX3-13535 ⁃ [Bug] Search engine issues on non-EN locales Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX3-13535 ⁃ [Bug] Search engine issues on non-EN locales FNX-13625 ⁃ [Bug] Search engine issues on non-EN locales Aug 11, 2020
Hershey's 🍫 automation moved this from Done to QA Review Aug 11, 2020
@Mugurell Mugurell moved this from QA Review to Done in Hershey's 🍫 Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX-13625 ⁃ [Bug] Search engine issues on non-EN locales FNX2-15318 ⁃ [Bug] Search engine issues on non-EN locales Aug 11, 2020
@Mugurell
Copy link
Contributor

Closing as per #12544 (comment)

Hershey's 🍫 automation moved this from Done to QA Review Aug 12, 2020
@Mugurell Mugurell moved this from QA Review to Done in Hershey's 🍫 Aug 17, 2020
@boek boek reopened this Sep 16, 2020
boek added a commit to boek/fenix that referenced this issue Sep 16, 2020
@boek
Revert "For mozilla-mobile#12544 - Prevent search engines mixup"
b933d3d 
This reverts commit 0a4a170.
@boek
Copy link
Contributor

boek commented Sep 17, 2020

@sv-ohorvath I believe this is working as intended.

When we moved to MLS to determine the set of search engines to use it caused some performance issues due to having to wait for a network request to select the region. So as a work around we load a fallback set from the device locale until that request is completed which looks like what you're experiencing here

@sv-ohorvath
Copy link
Contributor Author

So, to clarify: before this, having the device language set to Russian was setting Yandex as default and kept it that way. And now this will be the expected behavior?:

Actual behavior
On first launch, the engine is set right as Yandex. After a restart, it got changed to Google.

@boek
Copy link
Contributor

boek commented Sep 18, 2020

@sv-ohorvath Correct, If Google is the default browser for the region you tested in (by using MLS) it will override the search engines set by your devices locale

boek added a commit that referenced this issue Sep 18, 2020
@boek
Revert "For #12544 - Prevent search engines mixup"
8bdc9d0 
This reverts commit 0a4a170.
@sv-ohorvath
Copy link
Contributor Author

sv-ohorvath commented Sep 21, 2020 •
edited
Loading

I see the revert was merged into Nightly so I've verified the following cases:

  • VPN region set to Turkey, device language EN: Google search engine at first run and after a restart
  • VPN set to Turkey, device language TR: Yandex search engine at first run and after a restart
  • VPN set to Romania, device language TR: Yandex search engine at first run and Google after a restart
  • VPN set to Romania, device language EN: Google search engine at first run and after a restart

@boek if that looks good, should I close this issue now?

boek added a commit to boek/fenix that referenced this issue Sep 28, 2020
@boek
Revert "For mozilla-mobile#12544 - Prevent search engines mixup"
027671a 
This reverts commit 0a4a170.
st3fan pushed a commit that referenced this issue Sep 28, 2020
@boek
Uplifting mls revert (#15500)
d0abf58 
* Revert "For #12544 - Prevent search engines mixup"

This reverts commit 0a4a170.

* For mozilla-mobile/android-components#8429 - Dont initialize MLS if token is null
@amedyne
Copy link
Contributor

amedyne commented Jan 7, 2021

@sv-ohorvath Closing this per comments last September 21.

@amedyne amedyne closed this as completed Jan 7, 2021
Hershey's 🍫 automation moved this from Done to QA Review Jan 7, 2021
@sflorean sflorean moved this from QA Review to Done in Hershey's 🍫 Jan 12, 2021
@data-sync-user data-sync-user changed the title FNX2-15318 ⁃ [Bug] Search engine issues on non-EN locales [Bug] Search engine issues on non-EN locales May 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
🐞 bug Crashes, Something isn't working, .. E3 Estimation Point: average, 2 - 3 days eng:qa:verified QA Verified Feature:HomeScreen Feature:Search S1 Blocks development/testing, may impact more than 25% of users, causes data loss, potential chemspill
Projects
No open projects
Hershey's 🍫
  
Done
Milestone
No milestone
Development

No branches or pull requests
10 participants
@st3fan @kbrosnan @boek @BranescuMihai @sv-ohorvath @Mugurell @s-ankur @eliserichards @data-sync-user @amedyne