1972068 - Convert Default search engine to config-based prefs

Status: RESOLVED FIXED

(Firefox :: Settings UI, task)

Description

[Mark Striemer [:mstriemer]]

The Default search engine section on the Search settings page should be converted to the config-based settings system.

These changes can be made on the existing page. The end result should be the following options:

• Default search engine select
• Show search terms instead of URL on default search engine results page checkbox

Comment 1

[Stephanie Cunnane [:scunnane]]

Note: when the browser.search.separatePrivateDefault.ui.enabled pref is true, the user sees extra UI on the default search engine card on the current about:preferences#search pane. I checked with Paul Annett, who said this extra UI should be ported over from the old search settings to the new config-based settings.

Comment 2

[Stephanie Cunnane [:scunnane]]

Attachment: Default search engine card with separate private default engine UI.png

Comment 3

[Stephanie Cunnane [:scunnane]]

Attachment: Bug 1972068 - Convert default search engine section of search settings to config-based prefs. r?Standard8!

Comment 4

[Mark Striemer [:mstriemer]]

Attachment: WIP: Bug 1972068 - Part 0: Support more props in setting control (just support controlAttrs instead...)

Comment 5

[Mark Striemer [:mstriemer]]

Attachment: WIP: Bug 1972068 - start search settings

Comment 6

[Stephanie Cunnane [:scunnane]]

Hi Adrian, as part of the settings redesign project, I have reworked the default search engine settings to use config-based prefs rather than hard-coded markup. Could someone in QA use this build to confirm that the current default search engine settings functionality is preserved in the new config-based prefs version?

Please note that there are 3 known issues on the reusable components side (see this Slack thread for more details) .

1. the label sizing for "This is your default search engine. You can change it at any time" is too small
2. the widths of the 2 dropdowns are incorrect
3. when the dropdowns are open, the search engine icons are missing

Please also note that I've slightly updated the default search engine card copy per the new Figma design.
Thanks in advance for taking a look at this, much appreciated!

EDIT: I've just gotten some feedback from the recomp team, so I'm going to work through those changes first, then ask for QA to take a look at the default search engine card.

Comment 7

[Stephanie Cunnane [:scunnane]]

Hi Adrian, as part of the settings redesign project, I have reworked the default search engine settings to use config-based prefs rather than hard-coded markup. Could someone in QA use this build to confirm that the current default search engine settings functionality is preserved in the new config-based prefs version?

Please note that there is one known issue where the dropdowns don't yet show the search engine icons. This is being worked on on the reusable components side, and we won't be landing this patch until the search engine icons appear.

Thanks in advance for taking a look at this, much appreciated!

Comment 8

[Adrian Florinescu [:aflorinescu]]

The try build only has an Ubuntu artefact, is it fine if we take a look just on Ubuntu? If win/mac eyes are required, could you provide a try covering try builds for those as well?

Comment 9

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Hi Stephanie,
We conducted exploratory testing on the trybuild provided in Comment 7 on Ubuntu 22 (RS Stage). Some notes whilst testing the trybuild:

• The “This is your default search engine. You can change it at any time” label is now missing from the about:preferences#search page.
• Selector search engine dropdown is too wide (known issue as per Comment 6);
• There are no icons in the search engine dropdown for both the default engine and the private default engine dropdowns (known issue as per in Comment 7);
• Search functionality is working as expected; there is some lag on occasion whilst loading the SERP.
• Performance wise, Memory data is pretty normal, but CPU spikes happen ( up to 120%) . Nothing major as the search queries and SERP’s were displayed fine afterwards. We can see the same behavior on the latest nightly build as well.

Please let us know if there’s anything else we should take into account.

Comment 10

[Stephanie Cunnane [:scunnane]]

Adrian, this should be fine to just test with an Ubuntu build. I just wanted to do a sanity check test to verify that the old default search engine functionality was being preserved as I migrated to the new config-based prefs. In the coming months, there will be much more extensive settings redesign testing at the project-wide level handled by the recomp QA team.

Ardelean, thanks very much for taking a look here.

• That description was removed intentionally, as it's now been removed from the finalized Figma design
• For the CPU spikes, I'll wait about a week to see if this is fixed in Nightly, then send you a new build to retest. If this remains a problem, I'll point it out in the SRD project Slack channel.

Comment 11

[Stephanie Cunnane [:scunnane]]

Hi Ardelean,

Here's a new Linux build for this patch. Do you mind checking for the CPU spikes mentioned in Comment 9 above? If they are still there, could you send me a profile showing the spikes? I'll then discuss this with the SRD project team. Thanks in advance!

Comment 12

[Stephanie Cunnane [:scunnane]]

Hi Adrian, I just wanted to follow up on the comment above - do you mind checking this new build for any CPU spikes? Much appreciated!

Comment 13

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Hi Stephanie, apologies for the delay!

We conducted further testing on the new trybuild provided in Comment 12. Performance wise CPU is still capping over 100%, especially when using the Unified Search Button and/or accessing the about:preferences#search page from the same entry point. There is also some minor lag present. We also tested the latest Firefox Nightly build without any RS connection and default settings - new profile - and the performance is definitely better on the latest Nightly build.

Here are the profiles we managed to capture:

• https://share.firefox.dev/3MaBkFC
• https://share.firefox.dev/48lvI42
• https://share.firefox.dev/3LMZ8zt

Tests were performed on two different Ubuntu 22 machines on RS Stage.

Please let us know if there’s anything else we should account for.

Comment 14

[Stephanie Cunnane [:scunnane]]

Hi Ardelean, thanks for looking into this, much appreciated. The build I sent in comment 12 was actually a debug build. I'm not sure the degree to which a debug vs. optimized build would affect this CPU spike, so do you mind comparing profiles for the two builds here head to head? Apologies for all the back and forth - I probably should have sent both to begin with!

Comment 15

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Here the profiles for both builds:

• debug: https://share.firefox.dev/4igsYIL
• trybuild: https://share.firefox.dev/49ygbPG

The debug build is slower and CPU spikes are more prominent.

Regarding the trybuild, performance and search functionality is more optimised and there’s no lag present. We performed testing against the latest Firefox Nightly build - new profile - and the CPU spikes and overall performance is comparable between the two.

Both the debug and the trybuild were tested with RS Stage connection on Ubuntu 22.

Please let us know if we need to cover anything else, thank you!

Comment 16

[Stephanie Cunnane [:scunnane]]

Perfect, thanks for comparing these 2 builds - much appreciated. Since we're not seeing the performance issues in the optimized build, there's no real issue here that needs to be further investigated.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f058bb0a22b2
https://hg.mozilla.org/integration/autoland/rev/b9169f82b28e
Convert default search engine section of search settings to config-based prefs. r=Standard8,fluent-reviewers,desktop-theme-reviewers,hjones,bolsson

Comment 18

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/b9169f82b28e