Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default search should be versioned #12231

Closed
bsclifton opened this issue Oct 20, 2020 · 2 comments · Fixed by brave/brave-core#7090
Closed

Default search should be versioned #12231

bsclifton opened this issue Oct 20, 2020 · 2 comments · Fixed by brave/brave-core#7090
Assignees

Comments

@bsclifton
Copy link
Member

bsclifton commented Oct 20, 2020

Test plan

See brave/brave-core#7090

Description

Right now, there's not a way to tell what the default search engine was when the user's profile was created.
If the default is changed, anyone who has not set the search engine explicitly will get the NEW default.

Steps to Reproduce

  1. Fresh profile
  2. Take note of the default search engine Brave uses when searching on regular window/tabs (ex: Google)
  3. Have a developer update the code to change the default for that region (ex: Bing)

Actual result:

It doesn't matter if person had been using the default. The new default will be used instead (ex: Bing). The only way the value is retained is when they have changed the default explicitly

Expected result:

If user profile has existed for a while, the original provider for default search engine should be preserved (ex: Google in this case)

Reproduces how often:

100%

@bsclifton bsclifton added feature/global-settings Settings at browser level independent of shields settings feature/search OS/Android Fixes related to Android browser functionality OS/Desktop labels Oct 20, 2020
@bsclifton bsclifton self-assigned this Oct 20, 2020
bsclifton added a commit to brave/brave-core that referenced this issue Nov 16, 2020
bsclifton added a commit to brave/brave-core that referenced this issue Nov 17, 2020
bsclifton added a commit to brave/brave-core that referenced this issue Nov 20, 2020
@bsclifton bsclifton added this to the 1.19.x - Nightly milestone Nov 21, 2020
@LaurenWags
Copy link
Member

Desktop can be tested via test plan in brave/brave-core#7090, however pending Android specific test plan from @bsclifton 👍

@btlechowski
Copy link

btlechowski commented Dec 9, 2020

Verification passed on

Brave 1.18.69 Chromium: 87.0.4280.88 (Official Build) (64-bit)
Revision 89e2380a3e36c3464b5dd1302349b1382549290d-refs/branch-heads/4280@{#1761}
OS Windows 7 Service Pack 1 (Build 7601.24544)

Verified test plan from brave/brave-core#7090 (some verification were done in #12327)

Verified Russia on upgraded profile from 1.17.x
On 1.17.x google is default
image
On 1.18.x google is default, but Yandex has been added to suggested list
image


Verification passed on LG Nexus 5 with Android 5.1 running 1.18.70 Bravearm.apk

Verified test plan from brave/brave-core#7090 (comment) for Armenian and Russian.

For a new profile, confirmed that Yandex was selected as the default search engine for Armenian and Russian (Uzbek not available on device). Confirmed Yandex is default in Settings and selected by default on Onboarding.

For existing profile with default SE never modified, confirmed that updating from 1.17.75 CR: 87.0.4280.88 --> 1.18.70 Chromium: 87.0.4280.101 didn't replace the previous default SE of Google with Yandex. Tested for both Armenian and Russian.

For existing profile with default SE changed, confirmed that updating from 1.17.75 CR: 87.0.4280.88 --> 1.18.70 Chromium: 87.0.4280.101 didn't replace the previous selected SE of DDG with Yandex. Tested for both Armenian and Russian.

Note: Some of these regions don't have any localization but I ensured that:

  • Yandex is being set as the default
  • Yandex is available via the SE onboarding

Armenian new profile

Onboarding Settings
Armenian Onboarding Armenian Settings

Russian new profile

Onboarding Settings
Russian Onboarding Russian Settings

Additional testing notes can be found under #12979 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants