1297374 - [One-off searches] When changing the default search engine from Awesomebar, the icon of the new default engine is replaced by the icon of the old default engine

Status: VERIFIED FIXED

(Firefox :: Search, defect, P2)

Description

[Liviu Cirdei]

Attachment: screenshot_of_the_issue.png

[Affected versions]:
Nightly 51 Build ID 	20160822064441
 
[Affected platforms]:
Ubuntu 15.04 x64, Windows 10 x64

[Steps to reproduce]:
1. Open Firefox with a new profile.
2. Type something in the Awesomebar.
3. Right-click on one of the one-off searches icons. (e.g. Wikipedia).
4. From context menu click "Set as default search engine". 
 
[Expected result]:
The icon representing the search engine which was set as default should remain the same (e.g. Wikipedia)

[Actual result]:
The icon representing the search engine that was set as default is replaced by the icon of the old default search engine (Google). So we end up with two icons of the old search engine (in this case "Google").

See the attached screenshot.

[Note]:
After another search the icons are ok.

Comment 1

[Florian Quèze [:florian]]

This behavior happens because in the searchbar the current default engine isn't shown as a one-off button. For the location bar we decided to display the current default engine in the list of one-off buttons, so this behavior should be disabled there. Should be just a matter of adding a test.

Comment 2

[Mark Banner (:standard8)]

I have a patch in progress for this.

Comment 3

[Mark Banner (:standard8)]

Attachment: Bug 1297374 - When setting default engine from the url bar, stop the new engine replacing the old icon as the url bar always shows all engines.

Also ensure the telemetry origin is set correctly when the searchbar popup first opens.

Review commit: https://reviewboard.mozilla.org/r/87170/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/87170/

Comment 4

[Mark Banner (:standard8)]

(In reply to Mark Banner (:standard8) from comment #3)
> Also ensure the telemetry origin is set correctly when the searchbar popup
> first opens.

Just to expand on this slightly: When I was writing the test, I discovered the telemetry origin wasn't set when the searchbar popup first opened in the test. As a result, I found that starting FF, opening the search & trying to change the default search engine, it wouldn't change the icons for the search bar (and it threw an error).

I couldn't see any other adverse side effects, though I couldn't be certain about the telemetry parts.

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 8802911 [details]
Bug 1297374 - When setting default engine from the url bar, stop the new engine replacing the old icon as the url bar always shows all engines.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/87170/diff/1-2/

Comment 6

[Florian Quèze [:florian]]

Comment on attachment 8802911 [details]
Bug 1297374 - When setting default engine from the url bar, stop the new engine replacing the old icon as the url bar always shows all engines.

https://reviewboard.mozilla.org/r/87170/#review87334

Thanks! r=me, assuming try is green.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b9ca193b71b3
When setting default engine from the url bar, stop the new engine replacing the old icon as the url bar always shows all engines. r=florian

Comment 8

[Phil Ringnalda (:philor)]

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

Comment 9

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 51.0a1 (2016-08-23) on Windows 7 , 64 Bit !

This bug's fix is verified with latest Nightly

Build ID    :  20161025030205
User Agent  :  Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:52.0) Gecko/20100101 Firefox/52.0

[bugday-20161026]

Comment 10

[Liviu Cirdei]

Thanks Maruf for verifying this.

This is also verified on latest Nightly (Build ID 20161026030210) on the following OS-es:
 *Mac 10.11 
 *Ubuntu 16.04 and
 *Windows 10.

Comment 11

[Mark Banner (:standard8)]

Although bug 1180944 is disabled by pref, when it changed the code to add the urlbar buttons, it did adversely affect the search bar buttons as this bug shows (regardless of the pref setting).

Hence this bug is still valid for 51. I'll get a patch up in a bit.

Comment 13

[Mark Banner (:standard8)]

Attachment: Patch for Aurora

The only change here is to set the preference to enable the one-off buttons in the urlbar for the duration of the test, so that the test passes.

Approval Request Comment
[Feature/regressing bug #]: Bug 1180944 - although it has been disabled by pref, the changes caused the regression in the existing UI. 
[User impact if declined]: This patch fixes this bug, as well as bug 1314643. The bug 1314643 is the one we want to fix, but its easiest to fix both. For bug 1314643 - the first time a search bar is opened after start up or after opening a new window, the user won't be able to right-click and set the default engine.
[Describe test coverage new/current, TreeHerder]: Landed in m-c has a unit test
[Risks and why]: Low, small change that makes sure the buttons are correctly initialised and is covered by tests.
[String/UUID change made/needed]: None

Comment 14

[Gerry Chang [:gchang]]

Comment on attachment 8807128 [details] [diff] [review]
Patch for Aurora

Fix an issue related to change default search engine in awesomebar. Take it in 51 aurora.

Hi Andrei,
Can you help find someone to verify this and bug 1314643? Thanks.

Comment 15

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f29e3dc3f53e

Comment 16

[Liviu Cirdei]

The fix of this bug is verified on Aurora 51 (Build ID 	20161105004017). Tested on:  
*Windows 10 
*Mac 10.11 
*Ubuntu 16.04. 

In Aurora the one-off searches from Awesomebar are hidden by the pref, so users can't interact with them. However for testing purpose I enabled the one-off searches and when changing the default search engine, there was no duplication of the engine icons, so it works as expected.

Also bug 1314643 was verified.