Closed Bug 1330470 Opened 3 years ago Closed 3 years ago

Drag and drop from downloads dropdown(to finder, iTunes, etc) doesn't work on the macintosh

Categories

(Firefox :: Downloads Panel, defect, P3)

53 Branch
x86_64
macOS
defect

Tracking

()

VERIFIED FIXED
Firefox 55
Tracking Status
firefox-esr52 --- unaffected
firefox53 - wontfix
firefox54 + verified
firefox55 + verified

People

(Reporter: JustinCB, Assigned: spohl)

References

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170110075905

Steps to reproduce:

1. download a file in firefox
2. click the downloads dropdown(looks like a down arrow near the top right corner)
3. Drag a file and try to drop into another application


Actual results:

The file will not change if held over the window of another application and dropping it doesn't do anything


Expected results:

When the file is held over another window, it should look like a document that is part of the interface and when dropped the other application should notice/receive the file
This appeared in one of the nightlies in the past few weeks.  Before that drag-and-drop in the downloads panel worked fine.  I am using mac os x 10.11.6 on a mac mini(Late 2012) with a RAM upgrade to 16 GB(which is the only upgrade).  Electrolosys(e10s) is not disabled.
Severity: normal → minor
Component: Untriaged → Downloads Panel
OS: Unspecified → Mac OS X
Hardware: Unspecified → x86_64
It does the same thing non-e10s.  When the application is finder it will drop "Users/.fileloc", unless "Users/.fileloc" already exists, then it will drop "Users/ 2.fileloc", "Users/ 3.fileloc", etc, etc.  (The actuall filenames are without quotes)
Flags: qe-verify+
Is there anyone from QA available to find a regression range for this bug?
Flags: needinfo?(brindusa.tot)
Priority: -- → P3
Hi Justin,

I tried to reproduce this issue on Mac with FF Nightly 55.0a1(2017-05-07) and the actual result in my case is that when I drag and drop a downloaded file this is received in the pointed folder but under this form "Users/ovidiu.boca/Downloads/10MB-2.zip.fileloc" and I'm intrigued by the fact that from your actual result stands out the fact that dropping the downloaded file it doesn't do anything.

1. Can you please try to reproduce this with the latest Nightly, you can download it from here: https://nightly.mozilla.org/
2. For a better understanding can you please do a screen recording with the initial issue? You can use QuickTime Player, open the QuickTime Player in the dock and then push right click on it and you will see the screen recorder option. 

