1800662 - Search engine order is modified after region change

Status: VERIFIED FIXED

(Firefox :: Search, defect, P3)

Description

[Petruta Horea [:phorea], Desktop Test Engineering]

Attachment: search engines order.png

Found in

• Firefox 108 beta 1

Affected versions

• Firefox 108 beta 1
• latest Nightly 109.0a1 2022-11-15

Tested platforms

• Affected platforms: Win 10 64-bit, macOSX 10.14, Ubuntu 22.04
• Unaffected platforms: -

Steps to reproduce

1. Open Firefox with a clean profile
2. Go to youtube.com (or any other page that offers an open search engine) and add Youtube as search engine using address bar
3. Go to about:config and change browser.search.region value to a different one (our tests included RO->US and US->RO)

Expected result

• Search engines order remain the same when the region is changed

Actual result

• Amazon is moved towards the end of the list

Regression range

• DEBUG : Found commit message:
  Bug 1786324 - Add migration for engine ids to be saved on user settings.r=Standard8

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

:mcheang, since you are the author of the regressor, bug 1786324, could you take a look?

For more information, please visit auto_nag documentation.

Comment 2

[Mandy Cheang [:mcheang][she/her]]

We haven't found an obvious cause for this regression yet. It also doesn't seem urgent to fix at the moment and we'll keep an eye on it to further investigate the issue.

Comment 3

[Mark Banner (:standard8)]

With build id 20221101093931 (just before bug 1786324 landed), I reproduced this with a en-US build, and changing the region from GB to RU. It does not reproduce with a US to RU switch.

With the GB to RU switch, the issue is that we're switching from Amazon.co.uk to Amazon.com, which are seen as different search engines. Amazon.co.uk has the number 2 slot, YouTube has taken the last slot. When we switch to RU, Amazon.com has got no slot, so it gets added after the last one which has an allocated slot, i.e. YouTube.

Prior to the id changes landed in bug 1786324, the settings were indexed by the search engine name. For both US and RU, we have Amazon.com as the name, so they would share the settings relating to the order. Now they are indexed by id, they are seen as different engines because they use different locales from the application provided engines (the app provided ones use us and en).

I think the main issue here is that installing an additional search engine moves us into a "user ordered" mode - even though the user hasn't intentionally ordered any engine, we still behave like they have. Therefore, I think we should reconsider when we switch to an ordered mode, which would at least help with this.

I'm not sure there's much we can do for this when we are in a user ordered mode. We could potentially fill out gaps before appending, but I doubt that'd do the right thing in some circumstances.

Comment 4

[Mandy Cheang [:mcheang][she/her]]

Attachment: Bug 1800662 - Fix search engine order when locale is changed.r=Standard8

Comment 5

[Mark Banner (:standard8)]

Attachment: Bug 1800662 - Add test to ensure search settings are persisted when app engines change locale but keep the same name. r?mcheang!

Depends on D163384

Comment 6

[Mandy Cheang [:mcheang][she/her]]

Mark and I were investigating this yesterday and we thought of a solution to fix the regression. The patch is up.

The problem:
Switching from Regions US -> RO and Regions RO <- US changes the engine ids because these Regions have different locales.
In the search-config
The locales for US is us
The locales for RO is default

So what's causing the issue is that we now have two different engine ids (engine ids are a combination of webextension id plus the locale)
The engine id for amazon.com in the US Region with locale us is [e-mail]
The engine id for amazon.com in the RO Region with locale default is [e-mail]

Effectively treating these as different engines because their ids are different and the ordering for amazon won't persist once you switch from US to RO.

Quick solution to fix regression:
Our quick solution is to call #getEngineByName when we load engines from settings to prevent the ordering being out of place for locale switches in the same Region.

However this issue was a problem before the engine id patch in bug 1786324, because our quick fix doesn't solve mis-odering of amazon when we do a switch from GB -> RU. This is when we change from amazon.uk to amazon.com and amazon.com does not retain the order. The locales are the same but the regions are different. This was an existing issue not regressed by bug 1786324. In order to provide a permanent fix, we need more work on this as described below.

Permanent Solution:
There's more work and investigation needed to re-work the way we are keeping track of the user's search engines orders. We need to perhaps change when the user is in user search order mode vs. application search order mode. There's been discussion about writing up a proposal to think through how to do this re-work.

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 9305913 [details]
Bug 1800662 - Fix search engine order when locale is changed.r=Standard8

Beta/Release Uplift Approval Request

• User impact if declined: If the region is changed, or possibly some combinations of locale (e.g. via language switching), then the order of some search engines may change unexpectedly.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 0.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This effectively reverts part of the patch that caused the regression to work the way it use to previously.
• String changes made/needed: None
• Is Android affected?: No

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ab560bf7868b
Fix search engine order when locale is changed.r=Standard8
https://hg.mozilla.org/integration/autoland/rev/09a409fa17cb
Add test to ensure search settings are persisted when app engines change locale but keep the same name. r=mcheang

Comment 9

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/ab560bf7868b
https://hg.mozilla.org/mozilla-central/rev/09a409fa17cb

Comment 10

[Dianna Smith [:diannaS]]

Comment on attachment 9305913 [details]
Bug 1800662 - Fix search engine order when locale is changed.r=Standard8

Approved for 108.0b9

Comment 11

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/eaef2f2c5ed5
https://hg.mozilla.org/releases/mozilla-beta/rev/c40ebf17bcc8

Comment 12

[Petruta Horea [:phorea], Desktop Test Engineering]

Verified the scenario described in comment 0 using Firefox 108 beta 9 and latest Nightly 109.0a1 2022-12-05 under Win 10 64-bit, Mac OSX 10.14 and Ubuntu 22.04 64-bit.