321279 - Drag+drop of link to location bar broken, anchor node text gets included

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

[Jonathan Stanley]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051222 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051222 Firefox/1.6a1

The test case has the following link:

<a href="http://www.mozilla.org/">Mozilla Organisation</a>

... and when the link is dragged to the location bar, Firefox attempts to forward itself to:

http://www.mozilla.org/Mozilla Organisation

Reproducible: Always

Steps to Reproduce:
1. Load test case
2. Drag link to location bar
Actual Results:  
Page load fails as the link Firefox attempts to load is invalid. For anchor in the format:

<a href="http://foo.tld/bar">NODETEXT</a>

... the drag+drop to location bar will make Firefox attemmpt to load:

http://foo.tld/barNODETEXT

Expected Results:  
Firefox should just use the string of the "href" value, namely:

http://foo.tld/bar

Drag+drop to an empty spot on the tabs bar works as expected, curiously...

Comment 1

[Adam Guthrie]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20051222 Firefox/1.6a1

Works for me. I drag the link into the location bar and the URL http://www.mozilla.org/ is loaded.

Comment 2

[Jesse Ruderman]

WFM Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20051221 Firefox/1.6a1

If this is a regression, it's probably a bug revealed by the fix for bug 23485.

Comment 3

[Jonathan Stanley]

(In reply to comment #2)
> WFM Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1)
> Gecko/20051221 Firefox/1.6a1
> 
> If this is a regression, it's probably a bug revealed by the fix for bug 23485.
> 

It broke quite recently, so a regression from the fixing of that bug seems correct.

Comment 4

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20051220 Firefox/1.6a1 ID:2005122023

confirmed

regressed between
works 20051220 1429 pdt
fails 20051220 2329 pdt

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&filetype=match&whotype=match&sortby=Date&hours=2&date=explicit&mindate=20051220+1400&maxdate=20051220+2330&cvsroot=%2Fcvsroot
looks like bug 23485

Comment 5

[[e-mail]]

For some reason, Windows drag-and-drop of a link has always dragged two lines of text, the URL and the text, as you can see by dragging a link to a textarea. Linux and Mac only drag the URL.

Comment 6

[(not currently active) Ted Mielczarek]

I presume this and bug 324184 are both because on Windows, drag and drop to explorer requires the two lines in order to get an internet shortcut.  I'm hoping there's a data flavor available that has just the URL.  I'll look into this.

Comment 7

[Ria Klaassen (not reading all bugmail)]

*** Bug 327339 has been marked as a duplicate of this bug. ***

Comment 8

[(not currently active) Ted Mielczarek]

*** Bug 324184 has been marked as a duplicate of this bug. ***

Comment 9

[(not currently active) Ted Mielczarek]

So basically,
http://lxr.mozilla.org/mozilla/source/widget/src/windows/nsClipboard.cpp#121
is the culprit.  When you drag a link, a whole bunch of data flavors are added:
http://lxr.mozilla.org/mozilla/source/content/base/src/nsContentAreaDragDrop.cpp#1367
Unfortunately, the first link says that kURLMime gets advertised as CF_UNICODETEXT to the OS, so we lose the "just the URL" data flavor to the one with URL\nlinktext.  This particular code was added in bug 97413, I haven't read enough there to figure it all out yet.  Interesting that CF_TEXT contains just the URL.

Essentially this has been happening since 2003, but since the URLbar always chopped off strings at the first carriage return, nobody ever noticed.

Also, I wrote a small tool in C# to help me debug DnD:
http://ted.mielczarek.org/code/DNDebug.exe

I need to do more research on what the right course of action is, but that's the underlying cause.

Comment 10

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

*** Bug 347194 has been marked as a duplicate of this bug. ***

Comment 11

[(not currently active) Ted Mielczarek]

Attachment: don't step on CF_UNICODETEXT with url flavours

This works for me.  I don't know why those lines got added, but they make no sense.  The various URL flavours get advertised as CF_UNICODETEXT, which overrides the real unicode text flavour.  Removing this makes them all get their own clipboard format, and leaves unicode text alone, thus getting only the URL in a drag-drop into a textbox.

Comment 12

[(not currently active) Ted Mielczarek]

For reference, the offending lines began life with pinkerton's checkin:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsClipboard.cpp&branch=&root=/cvsroot&subdir=mozilla/widget/src/windows&command=DIFF_FRAMESET&rev1=1.41&rev2=1.42

"First stab at creating an internet shortcut when dragging a link to desktop."

Internet Shortcuts are now handled by the code from bug 250392, so I think these lines are bogus.

Comment 13

[(not currently active) Ted Mielczarek]

Comment on attachment 239750 [details] [diff] [review]
don't step on CF_UNICODETEXT with url flavours

Neil, care to take a look?

Comment 14

[[e-mail]]

Comment on attachment 239750 [details] [diff] [review]
don't step on CF_UNICODETEXT with url flavours

Seeing as we can't explain why pink put those lines there, and things work at least as well without them, I'll give you my r=me.

Comment 15

[(not currently active) Ted Mielczarek]

Checked in by timeless.

Comment 16

[(not currently active) Ted Mielczarek]

Verified on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060925 Minefield/3.0a1

Comment 17

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #16)
> Verified on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1)
> Gecko/20060925 Minefield/3.0a1

Just FYI, it's bad form to verify your own fix. 

Comment 18

[(not currently active) Ted Mielczarek]

We may want to consider this for the 1.8 branch, due to bug 372271.  I'll try to put together a branch patch soon.

Comment 19

[:Gijs (he/him)]

Attachment: Branch Patch

Branch Patch. Does the same, really.

Comment 20

[:Gijs (he/him)]

Comment on attachment 265776 [details] [diff] [review]
Branch Patch

Oops. Sorry for bugspam.

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 265776 [details] [diff] [review]
Branch Patch

sr+, but IMHO this patch does not meet branch criteria.

Comment 22

[:Gijs (he/him)]

(In reply to comment #21)
> (From update of attachment 265776 [details] [diff] [review])
> sr+, but IMHO this patch does not meet branch criteria.
> 

Thanks for the superreview!

Now, the dep of this bug (bug 372271) means that any branch app interacting with a trunk app will cause trouble when dragging links. That is, dragging a link from any gecko 1.8 Branch app (Thunderbird 2, XULRunner-based apps such as the Wikipedia cd, Joost and others) to a trunk/Gecko 1.9 app will break (and does already break). Getting this patch on branch will solve this problem, assuming it would also make it into a Thunderbird release.

I don't care as much about this patch getting on branch as I care about having the problem described above fixed. If you see a better, alternative way to do that, then I would love to hear about it.

Comment 23

[Daniel Veditz [:dveditz]]

Comment on attachment 265776 [details] [diff] [review]
Branch Patch

We don't yet know if we're releasing a Thunderbird with 2.0.0.5, and if we don't fix this in both at the same time won't we have the same problem? Fixing bug 372271 is not a compelling reason if the trunk is still in alpha (much as I find this bug annoying personally).

I think we're going with Roc's assessment and denying this one on the branch for now. As we get into trunk betas we can evaluate the real-world impact and reconsider.