Thanks
Flags: needinfo?(brindusa.tot) → needinfo?(JustinCB)
(In reply to ovidiu boca[:Ovidiu] from comment #4)
> this is received in the pointed folder but under this form
> "Users/ovidiu.boca/Downloads/10MB-2.zip.fileloc"

This is still a bug, we just need to find out what is the change that caused it.

Most probably, when dragging the file over applications other than Finder, the operation wouldn't work at all, hence the original steps to reproduce. A screen recording is a bonus, but I don't think we need to wait on one.
Flags: needinfo?(JustinCB)
I did a regression range and here is the result:

Last good revision: 567894f026558e6dada617a3998f29aed06ac7d8 (2016-12-20)
First bad revision: 5b0afeaeebdd3a60e1885912cda2e48a9233be52 (2016-12-21)

Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=567894f026558e6dada617a3998f29aed06ac7d8&tochange=5b0afeaeebdd3a60e1885912cda2e48a9233be52


I couldn't get deeper with this regression range because I have this output:

14:26.63 LOG: MainThread main INFO Switching bisection method to taskcluster
14:26.63 LOG: MainThread main INFO Getting mozilla-central builds between 567894f026558e6dada617a3998f29aed06ac7d8 and 5b0afeaeebdd3a60e1885912cda2e48a9233be52
14:30.69 LOG: Thread-98 Bisector WARNING Skipping build 5b0afeaeebdd3a60e1885912cda2e48a9233be52: Unable to find build info using the taskcluster route 'buildbot.revisions.5b0afeaeebdd3a60e1885912cda2e48a9233be52.mozilla-central.macosx64'
14:30.89 LOG: Thread-96 Bisector WARNING Skipping build 567894f026558e6dada617a3998f29aed06ac7d8: Unable to find build info using the taskcluster route 'buildbot.revisions.567894f026558e6dada617a3998f29aed06ac7d8.mozilla-central.macosx64'
14:31.00 LOG: Thread-97 Bisector WARNING Skipping build 7083c0d30e75fc102c715887af9faec933e936f8: Unable to find build info using the taskcluster route 'buildbot.revisions.7083c0d30e75fc102c715887af9faec933e936f8.mozilla-central.macosx64'
14:32.30 LOG: Thread-100 Bisector WARNING Skipping build 20774bffb62a3c1fecf98ed8ad9ee1a861bcd9b7: Unable to find build info using the taskcluster route 'buildbot.revisions.20774bffb62a3c1fecf98ed8ad9ee1a861bcd9b7.mozilla-central.macosx64'
14:32.59 LOG: Thread-101 Bisector WARNING Skipping build c36fbe84042debef0a5d58b7fc88185b401762ce: Unable to find build info using the taskcluster route 'buildbot.revisions.c36fbe84042debef0a5d58b7fc88185b401762ce.mozilla-central.macosx64'
14:32.59 LOG: Thread-99 Bisector WARNING Skipping build 5206da513654c0e0b36293c9ce149ef9ed907a41: Unable to find build info using the taskcluster route 'buildbot.revisions.5206da513654c0e0b36293c9ce149ef9ed907a41.mozilla-central.macosx64'
14:34.16 LOG: Thread-102 Bisector WARNING Skipping build 0f3603e365f029bc9ea5f926d32f6f0f8aa4d998: Unable to find build info using the taskcluster route 'buildbot.revisions.0f3603e365f029bc9ea5f926d32f6f0f8aa4d998.mozilla-central.macosx64'
14:34.16 LOG: MainThread main INFO There are no build artifacts on inbound for these changesets (they are probably too old).
To be useful, the range should be limited to a specific bug or just a few bugs. I'm not familiar with mozregression, but maybe someone from QA can suggest ways to find the actual regressing bug number in this case?
Flags: needinfo?(ovidiu.boca)
Prime suspect: bug 1235162.

It's inbound push doesn't show the results anymore https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=82ca4696b3425b6b5674ef48422d93a63dda2554 , so we have no inbound builds available to narrow that window down without backing out bug 1235162 and pushing that to Try.

spohl, can you take a look at this, please?
Blocks: 1235162
Flags: needinfo?(spohl.mozilla.bugs)
Assignee: nobody → spohl.mozilla.bugs
Thanks Sebastian for your help. I'll clean the NI? flag.
Flags: needinfo?(ovidiu.boca)
Duplicate of this bug: 1362078
Duplicate of this bug: 1365267
Attached patch Patch (obsolete) — Splinter Review
|NSFilenamesPboardType| is no longer a valid type for the new drag & drop API. Instead, we have to write the file URL directly to the pasteboard using NSPasteboard's |writeObjects:|. We previously used the |NSFilenamesPboardType| type to store the filenames in the NSDictionary. To retain the ability to store the URLs in the dictionary, I've introduced another internal type |kFileUrlsPboardType| to pass the file URL back and forth. In addition to writing the URL directly to the pasteboard, we also have to write it to the pasteboard as NSString* for |kUTTypeFileURL|. This seems to fully restore the previous functionality in my testing.
Flags: needinfo?(spohl.mozilla.bugs)
Attachment #8868658 - Flags: review?(mstange)
Comment on attachment 8868658 [details] [diff] [review]
Patch

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

::: widget/cocoa/nsDragService.mm
@@ +51,5 @@
>  NSString* const kPublicUrlPboardType      = @"public.url";
>  NSString* const kPublicUrlNamePboardType  = @"public.url-name";
>  NSString* const kUrlsWithTitlesPboardType = @"WebURLsWithTitlesPboardType";
>  NSString* const kCustomTypesPboardType    = @"org.mozilla.custom-clipdata";
> +NSString* const kFileUrlsPboardType       = @"org.mozilla.file-urls";

It might be nice to have some prefix or postfix on the constant name that indicates whether this type is something that we only use internally in Firefox (or Gecko applications in general) or if it's something that has some de-facto standard behavior in other parts of macOS.
Attachment #8868658 - Flags: review?(mstange) → review+
[Tracking Requested - why for this release]:
Regression from bug 1235162.
Too late to fix this in 53, but we might still take a patch in 54 if we can land this and verify the fix.
Attached patch Patch (obsolete) — Splinter Review
Sending back for one more review. I've updated this for the new patch 2 in bug 1358075 and addressed the following:

(In reply to Markus Stange [:mstange] from comment #13)
> Comment on attachment 8868658 [details] [diff] [review]
> Patch
> 
> Review of attachment 8868658 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/cocoa/nsDragService.mm
> @@ +51,5 @@
> >  NSString* const kPublicUrlPboardType      = @"public.url";
> >  NSString* const kPublicUrlNamePboardType  = @"public.url-name";
> >  NSString* const kUrlsWithTitlesPboardType = @"WebURLsWithTitlesPboardType";
> >  NSString* const kCustomTypesPboardType    = @"org.mozilla.custom-clipdata";
> > +NSString* const kFileUrlsPboardType       = @"org.mozilla.file-urls";
> 
> It might be nice to have some prefix or postfix on the constant name that
> indicates whether this type is something that we only use internally in
> Firefox (or Gecko applications in general) or if it's something that has
> some de-facto standard behavior in other parts of macOS.

I've changed this to use kMoz... for all internal types.
Attachment #8868658 - Attachment is obsolete: true
Attachment #8870160 - Flags: review?(mstange)
Comment on attachment 8870160 [details] [diff] [review]
Patch

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

Thanks, looks good.
Attachment #8870160 - Flags: review?(mstange) → review+
Attached patch PatchSplinter Review
Updated for the new patch 2 in bug 1358075. Carrying over r+.
Attachment #8870160 - Attachment is obsolete: true
Attachment #8870269 - Flags: review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0aad1d6b4acce125b5d93fcdd41141c3e2ede16
Bug 1330470: Restore ability to drag & drop entries from the download list to Finder on OSX. r=mstange
https://hg.mozilla.org/mozilla-central/rev/d0aad1d6b4ac
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Comment on attachment 8870269 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/Bug causing the regression]: bug 1235162
[User impact if declined]: Users will be unable to drag & drop entries from the list of downloads to Finder like they used to. The drags will fail.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: Yes, preferably. Steps in comment 0.
[List of other uplifts needed for the feature/fix]: bug 1358755 and bug 1358075
[Is the change risky?]: minimally
[Why is the change risky/not risky?]: The code that is being removed here was non-functional after moving to a new drag & drop API in bug 1235162. The new code is well understood and has undergone thorough manual testing. Minimal risk remains due to the various versions of OSX/macOS that we support.
[String changes made/needed]: none
Attachment #8870269 - Flags: approval-mozilla-aurora?
Comment on attachment 8870269 [details] [diff] [review]
Patch

aurora isn't a thing anymore.  Moving uplift request to beta.
Attachment #8870269 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(brindusa.tot)
I was able to reproduce this issue on an older Nightly 55.0a1, from 05.21.2017.

Verified as fixed on latest Nightly 55.0a1, build ID 20170524030204, on Mac OS X 10.12
Status: RESOLVED → VERIFIED
Flags: needinfo?(brindusa.tot)
Comment on attachment 8870269 [details] [diff] [review]
Patch

Fix an issue that users are unable to drag & drop entries from the list of downloads to Finder and was verified. Beta54+. Should be in 54 beta 11.
Attachment #8870269 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
This can't be uplifted to Beta until a decision on bug 1358075 is made.
Flags: needinfo?(gchang)
Comment on attachment 8870269 [details] [diff] [review]
Patch

Let's land this once we have verification and approval for the work in bug 1358075 and bug 1367720.  Bug 1358755 already landed on beta, but it's OK on its own according to RyanVM and spohl. This should wait to land with the other dependencies, though.
Attachment #8870269 - Flags: approval-mozilla-beta+ → approval-mozilla-beta?
Hi Brindusa, 
Per comment #27, need your help to verify this and bug 1358075 and bug 1367720 as well again. Once they are all verified, we can get them in 54.0b12.
Flags: needinfo?(gchang) → needinfo?(brindusa.tot)
Duplicate of this bug: 1367926
Verified as fixed on latest Nightly version 55.0a1, Build ID 20170526030203 on Mac OS X 10.12.
Flags: needinfo?(brindusa.tot)
Fails to apply to beta:

 warning: conflicts while merging widget/cocoa/nsChildView.mm! (edit, then use 'hg resolve --mark')
warning: conflicts while merging widget/cocoa/nsClipboard.mm! (edit, then use 'hg resolve --mark')
warning: conflicts while merging widget/cocoa/nsDragService.h! (edit, then use 'hg resolve --mark')
warning: conflicts while merging widget/cocoa/nsDragService.mm! (edit, then use 'hg resolve --mark')
Flags: needinfo?(spohl.mozilla.bugs)
Maybe because bug 1358075 didn't land yet?
It appears to be working now.  When it wasn't working, it dropped Users/.fileloc.  A few nightlies later it would drop Users/<username>/filepath.fileloc.  When I dropped it over finder, it would drop the .fileloc file.  When I dropped it over iTunes or some other application that didn't understand .fileloc files, it would just stay the same as it was when I started to dragging it from the recent 3 downloads for this session.  I think if I dropped it over an application that understood .fileloc files, the application would try to open the file.
(In reply to Sylvestre Ledru [:sylvestre] from comment #31)
> Fails to apply to beta:
> 
>  warning: conflicts while merging widget/cocoa/nsChildView.mm! (edit, then
> use 'hg resolve --mark')
> warning: conflicts while merging widget/cocoa/nsClipboard.mm! (edit, then
> use 'hg resolve --mark')
> warning: conflicts while merging widget/cocoa/nsDragService.h! (edit, then
> use 'hg resolve --mark')
> warning: conflicts while merging widget/cocoa/nsDragService.mm! (edit, then
> use 'hg resolve --mark')

(In reply to Sylvestre Ledru [:sylvestre] from comment #32)
> Maybe because bug 1358075 didn't land yet?

Yes, as stated in comment 21.
Flags: needinfo?(spohl.mozilla.bugs)
Comment on attachment 8870269 [details] [diff] [review]
Patch

fix drag&drop regression from bug 1235162, beta54+
Attachment #8870269 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I've reproduced this issue with an old Nightly build 55.0a1 (2017-05-01).

This is also verified fixed on FF 54.0 (20170605134926) under Mac OS X 10.11.6.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.