742441 - Cannot drag URL from location bar to desktop (create shortcut)

Status: RESOLVED WORKSFORME

(Firefox :: Address Bar, defect, P5)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Bug 478070 was supposed to fix dragging the URL from the location bar to the desktop, but I have not been able to see it working for me on Windows 7.

The editor is now using the new HTML5 drag/drop APIs, but it was broken for me before that switch.

I have a patch that fixes this on Windows (and likely all desktop platforms) and will upload it shortly.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug (Windows)

This bug includes the core functionality as well as the Windows theme changes. I'll upload patches that implement the theme changes for Linux and OS X soon.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug (OS X and Linux)

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 612270 [details] [diff] [review]
Patch for bug (Windows)

Tim, do you think you can review this?

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 612285 [details] [diff] [review]
Patch for bug (OS X and Linux)

The drag operation is cancelled almost immediately on Linux, so I'll have to do some more investigation here to find out why.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 612270 [details] [diff] [review]
Patch for bug (Windows)

So the main change here is replacing the dragImage with a <panel> that contains the dragImage. Why exactly does that fix drag&drop to create shortcuts? Does the panel have any special handling?

Comment 6

[David E. Ross]

Please clarify.    

When I hover my cursor over the icon in the address area (location bar) to the left of the URI, I see a tooltip that says "Drag and drop this icon to create a link to this page".  I can drag that icon to my desktop to create an Internet shortcut without any problem.  

Is the intent of this bug to drag the URI itself instead of the icon?  If so, was this ever a capability?

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 612270 [details] [diff] [review]
Patch for bug (Windows)

Cancelling review for now. This bug can be fixed by a simpler patch, and I'll move the panel addition to another bug.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug v2 (browser/base, Windows)

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug v2 (OS X and Linux)

I fixed the issue with the drag panel disappearing on Linux.

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 617527 [details] [diff] [review]
Patch for bug v2 (browser/base, Windows)

Clearing review, feedback was given offline.

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug

This patch is much simpler than the previous patches and just switches the urlbarbindings to use the dragstart event instead of the draggesture event.

There is a bug in our draggesture event with editor fields where the dataTransfer flavors are cleared.

Using dragstart gets us around that bug, uses the newer API, and also fixes one of the TODOs in our code.

Comment 12

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 627598 [details] [diff] [review]
Patch for bug

Review of attachment 627598 [details] [diff] [review]:
-----------------------------------------------------------------

Nice fix. r=me with the question answered.

::: browser/base/content/urlbarBindings.xml
@@ -714,5 @@
> -        //       See bug 499008 for details.
> -
> -        // Drag only if the gesture starts from the input field.
> -        if (event.originalTarget != this.inputField)
> -          return;

Do we still need this kind of check?

Comment 13

[Siddhartha Dugar [:sdrocking]]

Tim, will the new #identity-drag-panel land in another bug or has it been dropped for now?

Comment 14

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Siddhartha Dugar [:sdrocking] from comment #13)
> Tim, will the new #identity-drag-panel land in another bug or has it been
> dropped for now?

Looks like we don't need it anymore for now.

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Siddhartha Dugar [:sdrocking] from comment #13)
> Tim, will the new #identity-drag-panel land in another bug or has it been
> dropped for now?

I'd still like to implement it, I just need to file a new bug for it. sdrocking, can you file one for it and assign it to me?

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Tim Taubert [:ttaubert] from comment #12)
> Comment on attachment 627598 [details] [diff] [review]
> Patch for bug
> 
> Review of attachment 627598 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Nice fix. r=me with the question answered.
> 
> ::: browser/base/content/urlbarBindings.xml
> @@ -714,5 @@
> > -        //       See bug 499008 for details.
> > -
> > -        // Drag only if the gesture starts from the input field.
> > -        if (event.originalTarget != this.inputField)
> > -          return;
> 
> Do we still need this kind of check?

Yeah, we do need that check. Without it you can drag the refresh button to the desktop, which is kinda weird and I wouldn't want people to start getting used to it :). I'll add it back.

Comment 17

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ce426796f224

Comment 18

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ce426796f224

Comment 19

[Siddhartha Dugar [:sdrocking]]

