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

Open
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

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