1807594 - After switching search engine, search suggestions appear even when turned off in settings

Status: VERIFIED FIXED

(Firefox for Android :: Search, defect, P1)

Description

[Chris Peterson [:cpeterson]]

From github: https://github.com/mozilla-mobile/fenix/issues/25753.

Steps to reproduce

1. Go to "Settings" -> "Search" and ensure that "Show search suggestions" is turned off. Exit the Settings.
2. Start typing in the address bar.
3. Click the "Search engine" button, and then click on a different search engine in the menu.
4. Continue typing in the address bar.

Expected behaviour

Search suggestions should not appear because they are turned off in the settings.

Actual behaviour

Search suggestions do appear during step 4.

Device name

Google Pixel 3a

Android version

Android 12

Firefox release type

Firefox

Firefox version

102.1.1

Device logs

(I saved the logcat on my computer, but before sharing it, I'd need to review it for sensitive information, and I don't want to spend the time to do that unless/until the developers actually need the logcat, since the issue seems to be so easy to reproduce.)

Additional information

Screen recording (hosted on my own web server because the file is too big for GitHub).

The problem does not occur with Firefox 101.2.0, so this seems to be a regression. I consider it a privacy problem, so I plan to stay on Firefox 101.2.0 until it's fixed.

┆Issue is synchronized with this Jira Task

Change performed by the Move to Bugzilla add-on.

Comment 1

[Matt McCutchen]

FWIW:

I plan to stay on Firefox 101.2.0 until it's fixed.

I take this remark back and confess to exaggerating in an attempt to raise the priority of the issue. I'm satisfied with the workaround of entering my entire query before selecting a search engine; suggestions are shown, but the search engine doesn't receive any more data than it would in the absence of the bug. Staying on an old version of Firefox and missing out on security updates is a bad idea.

Comment 2

[Chris Peterson [:cpeterson]]

This is a regression we should fix. Adding to our backlog.

Comment 3

[BMO Automation]

Attachment: [mozilla-mobile/fenix] Bug 1807594 - Avoid showing search suggestions when show_search_suggestions preference is off (#28496)

Comment 4

[Chris Peterson [:cpeterson]]

Assigning this bug to Alexandru because he has a PR in review.

The GitHub issue says this was a regression in Fenix 102.

Comment 5

[Chris Peterson [:cpeterson]]

Adding back [fxdroid] whiteboard tag to sync this bug to Jira.

Comment 6

[BMO Automation]

Attachment: [mozilla-mobile/fenix] Bug 1807594 - Adds property to keep track of whether search suggestions are enabled in settings (#28504)

Comment 7

[Jason Prickett]

Will this be backported into the 110 branch?

Comment 8

[Chris Peterson [:cpeterson]]

Jeff, should we uplift your search suggestion fix to Beta 110? Your fix looks like safe one-liner and this is a privacy issue. (This bug was a regression in Fenix 102.)

We should probably hold off on uplifting until QA verifies the fix in Nightly 111. (They will then clears this bug's qe-verify+ flag.) After we uplift, we should set the qe-verify+ flag again to ask QA to verify the fix works in Beta 110.

Comment 9

[Jeff Boek [:boek]]

I think this is safe enough to uplift to 110. What is the process to uplift?

Comment 10

[Lorand Janos]

This issue is still reproducible with the latest Nightly 111.0a1 (2023-01-25) build.
Devices used:

• One Plus 6T (Android 9)
• Oppo find X5 (Android 12)

Comment 11

[Daniel Holbert [:dholbert]]

I can repro in latest Nightly 111.0a1 today, as well (2023-01-28), on a Google Pixel 6a phone.

I see the patch that landed here changed one line from...
showSearchSuggestions = true
...to...
showSearchSuggestions = action.settings.shouldShowSearchSuggestions
...though I notice two minor concerns that might explain why it didn't seem to fix the bug:
(1) this change was part of SearchDefaultEngineSelected which does not sound like the user-flow that's described in the STR here. The STR here aren't setting the default search engine, but are instead picking a different search engine "on the fly".
(2) this wasn't the only assignment to true that we had for this variable -- there's at least one a few lines further down.

This is kind of a shot in the dark, but maybe we really needed to apply a similar fix a few lines further down, in SearchShortcutEngineSelected? Depending on what "search shortcut engine selected" means, that sounds closer to what is described in the STR. And that snippet does currently hardcode showSearchSuggestions to true. Quote:

        is SearchFragmentAction.SearchShortcutEngineSelected ->
            state.copy(
                searchEngineSource = SearchEngineSource.Shortcut(action.engine),
                showSearchSuggestions = true,

https://github.com/mozilla-mobile/fenix/blob/72a0086ff8ebfff4ce832cb09743f221bbca1c4a/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt#L248

Comment 12

[Jeff Boek [:boek]]

This should be fixed with https://github.com/mozilla-mobile/fenix/pull/27810

tested 111.0a1 (build #2015932011)

Comment 13

[Ryan VanderMeulen [:RyanVM]]

That PR appears to have been merged a week ago. Lorand, can you confirm that it's working now?

Comment 14

[Daniel Holbert [:dholbert]]

I can confirm that it's working for me in current Nightly (build #2015931819), FWIW. (I was able to repro in comment 11, just over a week ago.)

I'll leave Lorand's needinfo open as additional verification, though.

Comment 15

[Lorand Janos]

The issue is no longer reproducible on the latest Nightly 111.0a1 (2023-02-09).
Tested with a clean build not to have suggestions from history.
Device used: Google Pixel 7 (Android 13).
Closing the ticket as verified.