Created attachment 434288 [details] [diff] [review] patch
Can we add a test for search bar dropping, too?
Created attachment 434860 [details] [diff] [review] version 2 That previous patch was completely wrong. Here is the real version.
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.
Attachment #434860 - Flags: review?(gavin.sharp) → review-
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.
Depends on: 539476
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).
Attachment #434860 - Flags: review- → review+
Created attachment 434949 [details] [diff] [review] version 3 Adjust test to check if the search starts a load. Uses an existing test which already creates some new tabs.
Attachment #434860 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
i suppose we can dupe Bug 545122 to this one(?)...
You need to log in before you can comment on or make changes to this bug.