72801 - Wrong URL dragged from browser window on 2nd drag when links are close together

Status: RESOLVED FIXED

(Core :: DOM: Copy & Paste and Drag & Drop, defect)

Description

[Gervase Markham [:gerv]]

See the test case. When a URL which is not the first <A> is selected and dragged 
(to, say, the URL bar) the wrong URL is pasted - that of the first <A> tag.

The problem is on line 60ish of contentAreaDD.js - 

          if (anchors.length > 0) {
            draggedNode = anchors[0];
            urlstring = draggedNode.href;
          }

I'm not sure why it's doing that, and how it's supposed to get the correct A 
node, so I haven't fixed it. Later in the comments, it says:

            // how are we going to get the URL, if any? Scan the selection
            // for the first anchor? See bug #58315

But bug 58315 is not relevant to this problem. Alecf wrote that, so CCing him.

The best way to see this on the test case is to drag one of the links (not the 
top one) to the URL bar twice in succession. First time, the correct link is 
pasted. Second time, the wrong one.

Gerv

Comment 1

[Gervase Markham [:gerv]]

Attachment: testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

Gerv, that testcase worksforme with linux build 2001-03-20-21.

As for the code you cite, look up just a tad:

var anchors = domselection.anchorNode.getElementsByTagName("a");

So "anchors" is the array of all <a> elements in the selected text (if there is
selected text.  If there is not, we're not even in that branch of the 
"if (domselection && !domselection.isCollapsed &&
domselection.containsNode(draggedNode,false))").  That makes sense -- if you d&d
text with a bunch of anchors in it, we sort of arbitrarily pick the first one as
the thing to drop.

Comment 3

[Gervase Markham [:gerv]]

I don't know why we are executing that code in the situation I describe - but we
are, because I added dump() statements to prove it. I agree we don't have this
problem on Linux - it seems drag and drop works differently there. Can you
actually drag the URL to the URL bar at all? I can't using KDE - I just get a
context menu. Maybe I'm using the wrong button. <shrug>

Anyway, it's a problem on Windows. I promise. :-)

Gerv

Comment 4

[Alec Flett]

ok, that is odd.. I think that the line which selects the first anchor is
correct, but it's actually the line above it:
          var anchors = domselection.anchorNode.getElementsByTagName("a");

that is broken - the anchor node is supposed to be the topmost node in the
selection, but maybe I need to check the index of the anchor node in the
selection as well.

Comment 5

[Alec Flett]

by the way, the key to reproducing this is to drag a link a 2nd time - i.e.
while it's still selected... and this only seems to happen in certain
circumstances, probably when the links are all at the same level... I think now
what I need to do is somehow find the first anchor which is contained in the
selection - I think the selection API will do that for me.

Comment 6

[Blake Ross]

Attachment: patch

Comment 7

[Blake Ross]

The problem here was with

aXferData.data.addDataForFlavour("text/unicode", isAnchor ? urlstring :  
titlestring);

We weren't properly setting isAnchor in this case, so even though urlstring was 
correct, it was using titlestring.  This patch just sets it correctly (but 
ensures first that the anchor we find is still in the selection).

Alec, sr?

Comment 8

[Alec Flett]

cool, good catch! 
sr=alecf

Comment 9

[Jason Kersey]

r=kerz

Comment 10

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 11

[Blake Ross]

Fix checked in.