554400 - Convert search.xml to use api from bug 545714

Status: RESOLVED FIXED

(Firefox :: Search, defect)

Description

[Neil Deakin]

Attachment: patch

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Can we add a test for search bar dropping, too?

Comment 2

[Neil Deakin]

Attachment: version 2

That previous patch was completely wrong. Here is the real version.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 434860 [details] [diff] [review]
version 2

This patch applied on top of attachment 434292 [details] [diff] [review] seems to break drag-to-search (the text is dropped but no search is triggered). search.xml's drop handler just isn't called.

The test should probably cover that... I guess that's a bit of a pain since you'd need a search engine that doesn't hit the network, but you can do that relatively easily the same way e.g. browser_415700.js does. You should probably also do the test in a new tab to avoid interfering with other tests.

Comment 4

[Neil Deakin]

Looks like there is also a dependency on the patch in bug 539476 as well. Otherwise the default editor listener will respond instead. This is likely why the original code used a capturing listener. The patch in bug 539476 changes the editor listeners to respond during the system group phase so that they act more like default handlers.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 434860 [details] [diff] [review]
version 2

OK, r=me with the test changes (doing the test in a new tab at the very least, since as it is now the test causes a load of Google and doesn't clean it up).

Comment 6

[Neil Deakin]

Attachment: version 3

Adjust test to check if the search starts a load. Uses an existing test which already creates some new tabs.

Comment 7

[Neil Deakin]

http://hg.mozilla.org/mozilla-central/rev/d9c5998f3e41

Comment 8

[Marco Bonardo [:mak]]

i suppose we can dupe Bug 545122 to this one(?)...