Comment 74 for bug 151162

Revision history for this message
In , Philbaseless-firefox (philbaseless-firefox) wrote :

(From update of attachment 386466)
I think roc is the reviewer name to set here.
Here's my notes from what I can compare to windows version. I don't have linux to check further than that.

>+ mTempFileCreated = PR_FALSE;

don't quite follow its need, but that may be me.

>+nsresult GetAttachmentDetails(nsITransferable * aTransferable, nsIURI

maybe use 'GetDownloadDetails' like windows or similiar generic name. This can be used for messages, files, or other data besides attachments.

>+ // check if text/x-moz-url is supported.

did you mean application/x-moz-file-promise-url

>+ // The desktop or file manager expects for drags of attachments the
>+ // text/uri-list flavor set to a temporary file that contains the
>+ // attachment.
>+ // We open a stream on the mailbox:// url here and save the content
>+ // to file:///tmp/dnd_attachment/<attachmentname> and pass this url
>+ // as text/uri-list flavor.

again, attachments are only one use. 'promise-file' or 'promise-file data' for clarity, and mailbox:// could be other handler <protocol>://

>+ // build the file:///tmp/dnd_attachment URL
>+ tmpDir->Append(NS_LITERAL_STRING("dnd_attachment"));
>+ tmpDir->CreateUnique(nsIFile::DIRECTORY_TYPE, 0775);

don't know how much 'naming' needs to be added to file name for linux use or obfuscation but I assume CreateUnique would be enough. Again 'attachment' should be more generic if needed at all.

>+ mTempFileCreated = PR_TRUE;
>+ mTemporaryFiles.AppendObject(tempFile);

again mTempFileCreated is set but can't see its use. If it is necessary it may be better set after the outputstream succeeds.

repeating! I'm not a reviewer. my comments are only notes. Use at your or reviewer's discretion.