Last Comment Bug 846765 - Dragging from Downloads window and dropping in Finder creates weird shortcut
: Dragging from Downloads window and dropping in Finder creates weird shortcut
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Cocoa (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal with 1 vote (vote)
: mozilla25
Assigned To: Marco Castelluccio [:marco]
:
Mentors:
: 875381 (view as bug list)
Depends on:
Blocks: 462172
  Show dependency treegraph
 
Reported: 2013-03-01 07:30 PST by David Tenser [:djst]
Modified: 2013-08-02 06:13 PDT (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (3.46 KB, patch)
2013-08-01 00:40 PDT, Marco Castelluccio [:marco]
no flags Details | Diff | Splinter Review
Patch v2 (3.52 KB, patch)
2013-08-01 10:39 PDT, Marco Castelluccio [:marco]
bgirard: review+
khuey: feedback+
Details | Diff | Splinter Review

Description David Tenser [:djst] 2013-03-01 07:30:51 PST
Bug 462172 appears to be cross-platform, but I noticed today that it doesn't work on the Mac. This is tested on Firefox 19 running on OS X 10.8.2. 

When dropping a file from the Download window on Firefox onto e.g. the desktop, a strange shortcut is created instead of actually moving/copying the file there. This is different from how it works on Windows.
Comment 1 Jesse Ruderman 2013-05-02 01:11:41 PDT
A weird shortcut indeed.  It's called "Users:.fileloc" in terminal or "Users/.fileloc" in finder, and it has an Opera icon.  But double-clicking it opens the PDF I downloaded in Preview.
Comment 2 Marco Castelluccio [:marco] 2013-07-31 20:06:29 PDT
*** Bug 875381 has been marked as a duplicate of this bug. ***
Comment 3 Marco Castelluccio [:marco] 2013-07-31 20:10:07 PDT
Is bug 544932 the problem?
Comment 4 Marco Castelluccio [:marco] 2013-08-01 00:40:36 PDT
Created attachment 784254 [details] [diff] [review]
Patch
Comment 5 Kyle Huey [:khuey] (khuey@mozilla.com) 2013-08-01 09:41:03 PDT
Comment on attachment 784254 [details] [diff] [review]
Patch

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

::: widget/cocoa/nsClipboard.mm
@@ +483,5 @@
> +      if (!file) {
> +        nsCOMPtr<nsISupportsInterfacePointer> ptr(do_QueryInterface(genericFile));
> +        if (ptr) {
> +          ptr->GetData(getter_AddRefs(file));
> +        }

I'm kind of surprised this compiles.  GetData returns an nsISupports*, not an nsIFile*.  I would expect you to need to pass in an getter_AddRefs<nsISupports> here and QI to nsIFile afterwards.
Comment 6 Marco Castelluccio [:marco] 2013-08-01 09:55:56 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #5)
> I'm kind of surprised this compiles.  GetData returns an nsISupports*, not
> an nsIFile*.  I would expect you to need to pass in an
> getter_AddRefs<nsISupports> here and QI to nsIFile afterwards.

On Windows we're doing the same: http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsDataObj.cpp#1357
Comment 7 Marco Castelluccio [:marco] 2013-08-01 10:39:21 PDT
Created attachment 784461 [details] [diff] [review]
Patch v2
Comment 8 Kyle Huey [:khuey] (khuey@mozilla.com) 2013-08-01 10:41:20 PDT
(In reply to Marco Castelluccio [:marco] from comment #6)
> (In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #5)
> > I'm kind of surprised this compiles.  GetData returns an nsISupports*, not
> > an nsIFile*.  I would expect you to need to pass in an
> > getter_AddRefs<nsISupports> here and QI to nsIFile afterwards.
> 
> On Windows we're doing the same:
> http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsDataObj.
> cpp#1357

Yeah turns out this is a bug in nsCOMPtr.  This shouldn't be allowed.
Comment 9 Kyle Huey [:khuey] (khuey@mozilla.com) 2013-08-01 10:42:37 PDT
Comment on attachment 784461 [details] [diff] [review]
Patch v2

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

Looks reasonable.
Comment 10 Ryan VanderMeulen [:RyanVM] 2013-08-01 18:28:58 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0f70faea778
Comment 11 Ed Morley [:emorley] 2013-08-02 05:35:04 PDT
https://hg.mozilla.org/mozilla-central/rev/d0f70faea778

Note You need to log in before you can comment on or make changes to this bug.