to Content-Type, add a size parameter if a feed publishes length and it's nonzero; add a parameter indicating this is an enclosure (to differentiate from detached email attachments).

Comment on attachment 522721 [details] [diff] [review] patch Overall this looks good, but I'm unfamiliar with x-moz-enclosure and can't find any references to it in the codebase, so that piece should get review by another reviewer.

Sorry for the delay in getting to this. I can't see any references to x-moz-enclosure either - can you explain why that is being added? (and maybe some references to where it is used?). Also, what will adding the size parameter do for the user?

well, it would be a new value. the idea is to know that a feed attachment is on (in usage not strictly technically) a remote network location and requires download. there is X-Mozilla-External-Attachment-URL but that, in usage, means a detached mail attachment on the local filesystem. if there were a non folder based way of knowing a message is a feed (bug 522645?), this probably isn't necessary, as the existing above header could be used in conjunction with knowing it's a feed. as for size parameter, it will tell the user the size.. there shouldn't be a distinction here with what is done for email attachments; if the publisher sends a size it should be displayed.

(In reply to comment #4) > if there were a non folder based way of knowing a message is a feed (bug > 522645?), this probably isn't necessary, as the existing above header could be > used in conjunction with knowing it's a feed. Does this code (which the content policy uses) help? http://hg.mozilla.org/comm-central/annotate/4af027ba1614/mailnews/base/util/nsMsgUtils.cpp#l1013

no, the server will not QI to nsIRssIncomingServer unless it's physically in a feed folder. you can easily see the issue with content policy as a feed message copied to a local folder or any other will show the blocked remote content notify. the correct way is to set a feed property on the message after it's added, per the above bug, which is simple but depends on bug 647699. i'll change the patch to forego the enclosure header part as it's hacky on second thought.

Comment on attachment 529615 [details] [diff] [review] just size. >- 'Content-Type: ' + this.mContentType + '; name="' + this.mFileName + '"\n' + >+ 'Content-Type: ' + this.mContentType + >+ '; name="' + this.mFileName + >+ (this.mLength ? '; size=' + this.mLength : '') + '\n' + I think you're missing closing the double-quotes on the filename. Maybe insert them into both halfs of the this.mLength ? : statement? Also, do you have a feed I could test this on?

here's one with an enclosure, currently. http://www.wired.com/magazine/feed/

Comment on attachment 529990 [details] [diff] [review] paying attention this time.. Ok, thanks, this looks better.

Checked in: http://hg.mozilla.org/comm-central/rev/053cdd2ba13c