1752110 - The "Google" string is briefly displayed in the placeholder text of the in-content search bar after the "New Tab" page is refreshed even if it is not the default search engine

Status: NEW

(Firefox :: New Tab Page, defect, P3)

Description

[Marius Coman [:mcoman], Ecosystem QA]

Attachment: rec of the issue.gif

[Affected versions]:

• Firefox Release 96.0.2 - Build ID: 20220119190439
• Firefox Beta 97.0b8 - Build ID: 20220125201015
• Firefox Nightly 98.0a1 - Build ID: 20220116095124

[Affected Platforms]:

• Windows 10 x64
• macOS 11.6.2
• Linux Mint 20.2 x64

[Prerequisites]:

• Have a Firefox build installed.
• Have Bing set as the default search engine.

[Steps to reproduce]:

1. Open Firefox with the profile from prerequisites.
2. Open a "New Tab" and refresh it.
3. Observe the placeholder text from the in-content search bar.

[Expected result]:

• The "Search with Bing or enter address" placeholder text is displayed.

[Actual result]:

• The "Google" string is briefly displayed instead of the "Bing" one.

[Notes]:

• This issue is reproducible with all the search engines.
• This issue is not reproducible if a new tab is opened instead of refreshing the page.
• I have tried to find a regression window, but it seems that this issue is not reproducible with Mozregression builds.
• Attached a screen recording of the issue.

Comment 1

[:Gijs (he/him)]

This seems to be because the code in Search.jsx uses this.props.Prefs.values["urlbar.placeholderName"] which never seems to actually come from the pref (!?), and relies on ContentSearchHandoffUIController to get the "real" information per the comment here.

Harry wrote this but is no longer here - Mark, do you know how this is "supposed" to work?

Comment 2

[Mark Banner (:standard8)]

(In reply to :Gijs (he/him) from comment #1)

Harry wrote this but is no longer here - Mark, do you know how this is "supposed" to work?

I think the problems here come from the history of the code. The in-content search UI was originally a box on the new tab page, and we didn't have any of the activity stream items. When the search box on the private window was implemented, the contentSearchHandoffUI was added.

When activity stream was implemented, I guess they skipped re-implementing the in-content search UI, and shoehorned it in by simply including the existing scripts. This means we have the activity stream parts and the in-content search parts working awkwardly together.

My guess around the existing UI is that we wanted some initial strings for the react parts to render when the box was put up, but that contentSearchHandoffUI "takes over" once started.

I should note at this stage, this bug only happens when you change the default search engine, until the next restart. It seems that the urlbar.placeholderName preference is loaded the first time but not updated later. It "works" because of the contentSearchHandoffUI take-over.

I think the reason the preference doesn't work, is that by default the Prefs object only operates on the browser.newtabpage.activity-stream. preferences.

A confusing part there is that PrefsFeed is using the full preference name to get the preference, but then only saving the mapping for the reduced name without the first part of the branch.

However, even with that preference somehow fixed, we're still going to have the content search UI attempting to doing subsequent updates, which is not ideal. Unfortunately this UI is used in the non-activity stream based about:privatebrowsing, as well as here. Also, the activity stream version currently still needs to support the non-handoff version as well as the handoff version, this makes it more complicated than doing an activity stream based implementation.

Anyway, the main issue appears to be the preference updating. I'm not sure if we could simply take out setting of the strings from Search.jsx - I guess we could at the expense of displaying the string slightly later? The other alternative would be to fix the preference issue, and leave the architectural performance issue to later.