Closed Bug 1297374 Opened 3 years ago Closed 3 years ago

[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

Categories

(Firefox :: Search, defect, P2)

51 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 52
Tracking Status
firefox49 --- unaffected
firefox50 --- unaffected
firefox51 --- verified
firefox52 --- verified

People

(Reporter: cirdeiliviu, Assigned: standard8, Mentored)

References

Details

(Whiteboard: [fxsearch])

Attachments

(3 files)

[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.
Priority: -- → P2
Whiteboard: [fxsearch]
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.
Mentor: florian
I have a patch in progress for this.
Assignee: nobody → 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 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.
Attachment #8802911 - Flags: review?(florian) → review+
Pushed by mbanner@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/b9ca193b71b3
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
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]
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.
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.
Duplicate of this bug: 1314643
Attached patch Patch for AuroraSplinter Review
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
Attachment #8807128 - Flags: approval-mozilla-aurora?
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.
Flags: needinfo?(andrei.vaida)
Attachment #8807128 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: qe-verify+
Flags: needinfo?(andrei.vaida) → needinfo?(liviu.cirdei)
Blocks: 1314643
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.
Flags: needinfo?(liviu.cirdei)
You need to log in before you can comment on or make changes to this bug.