(In reply to Marco Bonardo [::mak] from bug 1303346 comment #34) > we enabled search suggestions by default. I suspect that's related. > You want to set browser.urlbar.userMadeSearchSuggestionsChoice to true and > browser.urlbar.suggest.searches to false. It needs an update for: * https://dxr.mozilla.org/mozilla-central/source/testing/marionette/server.js (recommended prefs) * https://dxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette_driver/geckoinstance.py (for Firefox) Raajit, are you interested to work on this bug?

Yes I would love to. Can I get more information? Its not clear whats going on here. Thanks

It's just that you would have to add those two preferences with their changed default values to the referenced places. More details can be found at bug 1303346 comment #34 if you are really interested in. Or better would be if Marco can give us references to those changes.

(In reply to Henrik Skupin (:whimboo) from comment #2) > Or better would > be if Marco can give us references to those changes. The bug implementing the new suggestions hint was bug 1344924, while bug 1344928 switched the default. But both the prefs existed before, userMadeSearchSuggestionsChoice just makes us not show the hint, while suggest.searches is the pref controlling whether search.suggestions are enabled in the urlbar.

Comment on attachment 8872435 [details] Bug 1368034 - Disable the search suggestion for urlbar in Marionette https://reviewboard.mozilla.org/r/143932/#review147776 Looks fine beside the one filed issue. Please have a look at it. Also we should improve the commit message, so it explains `what` in the summary and the `why` in the details. ::: testing/marionette/client/marionette_driver/geckoinstance.py:477 (Diff revision 1) > > # Disable the UI tour > "browser.uitour.enabled": False, > > + "browser.urlbar.userMadeSearchSuggestionsChoice": True, > + "browser.urlbar.suggest.searches": False, Please have a look at the bug Marco referenced. It should help you to understand what will be changed by modifying the prefs. We would need a short comment above the blocks in both files.

Comment on attachment 8872435 [details] Bug 1368034 - Disable the search suggestion for urlbar in Marionette https://reviewboard.mozilla.org/r/143932/#review147784 ::: testing/marionette/server.js:159 (Diff revision 1) > + ["browser.urlbar.userMadeSearchSuggestionsChoice", true], > + ["browser.urlbar.suggest.searches", false], Please document.

Comment on attachment 8872435 [details] Bug 1368034 - Disable the search suggestion for urlbar in Marionette https://reviewboard.mozilla.org/r/143932/#review147788 ::: testing/marionette/server.js:159 (Diff revision 1) > + ["browser.urlbar.userMadeSearchSuggestionsChoice", true], > + ["browser.urlbar.suggest.searches", false], These also need to be added to testing/geckodriver/src/prefs.rs.

Raajit, thank you for the update. But can you please make sure to no longer create new mozreview requests, but simply update the existing one? Thanks.

So I have just seen that the second patch only includes the changes for geckodriver. Please make sure to provide that as part of your original review request.

Comment on attachment 8874187 [details] Bug 1368034 - Update default values for urlbar searches in Marionette https://reviewboard.mozilla.org/r/145612/#review150264

Yeah I have no idea what went wrong there. I just updated the patch locally then pushed it and this happened. I was thinking to discard and submit a new patch but you told me not to do so before.

Comment on attachment 8872435 [details] Bug 1368034 - Disable the search suggestion for urlbar in Marionette https://reviewboard.mozilla.org/r/143932/#review150268 I'm going to r- the patch until we have a final working solution. It will make it easier to spot, when the patch is ready to land. ::: commit-message-34ac1:1 (Diff revisions 1 - 2) > -Bug 1368034 Update default values for urlbar searches in Marionette > +Bug 1368034 - Disable the search suggestion for urlbar in Marionette The commit message is still missing a more details description of why we have to do it. I referenced the bugs which changed the behavior of Firefox earlier in this bug. So please get an understanding of it, and add another paragraph. For an example see: https://hg.mozilla.org/mozilla-central/rev/ff3c813fcdea ::: testing/marionette/server.js (Diff revisions 1 - 2) > ["browser.uitour.enabled", false], > > + // Disable the URL bar search suggestions > ["browser.urlbar.userMadeSearchSuggestionsChoice", true], > ["browser.urlbar.suggest.searches", false], > - Please do not remove this empty line. ::: testing/geckodriver/src/prefs.rs:111 (Diff revision 2) > // Disable the UI tour > ("browser.uitour.enabled", Pref::new(false)), > > + // Disable the URL bar search suggestions > + ("browser.urlbar.userMadeSearchSuggestionsChoice", Pref::new(true)), > + "browser.urlbar.suggest.searches", Pref::new(false), There are missing braces.

Raajit, the mozreview is still messed-up. Can you please contact me on IRC first before pushing another update? It has to get fixed first before I can do another review.

Comment on attachment 8872435 [details] Bug 1368034 - Disable the search suggestion for urlbar in Marionette https://reviewboard.mozilla.org/r/143932/#review150944

Comment on attachment 8874187 [details] Bug 1368034 - Update default values for urlbar searches in Marionette https://reviewboard.mozilla.org/r/145612/#review150948

I dont think it worked. What should I do now?

Comment on attachment 8874187 [details] Bug 1368034 - Update default values for urlbar searches in Marionette https://reviewboard.mozilla.org/r/145612/#review151728 We didn't finish the discussion about your problem. So I didn't expect that you push once more an update to mozreview. Please lets get everything sorted out first. ::: testing/geckodriver/src/prefs.rs:111 (Diff revision 3) > // Disable the UI tour > ("browser.uitour.enabled", Pref::new(false)), > > // Disable the URL bar search suggestions > ("browser.urlbar.userMadeSearchSuggestionsChoice", Pref::new(true)), > - "browser.urlbar.suggest.searches", Pref::new(false), > + ("browser.urlbar.suggest.searches", Pref::new(false),) This line is broken cause the comma is on the wrong position.

Raajit, would you have the time so that we can finish off this bug? I would appreciate that.

(In reply to Henrik Skupin (:whimboo) from comment #22) > We didn't finish the discussion about your problem. So I didn't expect that > you push once more an update to mozreview. Please lets get everything sorted > out first. I will surely get this done once we rectify all the mistakes here.

Hi Henrik, Really sorry for the delay. Can you please help me finish this bug?

Raajit, I would propose that you get in contact to me on IRC and we walk through all again.

No response from Raajit over the last 2 month. Putting it back into the pool for someone else to finish it up.

Comment on attachment 8911721 [details] Bug 1368034 - Update default values for urlbar searches in Marionette https://reviewboard.mozilla.org/r/183128/#review188326 Code-wise it looks fine but comments and the commits need an update. ::: testing/geckodriver/src/prefs.rs:112 (Diff revision 1) > > // Do not warn on quitting Firefox > ("browser.warnOnQuit", Pref::new(false)), > > + //Disable search suggestion hints, but keep search suggestions enabled > + ("browser.urlbar.userMadeSearchSuggestionsChoice", Pref::new(true)), Please use the description from here: https://dxr.mozilla.org/mozilla-central/source/testing/profiles/prefs_general.js#361 ::: testing/geckodriver/src/prefs.rs:113 (Diff revision 1) > // Do not warn on quitting Firefox > ("browser.warnOnQuit", Pref::new(false)), > > + //Disable search suggestion hints, but keep search suggestions enabled > + ("browser.urlbar.userMadeSearchSuggestionsChoice", Pref::new(true)), > + ("browser.urlbar.suggest.searches", Pref::new(false)), Please use the description from here: https://dxr.mozilla.org/mozilla-central/source/testing/profiles/prefs_general.js#357 ::: testing/marionette/client/marionette_driver/geckoinstance.py:478 (Diff revision 1) > "browser.tabs.warnOnOpen": False, > > # Disable the UI tour > "browser.uitour.enabled": False, > > + # Disable search suggestion hints, but keep search suggestions enabled Please update with the above mentioned descriptions. ::: testing/marionette/server.js:165 (Diff revision 1) > // Disable the UI tour. > // > // Should be set in profile. > ["browser.uitour.enabled", false], > > + // Disable search suggestion hints, but keep search suggestions enabled Please update with the above mentioned descriptions.

Comment on attachment 8911721 [details] Bug 1368034 - Update default values for urlbar searches in Marionette https://reviewboard.mozilla.org/r/183128/#review188334 ::: commit-message-7e962:3 (Diff revisions 1 - 2) > Bug 1368034 - Update default values for urlbar searches in Marionette r=whimboo > > -Search suggestions and an onboarding notification bar were added to Firefox. This patch updates the default settings in Marionette to enable the search suggestions but disable the onboarding notifications. > +Search suggestions create unneccessary network requests and the suggestions opt-in notification interferes with tests that don't expect it to be there. So, this patch updates the default settings in Marionette to disable both. Please limit the line length to 78 chars by doing manual wrapping. ::: testing/geckodriver/src/prefs.rs:117 (Diff revisions 1 - 2) > + // tests that don't expect it to be there. > ("browser.urlbar.userMadeSearchSuggestionsChoice", Pref::new(true)), > + > + // Turn off search suggestions in the location bar so as not to trigger > + // network connections. > ("browser.urlbar.suggest.searches", Pref::new(false)), nit: put this pref before the other one to keep alphabetical order. Please do that in all cases. Also I don't see that the number of list items has been increased. Geckodriver should still not build.

Comment on attachment 8911721 [details] Bug 1368034 - Update default values for urlbar searches in Marionette https://reviewboard.mozilla.org/r/183128/#review188702 The latest changes look fine to me. I pushed a try build to ensure it works all fine.

Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/72ccf148b3e6 Update default values for urlbar searches in Marionette r=whimboo