The keyword.URL reset prompt implemented in bug 718088 offers to re-set the search engine to the build's default search engine. However if the build's default engine has been modified, we don't undo that change when the user opts in to the re-set. That means that there are cases where we offer to reset to Google, but when you click the button, you don't actually get Google. We can fix this by also resetting the defaultenginename pref.

also added a basic test for the prompting functionality

Comment on attachment 607006 [details] [diff] [review] patch Review of attachment 607006 [details] [diff] [review]: ----------------------------------------------------------------- Callbacks and executeSoons abound, but it looks correct to me. :)

The test in this checkin actually launches a real search via https://www.google.com/othersearchfoo+bar I'm pretty sure you don't want to be doing that out of mochitest-other. otoh I think it may have just found a bug for me in an uncommited patch set. Thanks :)!

(In reply to Patrick McManus [:mcmanus] from comment #5) > The test in this checkin actually launches a real search via > > https://www.google.com/othersearchfoo+bar > (it generally somehow gets canceled btw (page nav'd away maybe?) - which is part of the fun of my bug... but that's going to be a timing thing, it could certainly go out to the network)

(In reply to Patrick McManus [:mcmanus] from comment #5) > The test in this checkin actually launches a real search via > > https://www.google.com/othersearchfoo+bar > > I'm pretty sure you don't want to be doing that out of mochitest-other. Why is it a problem, out of curiosity? I suppose there is a potential for side-effects in other tests, but this test doesn't depend on the load succeeding in any way. (I'll add a workaround to avoid triggering the load - I just didn't bother because I didn't think it would be a problem.)

(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #7) > Why is it a problem, out of curiosity? I think the risk is that you could jam up some queues (e.g. the dns queue) if there was no WAN for the test and that would interfere with resolving local names like example.org .. but mostly I thought the policy was no WAN activity in the testbase, it really doesn't matter to me though.. mostly I was surprised to see it so noted it here. if you're fine with it then I probably should just adjust my expectation. been there before :)

No, you're right that it's best to avoid in general. I just thought I might be able to get away with it in this case - the kind of dangerous attitude that leads to random orange! :)

FWIW, I backed the test out in bug 738804.