Does this bug really fix the issue? I am still unable to save the URL as a shortcut on my desktop using the latest Nightly. Some screenshots of the problem can be seen on http://forums.mozillazine.org/viewtopic.php?f=23&t=2481137&p=12028349#p12028349

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

Can you try again with tomorrow's nightly? Nightly updates have been turned on/off randomly within the past week so you might not be running the latest code.

If you still encounter the issue, please file a new bug and mark it as blocking this one.

Comment 21

[Siddhartha Dugar [:sdrocking]]

Sure. Just want to know if you or anyone else is able to do it ATM.

Comment 22

[Dão Gottwald [:dao]]

This is indeed not fixed in the 2012-06-01 nightly built from http://hg.mozilla.org/mozilla-central/rev/73783bf75c4c.

Comment 23

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug (part 2)

In fact I did need to change those two lines that I had adjusted in the earlier patch.

This patch only allows events that started in the text of the location bar so that the star button and refresh button don't allow drags. This will not remove the ability to drag the site identity icon.

Comment 24

[Dão Gottwald [:dao]]

Comment on attachment 629459 [details] [diff] [review]
Patch for bug (part 2)

gURLBar is 'this' here.

Can you explain why the originalTarget check stopped working?

Comment 25

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 629459 [details] [diff] [review]
Patch for bug (part 2)

event.originalTarget is an [object Text] in both draggesture and dragstart, so dragging the URL here was getting blocked since the this.inputField is [object HTMLInputField].

event.target is gURLBar (aka 'this') [object XULElement] in both events as well.

It seems that the simple fix for this bug is just to change this check here to only check event.target != gURLBar. This apparently would have worked with draggesture as well (but since I'm changing this area of the code and bug 499008 has been fixed now it is a good time to fix the TODO).

I can change the patch to use 'this' instead of gURLBar when I land.

Comment 26

[Dão Gottwald [:dao]]

Comment on attachment 629459 [details] [diff] [review]
Patch for bug (part 2)

Checking target is bad since this is going to be true for any anonymous content in the location bar. How about checking whether originalTarget is a text node?

Comment 27

[Dão Gottwald [:dao]]

Also, this is probably testable... not the drop on the desktop, but the drag flavors and content.

Comment 28

[Ian Nartowicz]

Is the current problem just with dragging the selected URL from the urlbar, or also with dragging the icon?  The datatransfer type when dragging the text used to be application/x-moz-file (eg. in FF10ESR), but is now text/plain (eg. FF17).  I know this breaks some things, maybe some desktops too.

Comment 30

[Martin Boggan]

Since installing the last firefox update I am getting an error message when I try to drag a url icon to the desktop for a shortcut. The error message is "the device is not ready." I'm using Windows 7, am not sure whether this is a windows 7 problme or a firefox problem. Once upon a time I could actually copy a firefox bookmark and paste it to the desktop for a shortcut. That was really easy, but now I'm getting that same error message trying to do that.

Comment 31

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Martin Boggan from comment #30)
> Since installing the last firefox update I am getting an error message when
> I try to drag a url icon to the desktop for a shortcut. The error message is
> "the device is not ready." I'm using Windows 7, am not sure whether this is
> a windows 7 problme or a firefox problem. Once upon a time I could actually
> copy a firefox bookmark and paste it to the desktop for a shortcut. That was
> really easy, but now I'm getting that same error message trying to do that.

I just tested in Firefox 21 Nightly on Windows 7 and was unable to reproduce the problem you have described. I dragged the lock icon, which can also be a globe icon, to the desktop and a shortcut was created for me.

Can you please test in Safe Mode (Help -> Restart with Add-ons Disabled...) and see if your issue still occurs? If it does, please file a new bug so we can diagnose it further there. Thanks!

Comment 32

[Martin Boggan]

Thanks, Jared. I followed your suggestion and yes, that worked. So, it would seem I have some kind of interference in my pc functions. I will need to check out the running processes via task manager when the problem reoccurs.

I'm embarrassed that I didn't think to try safe mode. Thanks loads for the reminder.

Comment 33

[Jared Wein [:jaws] (please needinfo? me)]

Unassigning as I haven't been working on this and I don't want to hold people up.

Comment 34

[Florencia Di Ciocco, NI to vbalducci]

Hi, I'm closing this bug as WFM since I think this has been done. If I'm wrong, please do reopen it.