Closed Bug 301367 Opened 17 years ago Closed 16 years ago
Tag Files with Source URL
This enhancement tags saved content with the URI that originated it. Complete files identify the URL used to fetch them. Fragments such as text & URLs dragged from pages identify the URL that contained them. URLs that identify the source as chrome:// are supressed (e.g. URLs dragged from the address bar). Files in four areas are affected: * nsExternalHelperAppService.cpp - Downloads * nsWebBrowserPersist.cpp - Save as * nsDragService.cpp - Drag & Drop * nsLocalFileOS2.cpp - .SUBJECT extended attribute handling While implementing this feature, I also moved .TYPE EA handling from nsDragService to nsLocalFileOS2 and revised nsDragService's file-creation routines to better comply with standard practises. This patch has been tested with Firefox, Thunderbird, and Seamonkey. In most cases, the URLs generated by Thunderbird & Messenger are mailbox:// URIs and aren't very useful. If I can work around build issues, I plan to convert them to mailto: URIs in a future patch.
Is this "tag" an equivalent to the Win32 Bug 267369 comment 1?
(In reply to comment #2) > Is this "tag" an equivalent to the Win32 Bug 267369 comment 1? Very similar.
(In reply to comment #2) > Is this "tag" an equivalent to the Win32 Bug 267369 comment 1? Probably - I'm not familiar enough with those filesystems to comment knowledgably. All of OS/2's native filesystems (including FAT) are capable of associating arbitrary data with a file. Unlike the resource fork of a Mac file, the data (known as an "extended attribute") is linked to the file at the directory level, not embedded in the file itself. Its disk space is allocated independently & is not transferred when a file is copied to a filesystem that doesn't support EAs (e.g. CDFS). FYI... this is simply a reimplementation of a feature Mike Kaply added to the OS/2 port of Netscape 2.02.
Thanks for the answers, that is what I wanted to know. Yes, for Windows on NTFS you can do the same with Alternate Data Streams as meta info for files (the data are not included in the file itself, only associated). You will lost this info if you copy the file to a non-NTFS filesystem. WinXP SP2 saves for example the zone information for downloaded files in such ADS.
Rich, it is interesting that other platforms want this as well. Maybe we should figure out a way to do it in nsLocalFile so other platforms get it as well?
(In reply to comment #6) > Maybe we should figure out a way to do it in nsLocalFile so other platforms > get it as well? nsILocalFile is a frozen interface, so any new method has to be added to the platform's subclass. Plus, only the tiniest fraction of all localfile objects will ever use this facility. Adding anything to localfile beyond a new method to support it would be a major waste of space. If other platforms want to implement this, it should be fairly simple: - add a "SetFileSource()" method to their nsLocalFileXXX - add an #ifdef to my code to QI the generic file to their platform's flavor - use the rest of my (very minimal) code as-is.
Comment on attachment 189829 [details] [diff] [review] patch to tag files with source url r=mkaply
Comment on attachment 189829 [details] [diff] [review] patch to tag files with source url Whenever you change an interface definition, you MUST change the corresponding "uuid" property of the interface. Otherwise, existing XPCOM components may crash when they attempt to use the old interface. By changing the uuid, you make it so that QueryInterface can be used properly to test for the existance of a particular version of the interface. Also, a style nit: "SaveFileSource( mCurrentBaseURI, aFile);" why the extra whitespace? is that how the rest of the source file is coded? if not, then please change your style to match that of the surrounding code, thx!
Attachment #189829 - Flags: superreview?(darin) → superreview-
FYI... I'm coordinating my efforts with those working on Bug #90918. When that bug's status is clearer, I'll submit another patch containing either the entire revision or just the OS/2-specific parts. Meanwhile, OS/2 builders can continue to use this patch if they want its features.
*** Bug 272001 has been marked as a duplicate of this bug. ***
This is an improved version of the previous patch. It only supports OS/2 and is intended primarily for those who create private OS/2 builds.
Attachment #189829 - Attachment is obsolete: true
Comment on attachment 204417 [details] [diff] [review] revised patch to tag files with source url As the cross-platform coordination in bug 90918 has failed I think it makes sense to try the OS/2 only approach again, hence I ask for reviews again. This patch attends to darin's comments about the first version.
It's Rich's patch so he should be assignee.
Assignee: mozilla → dragtext
No longer depends on: 90918
Comment on attachment 204417 [details] [diff] [review] revised patch to tag files with source url I don't really like the ifdef'ing in cross-platform code, but ok. sr=darin assuming mkaply is okay with the stuff inside the ifdefs ;-)
Attachment #204417 - Flags: superreview?(darin) → superreview+
Adding Mike to CC list, review reminder.
Comment on attachment 204417 [details] [diff] [review] revised patch to tag files with source url As this is OS/2 only I think I am within my boundaries to "steal" this review from Mike. This patch is by now well tested in my unofficial builds and I went through the code again and am happy with it. Rich, if you haven't had great ideas how to improve it since you made the original patch I am going to check it in soon.
Attachment #204417 - Flags: review?(mozilla) → review+
Patch checked into trunk.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment on attachment 204417 [details] [diff] [review] revised patch to tag files with source url Users really want this OS/2 only feature also on branch. While it changes an (OS/2 only) IDL file, it only adds functions to it, so it doesn't break the API freeze... (I could add that I have been shipping with this patch in my unofficial 1.8.x builds and it never caused a problem.)
Attachment #204417 - Flags: approval22.214.171.124?
Attachment #204417 - Flags: approval126.96.36.199? → approval188.8.131.52+
mkaply, what's your take on this? Rich, Andy, any other opinions?
(In reply to comment #21) > but can break any C++ code that calculates the sizes of vtables. What is the likelihood that such code exists? To my knowledge, no one has encountered such a problem due to this patch. (In reply to comment #22) > any other opinions? Peter, since you'll be the one hearing about it and fixing it if there is a problem, I'd say check it in.
I say we go for it. I'd love to get as many of these "Peter W only" patches into the branch as possible.
OK, great, so we all agree. :-) Landed the patch on 1.8 branch now.
(In reply to comment #23) > (In reply to comment #21) > > but can break any C++ code that calculates the sizes of vtables. > > What is the likelihood that such code exists? To my knowledge, no one has > encountered such a problem due to this patch. That's not the point. Were this a file subject to the 1.8-branch API freeze--and it is not IMO--then this would not be allowed, period. We're not allowed to guess whether it is or isn't likely someone's using a particular interface. Some recent patches that alter a previously altered interface have even had to create yet another subclass in the form of _MOZILLA_1_8_BRANCH2 -- it's ugly, but it's the rule. It's easier to follow the rule than to waste time proving that your interface doesn't need to. It's far better and easier to follow the rule than to break stuff and have to re-release and lose user's trust when their extensions break. OS/2 releases live in their own universe, no problems. comment 21 was merely intended clarify the "no API change" rule should some future patch need to change a shared interface.
Verified as fixed with the branch nightly Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:184.108.40.206pre) Gecko/20071003 BonEcho/220.127.116.11pre
You need to log in before you can comment on or make changes to this bug.