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

NEW
Unassigned

Status

()

defect
P5
normal
7 years ago
11 months ago

People

(Reporter: jaws, Unassigned)

Tracking

({regression})

13 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 4 obsolete attachments)

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.
Depends on: 712184
Posted patch Patch for bug (Windows) (obsolete) — Splinter Review
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.
Attachment #612285 - Flags: review?(ttaubert)
Attachment #612270 - Flags: review?(dao)
OS: Windows 7 → All
Hardware: x86_64 → All
Comment on attachment 612270 [details] [diff] [review]
Patch for bug (Windows)

Tim, do you think you can review this?
Attachment #612270 - Flags: review?(dao) → review?(ttaubert)
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.
Attachment #612285 - Flags: review?(ttaubert)
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

7 years ago
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 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.
Attachment #612270 - Flags: review?(ttaubert)
Attachment #612270 - Attachment is obsolete: true
Attachment #617527 - Flags: review?(dao)
I fixed the issue with the drag panel disappearing on Linux.
Attachment #612285 - Attachment is obsolete: true
Attachment #617529 - Flags: review?(ttaubert)
Comment on attachment 617527 [details] [diff] [review]
Patch for bug v2 (browser/base, Windows)

Clearing review, feedback was given offline.
Attachment #617527 - Flags: review?(dao)
Attachment #617529 - Flags: review?(ttaubert)
Posted patch Patch for bugSplinter Review
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.
Attachment #617527 - Attachment is obsolete: true
Attachment #617529 - Attachment is obsolete: true
Attachment #627598 - Flags: review?(ttaubert)
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?
Attachment #627598 - Flags: review?(ttaubert) → review+
Tim, will the new #identity-drag-panel land in another bug or has it been dropped for now?
(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.
(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?
(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.
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce426796f224
Flags: in-testsuite-
Target Milestone: --- → Firefox 15
No longer depends on: 712184
https://hg.mozilla.org/mozilla-central/rev/ce426796f224
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
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
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.
Sure. Just want to know if you or anyone else is able to do it ATM.
This is indeed not fixed in the 2012-06-01 nightly built from http://hg.mozilla.org/mozilla-central/rev/73783bf75c4c.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Attachment #629459 - Flags: review?(ttaubert)
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?
Attachment #629459 - Flags: review?(ttaubert) → review-
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.
Attachment #629459 - Flags: review- → review?(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?
Attachment #629459 - Flags: review?(dao) → review-
Also, this is probably testable... not the drop on the desktop, but the drag flavors and content.
Flags: in-testsuite- → in-testsuite?

Comment 28

6 years ago
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.
Duplicate of this bug: 749576

Comment 30

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

6 years ago
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.
Unassigning as I haven't been working on this and I don't want to hold people up.
Assignee: jaws → nobody
Status: REOPENED → NEW
Target Milestone: Firefox 15 → ---
Flags: in-testsuite?
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.