383252 - Cannot drag / drop URL or link onto tabbar

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Andy Boze]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a6pre) Gecko/20070604 SeaMonkey/2.0a1pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a6pre) Gecko/20070604 SeaMonkey/2.0a1pre

I can not highlight a URL and drop it onto the tab bar, nor can I drag a link and drop it onto the tab bar. 

Reproducible: Always

Steps to Reproduce:
1a.Highlight a URL in a Web page and drag it to the tab bar, OR
1b.drag a link to the tab bar.
2.Drop it onto an empty space on the tab bar or onto an existing tab.
3.
Actual Results:  
Nothing happens.

Expected Results:  
If the URL or link is dropped onto an empty spot on the tab bar, a new tab should open. If the URL or link is dropped onto an existing tab, the new page should load in the tab.

Comment 1

[Sven Grull]

It seems that drag & drop generally is broken. Even dropping a html-file into browser window does not open the document. Also drag & drop of extensions into browser window has no effect.

Comment 2

[Sven Grull]

When dropping a file to the browser window error console says:

Error: nsDragAndDrop.dragDropSecurityCheck is not a function
Source File: chrome://communicator/content/contentAreaDD.js
Line: 66

Comment 3

[Andrew Schultz]

This is a suiterunner regression.

Comment 4

[Caio Tiago Oliveira (asrail)]

And:

JavaScript error: chrome://navigator/content/tabbrowser.xml, line 1574: nsDragAndDrop.dragDropSecurityCheck is not a function

Comment 5

[Sven Grull]

I just checked nsDragAndDrop.js in toolkit.jar included in SM1.5 and SM2:
SM1.5 nsDragAndDrop.js has dragDropSecurityCheck included but SM2 not. Adding the function to nsDragAndDrop.js in SM2 toolkit.jar restores the drag&drop functionality. 
Firefox includes the SM2 nsDragAndDrop.js 
So it seems that we have to use our own (old) nsDragAndDrop.js or place dragDropSecurityCheck where Firefox includes it (in tabbrowser.xml ?).

Comment 7

[jag (Peter Annema)]

Attachment: Move dragDropSecurityCheck from tabbrowser to nsDragAndDrop

Per bug 285438 comment 33 I'm moving this to what I think is a better place for this function, and it'll fix this bug without suite having to duplicate the function in its tabbrowser. While I'm not dead set on it living in nsDragAndDrop, I would like it to be in a place where suite and FF can both use it.

BTW, I took this from suite's old nsDragAndDrop, but a diff against the method in tabbrowser shows the body is the same except for white-space differences and different line-breaking in the comment.

Comment 8

[Sven Grull]

(In reply to comment #7)
> BTW, I took this from suite's old nsDragAndDrop, but a diff against the method
> in tabbrowser shows the body is the same except for white-space differences and
> different line-breaking in the comment.

As I compared the both versions last time I found another difference in line 397:
SeaMonkey version:  var sel= tree.view.selection;
Toolkit version:    var sel= obo.view.selection;

I don't know whether you have to take care of this.

Comment 9

[jag (Peter Annema)]

Sven: nah. |tree.view| is a shorthand property for |tree.treeBoxObject.view|, and |obo| is |tree.treeBoxObject|, so it's all the same.

Comment 10

[Mike Connor [:mconnor]]

Comment on attachment 273250 [details] [diff] [review]
Move dragDropSecurityCheck from tabbrowser to nsDragAndDrop

r=me, though I really wish we were using aURL instead of aURI for strings, makes a lot of things easier to parse

Comment 11

[jag (Peter Annema)]

Attachment: Same patch, but s/aUri/aDraggedText/g

mconnor, how about this? It replaces aUri with aDraggedText, gets rid of urlStr (just re-use the aDraggedText var), and does some early returns to get the text less indented.

Also, your r= suffices, right? Or should I get sr on this?

Comment 12

[Philip Chee]

> diff -u -r1.233 tabbrowser.xml
...
> -            }
> +            nsDragAndDrop.dragDropSecurityCheck(aEvent, aDragSession, aUri);

If you have changed all the callers, why is this method still needed in tabbrowser.xml?

Comment 13

[jag (Peter Annema)]

Philip: I put in the forwarder in case extensions are using it (unlikely? Google didn't turn anything up), but I'm more than happy to remove it.

mconnor, your take?

Comment 14

[Philip Chee]

Google codesearch shows only one hit outside the mozilla.org cvs:
<http://www.google.com/codesearch?q=dragDropSecurityCheck&hl=en&btnG=Search+Code>
And this is piro's own reimplementation in an essentially obsolete and orphaned extension.

Unfortunately neither mozdev nor AMO has searchable MXR/LXR.

Comment 15

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

You can file a bug in addons.mozilla.org::Addons to request a grep of all the extensions on AMO that use of the function (see e.g. bug 388393).

Comment 16

[Mike Connor [:mconnor]]

Comment on attachment 273715 [details] [diff] [review]
Same patch, but s/aUri/aDraggedText/g

I like this much much better, cleaner to read than the old impl, thanks!

Comment 17

[jag (Peter Annema)]

Checked in. I've filed bug 389624 per comment 15, and I'll deal with removing that method from tabbrowser once I get an answer there.

Comment 18

[jag (Peter Annema)]

Oops. I forgot I can't check into browser/. I'll get that checked in soon (thank $DEITY I kept the forwarding method in tabbrowser).

Comment 19

[jag (Peter Annema)]

Ignore the previous comment. I was looking at a bonsai query that didn't include browser/ :-)

Comment 20

[jag (Peter Annema)]

Philip: according to bug 389624 about 4 extensions are using this method on tabbrowser. I'll leave it in for now.