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

Status: VERIFIED FIXED

(Firefox :: Downloads Panel, defect, P3)

Description

[JustinCB]

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

Comment 1

[JustinCB]

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.

Comment 2

[JustinCB]

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)

Comment 3

[:Paolo Amadini]

Is there anyone from QA available to find a regression range for this bug?

Comment 4

[ovidiu boca[:Ovidiu]]

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

Comment 5

[:Paolo Amadini]

(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.

Comment 6

[ovidiu boca[:Ovidiu]]

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).

Comment 7

[:Paolo Amadini]

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?

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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?

Comment 9

[ovidiu boca[:Ovidiu]]

Thanks Sebastian for your help. I'll clean the NI? flag.

Comment 12

[Stephen A Pohl [:spohl]]

Attachment: Patch

|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.

Comment 13

[Markus Stange [: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.

Comment 14

[Stephen A Pohl [:spohl]]

[Tracking Requested - why for this release]:
Regression from bug 1235162.

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 16

[Stephen A Pohl [:spohl]]

Attachment: Patch

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.

Comment 17

[Markus Stange [:mstange]]

Comment on attachment 8870160 [details] [diff] [review]
Patch

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

Thanks, looks good.

Comment 18

[Stephen A Pohl [:spohl]]

Attachment: Patch

Updated for the new patch 2 in bug 1358075. Carrying over r+.

Comment 19

[Stephen A Pohl [:spohl]]

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

Comment 20

[Ryan VanderMeulen [:RyanVM]]

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

Comment 21

[Stephen A Pohl [:spohl]]

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

Comment 22

[Julien Cristau [:jcristau]]

Comment on attachment 8870269 [details] [diff] [review]
Patch

aurora isn't a thing anymore.  Moving uplift request to beta.

Comment 23

[Gerry Chang [:gchang]]

Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!

Comment 24

[Brindusa Tot, Desktop QA]

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

Comment 25

[Gerry Chang [:gchang]]

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.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

This can't be uplifted to Beta until a decision on bug 1358075 is made.

Comment 27

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 28

[Gerry Chang [:gchang]]

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.

Comment 30

[Brindusa Tot, Desktop QA]

Verified as fixed on latest Nightly version 55.0a1, Build ID 20170526030203 on Mac OS X 10.12.

Comment 31

[Sylvestre Ledru [:Sylvestre]]

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')

Comment 32

[Sylvestre Ledru [:Sylvestre]]

Maybe because bug 1358075 didn't land yet?

Comment 33

[JustinCB]

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.

Comment 34

[Stephen A Pohl [:spohl]]

(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.

Comment 35

[Julien Cristau [:jcristau]]

Comment on attachment 8870269 [details] [diff] [review]
Patch

fix drag&drop regression from bug 1235162, beta54+

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8a44e418ac5a

Comment 37

[Ciprian Georgiu, Desktop QA]

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.