|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
95.48 KB, image/png
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.
58 bytes, text/x-review-board-request
|Details | Review|
11.85 KB, patch
|Details | Diff | Splinter Review|
Created attachment 8783947 [details] 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.
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.
I have a patch in progress for this.
(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.
Pushed by email@example.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
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.
Created attachment 8807128 [details] [diff] [review] 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 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.
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.