Fix an attachment duplication bug and move to top level for saving #18
Conversation
QMF is not exposing which filename it is using for a saved attachment since this name does not depend only on the part information. To know if an attachment has been saved, don't try to guess the QMF filename, but save it as a customField.
dcaliste
changed the title
|
I've changed the last commit to save directly in ~/Downloads. I chose not to keep the |
|
One more thing I started pondering is what should happen if user downloads a file, then modifies it, and then opens it again from the email. Should it open the modified file or the content that was gotten with the email? I'm thinking latter could make more sense. Storing and comparing the assumed file size could be a cheap check, md5 checksum more complicated but robust. Though the current way of saving to deeper directory hierarchy doesn't either ensure opening the original content so this detail shouldn't block merging. Can be left as future work, possibly now a TODO comment or something. |
|
It's interesting that you raised this concern. I actually first implemented the MR with a check on the md5 sum for the reasons you provided. I imagine a case where you receive a I'll add it again and check that it's working. Should be ready tomorrow or on Tuesday. |
Since there is a possibility to uniquely link attachments and file locations, use now the top level location to save attachments.
|
I've amended the second commit to add the md5 check, ensuring that we actually open the file attached to the email and not a file with the same name. |
There was a problem hiding this comment.
Seems to work but now testing same file name from different emails the qmf mechanism of forming name as "." looks strange. And didn't even have to create test emails, when I already had two boardingPass.pdf from trip to fosdem, latter one now as PzJ27.boardingPass.pdf on my download folder. Yugh.
Could be nicer having some more conventional file(1).pdf type of naming, or even just the random chars after the base name, but guess this is already less bad than hiding the attachments under the tree. Different names would need adjustments to messagingframework. Though unfortunate that the output format is defined by the documentation.
Could be also an option to consider should we give user the possibility to override the name when saving, but that again would require UI changes.
All in all this PR should be good enough to go in.
|
Thank you for merging this. Yes, the naming convention to avoid file clash in QMF is a bit "unusual".
Do you mean, the doc string of |
Yes. Indeed behavior could be changed. Maybe even leaving the exact format out. |
@pvuorela, I tried to fix an issue where attachments become duplicated on disk when opened from the email application. It's a bit of a corner case, but anyway :
image/png, but without a proper extension (like no extension at all for instance).This is due to the fact that the EmailAgent is looking for a file without the extension, don't find it and ask QMF to save it again. While QMF is adding the extension, and thus finds the file and save it again with a different name.
To fix this, instead of trying to get the filename out of QMF, I prefered to go along the way you proposed once : associate the saved filename to the mail and reuse this information to know if the file has been saved already or not.
QMailMessageMetaData::customField()was a good candidate to save such data. This is the purpose of the first commit.It's quite clean in my opinion, expect when saving the custom field. Doing so make any QMailStore client to react on message updated signal. And the attachment list is thus resetting itself. Not a big deal, except that it is happening during a JS call within a delegate and the current roles are then unavailable in the JS function since the model reset itself. That's why, I delay the store update with a QTimer::singleShot() call.
If you agree with this first commit, then it becomes possible not to save anymore in this complicated tree structure based on account ids, attachment QMF locations… One can easily save in proper XDG directories, in first levels of such directories. QMF will take care of deduplicating, and the custom field information will give the possibility to have the information back when knowing an attachment location. This is the purpose of the second commit.