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

[16.0][MIG] mail_embed_image module #1499

Draft
wants to merge 19 commits into
base: 16.0
Choose a base branch
from

Conversation

StephaneMangin
Copy link
Contributor

@StephaneMangin StephaneMangin commented Nov 14, 2024

Superseed #1402

@StephaneMangin StephaneMangin force-pushed the cid_content_change_and_data_embeddings branch 4 times, most recently from 47b33e5 to 022e0d3 Compare November 15, 2024 13:53
@StephaneMangin StephaneMangin force-pushed the cid_content_change_and_data_embeddings branch from 022e0d3 to 6dc1479 Compare December 12, 2024 09:24
@StephaneMangin StephaneMangin changed the title Cid content change and data embeddings [16.0] [MIG] mail_embed_image module Dec 12, 2024
@StephaneMangin StephaneMangin changed the title [16.0] [MIG] mail_embed_image module [16.0][MIG] mail_embed_image module Dec 12, 2024
Copy link
Member

@hbrunn hbrunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests are fixed in #1519

):
image_path = img.get("src")
try:
response = requests.get(image_path, timeout=10)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a test case where we have an image with a relative url, I believe this will fail here

>
<span class="o_form_label">Email Preprocessing</span>
<div class="text-muted">
Method used to embed images in HTML emails. CIDs attachment does not work with all email clients. Data SRC is more reliable.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that actually depends very much on the clients you target, with old outlooks it's the other way around. I'd propose not to make claim about what works better, and rather suggest to test with whatever are the expected mail clients

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue regarding Outlook has been tackled elsewhere. WIP
Need more testing regarding it before proposing.

@StephaneMangin StephaneMangin force-pushed the cid_content_change_and_data_embeddings branch 2 times, most recently from d77d383 to 476b0ef Compare December 19, 2024 11:10
@StephaneMangin StephaneMangin marked this pull request as draft December 19, 2024 11:10
 - CIDs are not working in some email managers (gmail and office365)
   To allow this behavior we need to change the content type to
   multipart/related and reorganize the parts (see comments in code)
 - A new option to embed images is to include the base64 content inside
   the src.
 - Then to select these options, a selected field has been added to company
@StephaneMangin StephaneMangin force-pushed the cid_content_change_and_data_embeddings branch from 476b0ef to f81e4c2 Compare December 19, 2024 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants