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

VERIFIED FIXED

Status

()

Core
Widget: Win32
VERIFIED FIXED
13 years ago
11 years ago

People

(Reporter: Jonathan Stanley, Assigned: ted)

Tracking

({regression})

Trunk
x86
Windows XP
regression
Points:
---
Bug Flags:
blocking1.9 -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

13 years ago
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

13 years ago
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.
Version: unspecified → 1.5 Branch

Comment 2

13 years ago
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.
Keywords: regression
(Reporter)

Comment 3

13 years ago
(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.
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

Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 5

13 years ago
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.
Assignee: nobody → emaijala
Component: Location Bar and Autocomplete → Widget: Win32
Product: Firefox → Core
Version: 1.5 Branch → Trunk
Flags: blocking1.9a1?
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.
*** Bug 327339 has been marked as a duplicate of this bug. ***
*** Bug 324184 has been marked as a duplicate of this bug. ***
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.
Assignee: emaijala → ted.mielczarek
QA Contact: location.bar → ian
*** Bug 347194 has been marked as a duplicate of this bug. ***

Updated

12 years ago
Flags: blocking1.9a1? → blocking1.9-
Whiteboard: [wanted-1.9]
Created attachment 239750 [details] [diff] [review]
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.
Attachment #239750 - Flags: review?
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 on attachment 239750 [details] [diff] [review]
don't step on CF_UNICODETEXT with url flavours

Neil, care to take a look?
Attachment #239750 - Flags: review? → review?(neil)

Comment 14

12 years ago
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.
Attachment #239750 - Flags: review?(neil) → review+
Attachment #239750 - Flags: superreview?(roc)
Attachment #239750 - Flags: superreview?(roc) → superreview+
Whiteboard: [wanted-1.9] → [wanted-1.9] [checkin needed]
Checked in by timeless.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Verified on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060925 Minefield/3.0a1
Status: RESOLVED → VERIFIED
Whiteboard: [wanted-1.9] [checkin needed]
(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. 
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.
Blocks: 372271

Comment 19

11 years ago
Created attachment 265776 [details] [diff] [review]
Branch Patch

Branch Patch. Does the same, really.
Attachment #265776 - Flags: superreview?(roc)
Attachment #265776 - Flags: review?(roc)
Attachment #265776 - Flags: approval1.8.1.5?

Comment 20

11 years ago
Comment on attachment 265776 [details] [diff] [review]
Branch Patch

Oops. Sorry for bugspam.
Attachment #265776 - Flags: review?(roc) → review?(neil)
Comment on attachment 265776 [details] [diff] [review]
Branch Patch

sr+, but IMHO this patch does not meet branch criteria.
Attachment #265776 - Flags: superreview?(roc) → superreview+

Comment 22

11 years ago
(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.

Updated

11 years ago
Attachment #265776 - Flags: review?(neil) → review+
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.
Attachment #265776 - Flags: approval1.8.1.5? → approval1.8.1.5-
You need to log in before you can comment on or make changes to this bug.