1690114 - Updating the Search Engine Name doesn't work from an add-on

Status: VERIFIED FIXED

(Firefox :: Search, defect, P2)

Description

[Richie]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/88.0.4324.96 Safari/537.36

Steps to reproduce:

Step 1: Have a searchprovider with a name: WebSearch1
Step2: Update the name of searchProvider to WebSearch2

Actual results:

The extension fails and after restart defaults to google search

Expected results:

Should have worked with name as WebSearch 2

After debugging the ff code found that in SearchServices.jsm the code searches for the searcheengine based on the key and any update on engine name does not change the key and only the _name inside values

Comment 1

[Daniel Bodea [:danibodea]]

I would like to confirm your issue and move it forward, but I am confused of the steps of reproduction.
Would you update the steps to reproduce to a much more detailed version? Thanks.

Please include information:

1. Do you set the search provider from the browser's Preferences, Search section, Default Search Engine?
2. How exactly do you update the name of the searchProvider?
3. In the "Actual results" section you mentioned about an extension. Which extension are you talking about?
4. How did you "restart defaults to google search"?
   (Keep in mind that exact steps to reproduce might help me a lot in moving it forward fast)

Sorry about the specific questions, but it's better to avoid the "back-and-forth" commenting and request more rather than less.
Thank you for your contribution, Richie.

Comment 2

[Richie]

Steps To reproduce the issue:

Step 1: Create a addon with search provide override in the manifest file
"chrome_settings_overrides": {
"search_provider": {
"name": "WebSearch1",
"keyword": "Easy Games Tab",
"favicon_url": "",
"suggest_url": "search.com", (example)
"is_default": true,
"encoding": "UTF-8"
}
},
Step 2: Load it as a temporary addon or pack it into XPI and load it
Step 3: Change the search provider name to WebSearch2 from WebSearch1.

Now if you search on the urlbar the search fails and if you restart the browser the search engine defaults to google from search.com

Comment 3

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

The Bugbug bot thinks this bug should belong to the 'WebExtensions::Untriaged' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 4

[Alex Cornestean]

Hello,

I’ve also attempted reproducing the issue on the latest Nightly (87.0a1/20210209214921) and Release (85.0.2/20210208133944) on Windows 10 x64, but without success.

I’ve downloaded a search engine add-on and changed the search provider name to WebSearch1, in the add-on’s manifest.json file, as in the STR you provided.

Afterwards I’ve loaded the add-on in about:debugging, as a temporary extension and checked if the default search engine was set to the one provided by the add-on, WebSearch1 in this case. And indeed in about:preferences#search, the default search engine was set to WebSearch1. I’ve also performed a search by typing something in the URL bar and the search returned results as expected.

After performing the search test, I’ve changed the name of the search provider to WebSearch2 and used the “Reload” button in order for the changes to be reflected in Firefox as well. Also reloading the about:preferences#search page indicated that the name change was successful as the default search engine was set to WebSearch2. Performed another search by typing something in the URL bar and the search returned results as expected once more.

If these are the actions you have also performed when you uncovered the issue, please let me know or provide more detailed steps to reproduce if otherwise.

Regarding the default search engine reverting to Google after restarting, this is most certainly due to the fact that if you restart the browser while a temporary add-on is loaded (provided you did indeed do that), it will be gone after the browser re-launches (as this is how that feature is designed, to only keep temporary add-ons until browser restart) and Firefox will re-set Google as a search engine automatically.

Thank you !

Comment 5

[Richie]

Attachment: ffissue.mov

have attached the video of what i was changing and the error when it appears

Comment 6

[Alex Cornestean]

Hello Richie and thank you for the video !

I’ve managed to reproduce the issue on the latest Nightly (87.0a1/20210216215129), Beta (86.0/20210215141125) and Release (85.0.2/20210208133944) on Windows 10 x64 and Ubuntu 16.04 LTS.

After changing the search provider name, the add-on fails i.e it does not perform the search. Note that I’ve used and modified the Yahoo Search Addon (https://addons.mozilla.org/en-US/firefox/addon/yahoo-search-addon/) to reproduce the issue.
On Release and Beta, the browser console displays multiple “engine is null” error when the issue occurs. However, on Nightly, the console is blank.

I’ve also observed that after the add-on fails, if I type something else in the URL bar ( thus changing the original search terms) the add-on works. This is done in the same tab as the search that failed.

Comment 7

[Shane Caraveo (:mixedpuppy)]

What is the behavior the search service uses with name changes?

Comment 8

[Mark Banner (:standard8)]

We should be allowing name changes, we just don't allow the name to be the same as one of the application provided engines.

There's bug 369739 already existing on this subject, though I think in theory we could probably fix the WebExtension only case so I've set that as a see-also for now. It might be worth just fixing both at the same time.

I'm moving this across to the search service, as that's where we need to fix this. I'll see if I can take a look sometime in the next week or two.

Comment 9

[Mark Banner (:standard8)]

Attachment: Bug 1690114 - Fix handling of updating the search engine name when a WebExtension updates. r?daleharvey!

Comment 10

[Mark Banner (:standard8)]

Attachment: Bug 1690114 - When a search engine changes properly update the default and the tree in preferences. r?#preferences-reviewers!

Prior to this patch we would only update the icons.

Depends on D107668

Comment 11

[Mark Banner (:standard8)]

Try push: https://treeherder.mozilla.org/jobs?repo=try&revision=c43dbe5df5666c2e94906581e7ea0a2e71940bc5

Comment 12

[Mark Banner (:standard8)]

Attachment: xpi_v1.xpi

Comment 13

[Mark Banner (:standard8)]

Attachment: xpi_v2.xpi

Comment 14

[Mark Banner (:standard8)]

For QE Verify:

1. Open about:debugging, select "This Nightly" (or "This ..." depending on the version you're testing).
2. Install xpi_v1.xpi via "Load Temporary Add-on..."
3. Check the preferences UI, address bar & other access points for the search engine.
4. Install xpi_v2.xpi
5. Re-check the preferences UI, address bar etc.

Also check with it set as default, in step 2, and with it not as default.

We might want to also uplift this to the 78 esr series. I'm not quite sure yet, I need to go back and look at how different the code was.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a29f0284b2d
Fix handling of updating the search engine name when a WebExtension updates. r=daleharvey
https://hg.mozilla.org/integration/autoland/rev/4502b761e3f0
When a search engine changes properly update the default and the tree in preferences. r=preferences-reviewers,Gijs

Comment 16

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/5a29f0284b2d
https://hg.mozilla.org/mozilla-central/rev/4502b761e3f0

Comment 17

[Alex Cornestean]

Hello,

Verified the fix using the STR from Comment 14 on the latest Nightly (88.0a1/20210311093001) under Windows 10 x64. Checked with both setting xpi_v1 as default and not setting it as default scenarios.

As soon as xpi_v2 is loaded, the search engine name is updated in about:preferences and in the address bar, confirming the fix.

Comment 18

[Deepak]

Alex,

Question: What will be the expected default search engine be after the release goes out for users impacted by this bug?

E.g

1. User has searchprovider with a name: WebSearch1
2. Update the name of searchProvider to WebSearch2
3. The extension fails and after restart defaults to google search
4. After the issue is fixed for old users who have their search engine set at google search because of the issue will it get reverted to WebSearch2 as that should have been the case.

Thanks,
Deepak

Comment 19

[Ryan VanderMeulen [:RyanVM]]

(In reply to Mark Banner (:standard8) from comment #14)

We might want to also uplift this to the 78 esr series. I'm not quite sure yet, I need to go back and look at how different the code was.

Any thoughts on this?