Last Comment Bug 337051 - Firefox for Mac OS X does not write kMDItemWhereFroms metadata attribute to downloaded files (put URLs in "More Info" section)
: Firefox for Mac OS X does not write kMDItemWhereFroms metadata attribute to d...
Status: RESOLVED FIXED
[sg:want]
: sec-want
Product: Firefox
Classification: Client Software
Component: File Handling (show other bugs)
: unspecified
: All Mac OS X
: P3 enhancement with 21 votes (vote)
: Firefox 51
Assigned To: Josh Aas
:
Mentors:
: 503629 566068 623882 630289 (view as bug list)
Depends on: 1302646
Blocks:
  Show dependency treegraph
 
Reported: 2006-05-07 13:34 PDT by pelle.beckman
Modified: 2016-09-15 16:31 PDT (History)
39 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
Use the undocumented MDItemSetAttribute call (3.41 KB, patch)
2007-12-07 15:57 PST, Mark Mentovai
no flags Details | Diff | Splinter Review
security dialog that does not appear for Firefox users (46.83 KB, image/png)
2011-01-31 17:22 PST, Jesse Ruderman
no flags Details
Patch for this bug and for 401748. (14.81 KB, patch)
2012-04-08 15:33 PDT, stefan.vaillant
smichaud: feedback+
Details | Diff | Splinter Review
Patch for this bug and bug 401748, updated to current trunk (15.21 KB, patch)
2012-10-08 16:22 PDT, Steven Michaud [:smichaud] (Retired)
mstange: review+
Details | Diff | Splinter Review
Josh Fix v0.9 (9.62 KB, patch)
2016-08-20 21:42 PDT, Josh Aas
no flags Details | Diff | Splinter Review
Josh fix v1.0 (12.53 KB, patch)
2016-08-25 22:20 PDT, Josh Aas
paolo.mozmail: review+
mstange: review+
Details | Diff | Splinter Review
Josh fix v1.1 (12.12 KB, patch)
2016-09-12 16:07 PDT, Josh Aas
no flags Details | Diff | Splinter Review
Followup Fix v1.0 (1.67 KB, patch)
2016-09-13 21:33 PDT, Josh Aas
gijskruitbosch+bugs: review+
Details | Diff | Splinter Review

Description pelle.beckman 2006-05-07 13:34:25 PDT
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; sv-SE; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; sv-SE; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3

