Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ADD] New module incoming_mail_embedded_picture_reader #329

Conversation

JonathanNEMRY
Copy link

This PR add a new module that allows to read mail having embedded picture into its body.
Take care that this is currently only possible with odoo/odoo#10435 (wait for merge)

@hbrunn
Copy link
Member

hbrunn commented Jan 13, 2016

@hbrunn hbrunn added this to the 8.0 milestone Jan 13, 2016
@hbrunn
Copy link
Member

hbrunn commented Jan 13, 2016

any reason not to do this on the client side rather than messing with an email's content?

@JonathanNEMRY
Copy link
Author

Hi @hbrunn

  • What's this missing tag 'images'? I don't see it into https://github.com/OCA/maintainer-tools/blob/master/template/module/__openerp__.py
  • any reason not to do this on the client side rather than messing with an email's content?.
    With the website you have the possiblity to add embedded picture too. But Actually what Odoo does: a attachment link. I simply re-introduce the same behavior. Keeping a cid:id from a multi-part content has no sense as it is not understandable for Odoo. Moreover, the inverse process when sending email involves the transformation of the url attachment into a correct cid:id + content-id so if you want to forward a received mail with embedded picture, this conversion is required into the body ans then we have two separate and clean process.

@hbrunn
Copy link
Member

hbrunn commented Jan 13, 2016

images: The third heading here: https://www.odoo.com/apps/upload - I personally think we could do without it, but as this test is introduced, we should adhere to it
Let's see if we're on the same page here. 'Embedded images' could be understood two ways: Either an img tag with a data uri (should be supported already) or multipart messages (https://tools.ietf.org/html/rfc2110#page-13) - you mean the latter, right?

I have huge reservations against parsing and rewriting each message's HTML. The sending part is another issue, that should be handled by the mailing wizard. The model mail.message should be a stupid container, I think.

Why doesn't it work for you to iterate over all img tags (actually, you should support everything that can be embedded according to the RFC anyways) at display time, check if the src attribute is one of the messages' attachment content-id or content-location headers (those will have to be saved on the attachment in the lines of your patch)?

As for the patch itself: I'd be pretty surprised if it was merged to upstream, and we can't accept modules that don't work with an unpatched core. Did you experiment with overriding _message_extract_payload, remove the parts you're interested in, and only pass the remaining ones to super?

@JonathanNEMRY JonathanNEMRY force-pushed the 8.0-incoming_mail_embedded_picture_reader-jne branch from 55c344e to 8f3ab07 Compare January 15, 2016 16:38
Jonathan Nemry (ACSONE) and others added 2 commits April 20, 2017 20:53
@Olivier-LAURENT Olivier-LAURENT force-pushed the 8.0-incoming_mail_embedded_picture_reader-jne branch from 8f3ab07 to c37bdd8 Compare April 20, 2017 19:03
@github-actions
Copy link

github-actions bot commented Dec 5, 2021

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Dec 5, 2021
@github-actions github-actions bot closed this Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.0 needs review question stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants