Not possible to recognize which search engine will be used before performing drop text to Searchbar I.e., transmits the data to the unexpected site. The browser should be explicit search engine to be used.

Philipp, I don't think we have ever discussed this case; do you see anything obvious we can do here?

As far as I understand, this is polishing. We won't block the release for this.

In addition to dropping text this also happens when right-clicking in the search field and selecting "paste" / "paste & search" and when jumping into the search field with ctrl+k and ctrl+v. This ultimately stems from the removal of the search engine name from the search box. This is something that should be reconsidered.

The behavior that the search bar triggers a search automatically seems odd in general. The awesomebar just shows the text as if the user has pasted it manually. Seems like the search box should do the same.

Rephrasing the summary to match my understanding of comment 4.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=57f8515822f1 Also asking for ui-review for confirmation, because the behavior we are changing has been in place since probably Firefox 1.0 (I couldn't find the exact check-in, but it's from the aviary branch).

Comment on attachment 8554541 [details] [diff] [review] Patch Review of attachment 8554541 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/search/test/browser_426329.js @@ -226,5 @@ > "Shift+MiddleClick loaded results in background tab"); > is(event.originalTarget.URL, expectedURL(searchBar.value), "testShiftMiddleClick opened correct search page"); > }); > > -add_task(function testDropText() { Can we still test that the popup opened here?

(In reply to Dave Townsend [:mossop] from comment #7) > ::: browser/components/search/test/browser_426329.js > @@ -226,5 @@ > > "Shift+MiddleClick loaded results in background tab"); > > is(event.originalTarget.URL, expectedURL(searchBar.value), "testShiftMiddleClick opened correct search page"); > > }); > > > > -add_task(function testDropText() { > > Can we still test that the popup opened here? How would this be different from the test I added in browser_searchbar_openpopup.js?

Comment on attachment 8554541 [details] [diff] [review] Patch Review of attachment 8554541 [details] [diff] [review]: ----------------------------------------------------------------- Never mind, missed the added test. r+ assuming this gets ui+

Comment on attachment 8554541 [details] [diff] [review] Patch Looks good!

(In reply to Florian Quèze [:florian] [:flo] from comment #6) > Created attachment 8554541 [details] [diff] [review] > Patch > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=57f8515822f1 This is consistently failing on Windows XP debug (but not on Windows XP opt, or Windows 7 debug). I don't see any obvious way to debug this. Is there an easy way to disable the test only for XP debug (but keep it running on 7 debug)?

(In reply to Florian Quèze [:florian] [:flo] from comment #11) > (In reply to Florian Quèze [:florian] [:flo] from comment #6) > > Created attachment 8554541 [details] [diff] [review] > > Patch > > > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=57f8515822f1 > > This is consistently failing on Windows XP debug (but not on Windows XP opt, > or Windows 7 debug). I don't see any obvious way to debug this. Is there an > easy way to disable the test only for XP debug (but keep it running on 7 > debug)? I am reliably informed that this should work: skip-if = (os == "win" && os_version == "5.1" && debug)

sorry had to back this out for merge conflicts down from m-c in browser_searchbar_openpopup.js

Comment on attachment 8554541 [details] [diff] [review] Patch Approval Request Comment [Feature/regressing bug #]: bug 1088660. [User impact if declined]: Dropping text on the searchbox starts a search without the user having an opportunity to see which search engine will be used. [Describe test coverage new/current, TreeHerder]: the patch contains a test. [Risks and why]: Not much technical risk; the code change is a one liner and has tests, but some users may be upset by this new behavior, so I don't think we should uplift it to beta; better have a whole 6 weeks for people to file bugs if there are edge cases where we should polish the behavior. [String/UUID change made/needed]: none

Comment on attachment 8554541 [details] [diff] [review] Patch I agree that we should be cautious about changing behaviour that some users may have come to rely upon. Let's take this simple change on Aurora and get a full Beta cycle to test as well. Aurora+

Backed out from Aurora for test failures. https://hg.mozilla.org/releases/mozilla-aurora/rev/a57cce6af02c https://treeherder.mozilla.org/logviewer.html#?job_id=569134&repo=mozilla-aurora

The search panel is opened when dropping text in it - note that this only happens in non e10s windows (due to bug 936092). Tested with Nightly 38.0a1 2015-02-08 under Ubuntu 12.04 LTS 32-bit, Windows 7 64-bit and Mac OS X 10.9.5.

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #21) > Backed out from Aurora for test failures. > https://hg.mozilla.org/releases/mozilla-aurora/rev/a57cce6af02c > > https://treeherder.mozilla.org/logviewer.html#?job_id=569134&repo=mozilla- > aurora This is failing only on XP debug. This test is disabled on central for (os == "win" && os_version == "5.1" && debug) (see comment 12). I think this code confusing because of the backout that happened when merging fx-team and mozilla-central. I landed in comment 14 with the test disabled, Tomcat backed it out in comment 16 (without reenabling the test), and then relanded it in comment 17 (which I guess is what was uplifted).

Good call, thanks for looking into it. https://hg.mozilla.org/releases/mozilla-aurora/rev/511fb27a5333

Verified as fixed using Firefox 37 beta 1 under Ubuntu 12.04 LTS 32-bit, Windows 7 64-bit and Mac OS X 10.9.5.