Files indexed in Mac OS X 10.4 metadata store may have the kMDItemWhereFroms attribute (see http://developer.apple.com/documentation/Carbon/Reference/MetadataAttributesRef/index.html for reference). This allows for using Mac OS X own search system to search for files based on where they were downloaded from.
This feature can be seen in Safari; download a file (with Safari) and bring up it's details in Finder. You should see the address from where it was downloaded.

Reproducible: Always

Actual Results:  
-

Expected Results:  
-

-
Comment 1 Jo Hermans 2006-05-07 13:53:47 PDT
dupe of bug 90918 ?
Comment 2 pelle.beckman 2006-05-07 14:16:46 PDT
Well, I didn't read all of the comments on bug the bug mentioned (#90918) but from what I can tell that regards writing the address in the comments field (kMDItemFinderComment?). This is a good idea aswell since the kMDItemWhereFrom is _not_ duplicated when copying a file (hopefully that behaviour will change in later versions of OS X).

This feature would be OS X 10.4 (or later) specific. 
Comment 3 Smokey Ardisson (offline for a while; not following bugs - do not email) 2006-05-07 22:34:14 PDT
If bug 90918 ever gets fixed, Fx could use those methods to set that bit of metadata.  In the meantime, Fx can probably do something itself (within the limitations of the publicly-available APIs); see bug 325873 comment 4 for some discussion of the issue.
Comment 4 pelle.beckman 2006-05-08 06:33:04 PDT
Related bugs/enhancements:
#125729
#154580
#325873
#90918

Could this OS X 10.4 specific feature be implemented as an add-on?
I'm new to Mozilla/Firefox development so I can't really tell...

Related: see Ecamms DownloadComment Safari plug-in (http://www.ecamm.com/mac/free/) - it adds the download URL to comments for Safari.
Comment 5 Jacob Rus 2006-05-09 08:24:26 PDT
pbeck:

http://www.liquidx.net/blog/tag/coding/

-jacobolus
Comment 6 Jacob Rus 2006-05-09 16:55:13 PDT
also: http://tinyurl.com/juhae

Looks like it's an undocumented call to add kMDItemWhereFroms, but some people have gotten it working.
Comment 7 Colin Barrett [:cbarrett] 2007-10-30 21:40:25 PDT
In Mac OS X 10.5 there's a way to set this normally, with constants in LSQuarantine.h.

In any case it just looks to be a binary plist in the form of [data URL, origin URL] set on the com.apple.metadata:kMDItemWhereFroms xattr. Might be able to port it back to Tiger as well.
Comment 8 Mark Mentovai 2007-12-07 15:57:15 PST
Created attachment 292163 [details] [diff] [review]
Use the undocumented MDItemSetAttribute call

The LSQuarantine API won't set kMDItemWhereFroms.  I know, because I'm working with it over on bug 407215.

But this will.  (It's a patch to the Camino trunk and will only work in conjunction with the patch on bug 407215, which hasn't landed yet.)  It demonstrates the use of the undocumented MDItemSetAttribute call.

This patch is 10.4-and-up-only and requires the 10.4 SDK.  Since this is currently undocumented, in a landable patch, I might look up the symbol at runtime, in case it disappears from a future SDK or in case they decide to declare it publicly (in which case the declaration in this patch would conflict).
Comment 9 Mark Mentovai 2007-12-14 07:47:54 PST
Comment on attachment 292163 [details] [diff] [review]
Use the undocumented MDItemSetAttribute call

This proof-of-concept Camino implementation is obsolete, the real implementation is on bug 325873.  A core patch can be modeled after the work in that bug, possibly working in nsDownloadManager.cpp, in the same way that bug 401748 can do quarantining based on bug 407215.
Comment 10 Nicholas Shanks 2009-03-20 04:31:53 PDT
This is a security issue and should be treated as such.
Comment 11 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-05-15 07:57:45 PDT
*** Bug 566068 has been marked as a duplicate of this bug. ***
Comment 12 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-05-15 07:59:45 PDT
*** Bug 503629 has been marked as a duplicate of this bug. ***
Comment 13 Roger Wagner 2010-05-15 19:27:44 PDT
Just a word of thanks for the people looking into this.  HyperStudio 5 for Mac OS X now uses the feature in Safari extensively in support of Creative Commons licensing, and support of students doing classroom work where HyperStudio lets then put in the attributions and source URL for every media element. Thanks! Roger Wagner (Designer, HyperStudio)
Comment 14 viktor bergquist 2010-09-11 08:18:14 PDT
after extensive testing to get bash to deal with null-bytes (it would be nice in many hackish situations) and then hacking on macports's xattr (http://trac.macports.org/browser/trunk/dports/sysutils/xattr/Portfile) (mostly to get it to handle utf-8 file-paths correctly)
-
i finally try to set xattr com.apple.metadata:kMDItemWhereFroms to the xml form of a plist:
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"><plist version="1.0"><array><string>source</string></array></plist>
and by golly gosh it worked - simple as that

btw for those of you who want to know the data in the xattr is the binary1 format which can be converted to xml1 with the plutil(1) utility (but then you need a temp file cause the little thing doesn't think printing binary data "makes sense" :/ )

hope this helped
pzss \/ike
Comment 15 Kevin Brosnan 2011-01-31 12:05:58 PST
*** Bug 623882 has been marked as a duplicate of this bug. ***
Comment 16 Kevin Brosnan 2011-01-31 12:06:06 PST
*** Bug 630289 has been marked as a duplicate of this bug. ***
Comment 17 Jesse Ruderman 2011-01-31 17:22:13 PST
Created attachment 508627 [details]
security dialog that does not appear for Firefox users

Tested by downloading e.sh in http://www.squarefree.com/bug337051/.
Comment 18 Jesse Ruderman 2011-01-31 17:44:06 PST
For maximal security benefit, we'll also need to fix bug 401748.  com.apple.quarantine works best when kMDItemWhereFroms is also present.

(I sniffed around with mdls, GetFileInfo, and xattr.)
Comment 19 stefan.vaillant 2012-04-08 15:33:46 PDT
Created attachment 613200 [details] [diff] [review]
Patch for this bug and for 401748.

Overview:
- Core logic based on file_metadata_mac.{h,mm} from Chromium project.
- Core logic is called from nsDownload::setState, if state transitions to DOWNLOAD_FINISHED

Questions:
- Any idea how to avoid the ugly (const char*) casts?
- The include of file_metadata_mac from nsDownloadManager using #include "../../../ipc/chromium/src/chrome/common/file_metadata_mac.h"  does not feel right, any ideas?
Comment 20 stefan.vaillant 2012-04-08 15:52:35 PDT
Additional comments:
- This is only manually tested, automated test case seems non-trivial.
- Compiled against MacOS 10.6 SDK and not really latest mozilla-central.
Comment 21 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-04-13 11:00:17 PDT
Comment on attachment 613200 [details] [diff] [review]
Patch for this bug and for 401748.

Hi Stefan, thanks for the patch! I'll flag one of our Mac pros to provide some feedback on it. One thing I noticed is that you're adding code to ipc/chromium/, which probably isn't what we'd want to do - this code probably belongs somewhere under widget/cocoa, and we probably want to expose it via XPCOM on an existing interface (maybe nsILocalFileMac?). Markus may have better suggestions.
Comment 22 stefan.vaillant 2012-04-13 14:44:20 PDT
Hi Gavin, original reason to add it to ipc/chromium was that a) code is coming from chromium project b) there is already a similar, but unused file quarantine_mac.{h,mm} in the same folder.
Comment 23 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-04-13 15:25:30 PDT
Yeah, I was aware of the origin. There is some unused code in ipc/chromium because we just copied over things wholesale. There's no reason to put other, non-IPC related code under ipc/chromium, so it would be better to put it in a more logical spot based on function.

We'll need to confirm that the license terms for the copied code is suitable. I imagine it uses the same license as all the other chromium code that we've used, but we'll need to double check :)
Comment 24 Markus Stange [:mstange] 2012-04-13 17:09:18 PDT
Looks like some of the original code is actually from Camino (bug 325873).
Comment 25 Hanno Schlichting [:hannosch] 2012-07-25 15:00:05 PDT
As a reference, while I was researching this: Only Firefox stable + beta are automatically opted-in to quarantine via their bundle id (/System/Library/CoreServices/CoreTypes.bundle/Contents/Resources/Exceptions.plist). Aurora and Nightly each have different bundle ids and currently don't mark any files. Safari and Chrome have the LSFileQuarantineEnabled entry in their Info.plist but no Firefox build has.

With OS X 10.8 there's a new feature called Gatekeeper. But the Gatekeeper checks are only called on files having the com.apple.quarantine flag being set. So any software downloaded with Firefox Aurora and Nightly currently prevents Gatekeeper from doing its job.
Comment 26 Steven Michaud [:smichaud] (Retired) 2012-07-30 14:21:24 PDT
(In reply to comment #25)

Hanno, your comment isn't really relevant to this bug.

> the Gatekeeper checks are only called on files having the
> com.apple.quarantine flag being set

This is an Apple bug ... or a design flaw.

> Only Firefox stable + beta are automatically opted-in to quarantine
> via their bundle id
> (/System/Library/CoreServices/CoreTypes.bundle/Contents/Resources/Exceptions.plist). Aurora
> and Nightly each have different bundle ids and currently don't mark
> any files. Safari and Chrome have the LSFileQuarantineEnabled entry
> in their Info.plist but no Firefox build has.

This is indeed a problem with Firefox, but (as best I can tell) no bug
has ever been opened on it (though bug 401744, bug 401748 and bug
400227 all touch on aspects of it).

Please open a bug on this yourself.  Categorize it the same way as bug
401748, and call it something like "Firefox nightlies need to opt in
to Apple's quarantine system".
Comment 27 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-08-31 16:12:48 PDT
Comment on attachment 613200 [details] [diff] [review]
Patch for this bug and for 401748.

There's already some feedback to be addressed re: code location, but can someone provide feedback on the code itself?
Comment 28 Steven Michaud [:smichaud] (Retired) 2012-10-08 16:20:09 PDT
Comment on attachment 613200 [details] [diff] [review]
Patch for this bug and for 401748.

I just tested this (an updated version) in current code, and basically it works fine.  But not in distros whose bundle id isn't "org.mozilla.firefox" (i.e. not in nightlies).  Which means that the patch is incomplete by itself, and that we'll also need to add the following key to Info.plist:

<key>LSFileQuarantineEnabled</key>
<true/>

I tested downloading the current version of Firefox from http://www.mozilla.org/.  I used 'xattr -l' to see what "extended attributes" were added to the downloaded dmg file itself, and to the copy of Firefox dragged out from it.

The kMDItemWhereFroms stuff (included in the com.apple.metadata extended attribute) always gets added to downloaded files (e.g. the FF 15.0.1 dmg file), regardless of whether or not the Info.plist file of the patched FF distro has the LSFileQuarantineEnabled key described above.  However it doesn't also get added to the copy of an app dragged from a downloaded dmg file.

No quarantine stuff at all gets added, either to the downloaded dmg file or to the app dragged from it, if the patched FF distro's Info.plist file doesn't have the LSFileQuarantineEnabled key.

But if the patched FF distro's Info.plist file has this key, the downloaded dmg file and the app dragged out from it have a com.apple.quarantine extended attribute that includes the standard quarantine information plus the information added by the call to AddQuarantineMetadataToFile().

The com.apple.metadata extended attribute, to which data is added by the call to AddOriginMetadataToFile(), doesn't seem to contribute to the information stored in the com.apple.quarantine extended attribute.  Commenting out the call to AddOriginMetadataToFile() makes no difference to its contents.

Finally (and best of all), if an app has a com.apple.quarantine extended attribute with the additional data added by the patch's call to AddQuarantineMetadataToFile(), you see additional information in the quarantine dialog displayed by the OS the first time you try to run it:  There's a Show Web Page button that will take you to the page from which its dmg file was downloaded.
Comment 29 Steven Michaud [:smichaud] (Retired) 2012-10-08 16:22:00 PDT
Created attachment 669327 [details] [diff] [review]
Patch for this bug and bug 401748, updated to current trunk
Comment 30 Jeffrey Carpenter 2015-05-25 20:31:22 PDT
What is the hold up on this? Would love to see this feature get added!
Comment 31 Roger Wagner 2015-05-25 20:38:42 PDT
I would love to see this done.  Many schools and universities require students to cite sources, and for Creative Commons attributions are required. Google Docs has provisions for citations, and some software automatically grabs the source url for images and text dragged from a browser window into a document.  However, that only works with Safari on the Mac, not FireFox.

It's been 8 years since this feature was requested. I hope someday it can be accomplished.
Comment 32 :Gavin Sharp [email: gavin@gavinsharp.com] 2015-07-03 22:48:58 PDT
Assuming the needinfo is just "Can we get this moving again", perhaps dolske can help find an owner/reviewer for this.
Comment 33 Justin Dolske [:Dolske] 2015-07-20 17:47:40 PDT
CC Paolo in case he knows anyone interested in working on this.

I don't know of a better person for feedback than mstange -- we don't have many Mac hackers.
Comment 34 Steven Michaud [:smichaud] (Retired) 2015-07-21 09:54:33 PDT
OK then, I'll ask Markus :-)

I doubt I'll have time to get to this before I retire later this year.
Comment 35 Markus Stange [:mstange] 2015-07-21 10:26:26 PDT
Comment on attachment 669327 [details] [diff] [review]
Patch for this bug and bug 401748, updated to current trunk

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

I'm sorry for having dropped this completely.

This mostly looks good. There's some end-of-line whitespace scattered around this patch that should be removed before landing.

::: toolkit/components/downloads/nsDownloadManager.cpp
@@ +42,5 @@
>  #endif
>  
>  #ifdef XP_MACOSX
>  #include <CoreFoundation/CoreFoundation.h>
> +#include "../../../ipc/chromium/src/chrome/common/file_metadata_mac.h"

Can we instead add the file to the list of exported headers in that directory's moz.build, and have a shorter include path here?

@@ +2363,5 @@
> +            rv = refUri->GetSpec(referrer);
> +            NS_ENSURE_SUCCESS(rv, rv);
> +        }
> +      
> +        // TODO: Isn't there a better ways to get a const char* from a unsigned short* ???

NS_ConvertUTF16toUTF8(path).get() is the right way to do this, as far as I know.

@@ +2365,5 @@
> +        }
> +      
> +        // TODO: Isn't there a better ways to get a const char* from a unsigned short* ???
> +        file_metadata::AddOriginMetadataToFile(
> +            (const char*) NS_ConvertUTF16toUTF8(path).get(), 

These .get()s should already be returning const char*s, I'm surprised these casts are necessary.
Comment 36 Markus Stange [:mstange] 2015-07-21 10:28:13 PDT
Comment on attachment 669327 [details] [diff] [review]
Patch for this bug and bug 401748, updated to current trunk

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

::: ipc/chromium/src/chrome/common/file_metadata_mac.mm
@@ +166,5 @@
> +// LSFileQuarantineEnabled entry in our Info.plist, but it knows relatively
> +// little about the files. We add more information about the download to
> +// improve the UI shown by the OS when the users tries to open the file.
> +//
> +// Firefox's does _not_ have an entry for LSFileQuarantineEnabled

Should we just add this entry then, and remove this comment?
Comment 37 Markus Stange [:mstange] 2015-07-21 10:31:27 PDT
Comment on attachment 613200 [details] [diff] [review]
Patch for this bug and for 401748.

I don't really know what was the holdup here; I think smichaud waited for me to comment and I waited for him...
As far as I can tell, once my review comments have been addressed, we can just land this patch. Maybe we should get another review for dropping the new file into the chromium directory, and for the nsDownloadManager.cpp change itself.
Is anybody willing to finish the patch or should I just do it myself?
Comment 38 Steven Michaud [:smichaud] (Retired) 2015-07-21 10:39:55 PDT
>> +// Firefox's does _not_ have an entry for LSFileQuarantineEnabled
>
> Should we just add this entry then, and remove this comment?

Sounds good to me.

> Is anybody willing to finish the patch or should I just do it myself?

Also sounds good to me.  Thanks in advance!
Comment 39 Thomas Christinck 2016-01-04 01:10:46 PST
Again holdup? 2016 is year #10 for this topic ;)

Thank you very much for continuing on this.
Comment 40 Erik Elmgren 2016-03-01 05:45:20 PST
Is there some way I can help fix this bug/missing feature? Our application uses this information and a part of it only works correctly if the user downloads with Safari or Chrome.
Comment 41 juandesant 2016-03-01 07:25:44 PST
Shouldn't this bug be filed instead against the Download Manager, as per the description of the File Handling and Download Manager components? In any case, it looks like a simple matter.
Comment 42 Botond Ballo [:botond] 2016-03-01 11:32:01 PST
(In reply to Erik Elmgren from comment #40)
> Is there some way I can help fix this bug/missing feature?

Based on this comment:

(In reply to Markus Stange [:mstange] from comment #37)
> As far as I can tell, once my review comments have been addressed, we can
> just land this patch. Maybe we should get another review for dropping the
> new file into the chromium directory, and for the nsDownloadManager.cpp
> change itself.

it sounds like you could help by addressing Markus' review comments (I assume he means the one in comment 35 and comment 36), and uploading an updated patch.

(I'd needinfo Markus to confirm this, but he's already needinfo'd :)).
Comment 43 pti 2016-05-14 03:33:22 PDT
Can this proceed? This is very useful for automagically processing/sorting files.
Comment 44 William Hsu [:whsu] 2016-08-01 03:41:39 PDT
This feature isn't in our current planning.
Put it on product backlog. Thanks!
Comment 45 William Hsu [:whsu] 2016-08-19 02:26:51 PDT
Remove NI since it has been put in product backlog.
Comment 46 Josh Aas 2016-08-20 00:51:38 PDT
Doesn't look like mstange is looking at this. No promises, but I might be able to take care of it in the next week or two. Assigning to me as a reminder.
Comment 47 Josh Aas 2016-08-20 21:42:32 PDT
Created attachment 8783261 [details] [diff] [review]
Josh Fix v0.9

If we're going to fix this I don't think we should take the Chromium code as the previous patches do. It's not organized the way we'd want, it uses deprecated APIs, and takes unnecessary and confusing detours into Objective-C. We can do better.

I had some time to hack on a plane today, came up with this patch. Unfortunately I ran out of time to test it and make sure it works. I just know it compiles.

I was looking at the metadata on files downloaded by Safari 9.1.2 on OS X 10.11 and it doesn't appear to write the origin and referrer URLs like Chrome does. Maybe they stopped? Maybe it's not important to implement this now that Safari doesn't do it any more?
Comment 48 Thomas Christinck 2016-08-21 10:32:24 PDT
Sure? Safari still dies it!

Safari 9.1.2 on OSX 10.11.6 does save the source with every Download.
And its reported here 10 Years ago.
And I need it.
Please ;)

PS.: 
It's the reason I cannot use Firefox most time. And most of my friends either. We would like to use Firefox with this litte change.
Comment 49 Roger Wagner 2016-08-21 12:18:11 PDT
It's still important to do, because if you've ever dragged a photo to a folder and then later tried to remember where it came from, this is the only way to do it.

Also, it is a required part of online research now in U.S. K-12 schools (not just higher ed) to cite where media comes from.

I reported this difference/bug years ago because of the importance of this functionality. I hope someday it will be implemented.
Comment 50 Josh Aas 2016-08-25 22:20:25 PDT
Created attachment 8785155 [details] [diff] [review]
Josh fix v1.0

Had a bit more time tonight, this patch seems to actually work. Turns out we use a js-based download module now, moved the integration code over there.
Comment 51 Markus Stange [:mstange] 2016-08-29 10:22:58 PDT
Comment on attachment 8785155 [details] [diff] [review]
Josh fix v1.0

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

Thank you so much for picking this up.

This looks good to me. I haven't tested it; would you like me to?

We should also get a review from somebody who knows the downloads code, if only for the addition of the referrer argument.
Comment 52 :Paolo Amadini 2016-08-30 07:36:10 PDT
Comment on attachment 8785155 [details] [diff] [review]
Josh fix v1.0

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

Does this metadata actually cause the file to be quarantined and display an additional warning before it can be opened? We do something similar on Windows, if this is the case it should probably be documented as a new feature.

::: toolkit/components/jsdownloads/public/mozIDownloadPlatform.idl
@@ +6,5 @@
>  
>  interface nsIURI;
>  interface nsIFile;
>  
> +[scriptable, uuid(E9DB5998-7B8F-4047-A724-C909464D9655)]

There is no need to update interface UUIDs anymore, anyways we generally use lowercase.

@@ +40,1 @@
>                      in ACString aContentType, in boolean aIsPrivate);

From an MXR search it seems that no add-ons are calling or implementing this, so it's fine to add the new argument in the middle.
Comment 53 Josh Aas 2016-09-02 21:36:04 PDT
(In reply to Thomas Christinck from comment #48)
> Sure? Safari still dies it!
> 
> Safari 9.1.2 on OSX 10.11.6 does save the source with every Download.

I just tested again on OS X 10.11.6 and Safari 9.1.3 does not do it. I downloaded the same '*.tar.gz' file from both Chrome and Safari and in the File Info -> More Info section the Chrome download shows URLs, the Safari download does not. Are you looking elsewhere?
Comment 54 Thomas Christinck 2016-09-03 00:16:27 PDT
> I just tested again on OS X 10.11.6 and Safari 9.1.3 does not do it. I
> downloaded the same '*.tar.gz' file from both Chrome and Safari and in the
> File Info -> More Info section the Chrome download shows URLs, the Safari
> download does not. Are you looking elsewhere?

For me Safari does it: 
Just downloaded a *.tar.gz file (just in case the filetype matters) with Safari 9.1.3 on OSX 10.11.6. 
Its a german Version, so there is in Finder/Information/Weitere Informationen the text:
"Quelle: https://nagios-plugins.org/download/nagios-plugins-1.3.1.tar.gz, https://nagios-plugins.org/download/"
Comment 55 Nomis101 2016-09-03 00:39:08 PDT
(In reply to Thomas Christinck from comment #54)
> > I just tested again on OS X 10.11.6 and Safari 9.1.3 does not do it. I
> > downloaded the same '*.tar.gz' file from both Chrome and Safari and in the
> > File Info -> More Info section the Chrome download shows URLs, the Safari
> > download does not. Are you looking elsewhere?
> 
> For me Safari does it: 
> Just downloaded a *.tar.gz file (just in case the filetype matters) with
> Safari 9.1.3 on OSX 10.11.6. 
> Its a german Version, so there is in Finder/Information/Weitere
> Informationen the text:
> "Quelle: https://nagios-plugins.org/download/nagios-plugins-1.3.1.tar.gz,
> https://nagios-plugins.org/download/"

I can confirm this. Just downloaded the ffmpeg source in Safari 10.0, and in the Finder More Info section of the file it tells me "Quelle: http://ffmpeg.org/releases/ffmpeg-3.1.3.tar.bz2".
Interestingly Safari is not doing this anymore for e.g. pictures, like it did in the past.
Comment 56 Thomas Christinck 2016-09-04 00:43:29 PDT
Hmmm, for me Safari 9.1.3 on OSX 10.11.6 does.
Just tried a Gravatar JPG and a PNG from mozilla.org page: both have their source save in "more info".


> Interestingly Safari is not doing this anymore for e.g. pictures, like it
> did in the past.
Comment 57 Josh Aas 2016-09-12 16:07:07 PDT
Created attachment 8790486 [details] [diff] [review]
Josh fix v1.1

Updated patch and a clean try run:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d67b3bff3a9

Found an xpcshell test failure on my first run, this updated patch fixes it.
Comment 58 Pulsebot 2016-09-13 00:26:06 PDT
Pushed by jaas@kflag.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dac96d576734
Add origin and referrer URL metadata to downloaded files on OS X. r=mstange,pamadini
Comment 59 Tom Schuster [:evilpie] 2016-09-13 03:42:03 PDT
Comment on attachment 8790486 [details] [diff] [review]
Josh fix v1.1

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

::: toolkit/components/jsdownloads/src/DownloadIntegration.jsm
@@ +680,5 @@
>      }
>  
> +    let aReferrer = null;
> +    if (aDownload.source.referrer) {
> +      aReferrer: NetUtil.newURI(aDownload.source.referrer);

This should be =, aReferrer is always null.
Comment 60 Josh Aas 2016-09-13 11:19:57 PDT
My JS is admittedly rusty. I copied that code verbatim though from earlier in the file, in the function 'shouldBlockForReputationCheck'. Perhaps both need to be fixed?
Comment 61 Tom Schuster [:evilpie] 2016-09-13 12:17:12 PDT
Yes both need to be fixed.
Comment 62 Ryan VanderMeulen [:RyanVM] 2016-09-13 17:48:46 PDT
https://hg.mozilla.org/mozilla-central/rev/dac96d576734
Comment 63 Josh Aas 2016-09-13 21:30:31 PDT
Reopening for followup fix. Tom is right, the referrer assignment doesn't work and needs to be fixed in both places.
Comment 64 Josh Aas 2016-09-13 21:33:49 PDT
Created attachment 8791055 [details] [diff] [review]
Followup Fix v1.0
Comment 65 Jean-Yves Avenard [:jya] 2016-09-14 00:34:22 PDT
this breaks compilation on mac.
you need:
diff --git a/toolkit/components/jsdownloads/src/DownloadPlatform.cpp b/toolkit/components/jsdownloads/src/DownloadPlatform.cpp
index fad8185..9f08ddf 100644
--- a/toolkit/components/jsdownloads/src/DownloadPlatform.cpp
+++ b/toolkit/components/jsdownloads/src/DownloadPlatform.cpp
@@ -24,7 +24,7 @@
 
 #ifdef XP_MACOSX
 #include <CoreFoundation/CoreFoundation.h>
-#include "../../../../../xpcom/io/CocoaFileUtils.h"
+#include "../../../../xpcom/io/CocoaFileUtils.h"
 #endif
 
 #ifdef MOZ_WIDGET_ANDROID
Comment 66 :Gijs Kruitbosch 2016-09-14 06:29:46 PDT
Comment on attachment 8791055 [details] [diff] [review]
Followup Fix v1.0

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

Stealing, r=me.
Comment 67 Pulsebot 2016-09-15 06:00:00 PDT
Pushed by jaas@kflag.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c28b406521d8
Follow-up fix for referrer assignments. r=gijskruitbosch
Comment 68 Wes Kocher (:KWierso) 2016-09-15 16:31:51 PDT
https://hg.mozilla.org/mozilla-central/rev/c28b406521d8

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