Make SMTP notify send images as attachments if html is disabled

[aptalca]

Breaking change

SMTP integration will send images as attachments to a plain text email instead of html in-line when the html field is not set. Previous behavior was to send all images as html in-line even when the html field was not set. To continue sending images as in-line, please set the optional html field and include the images as <img src="cid:image_name.ext"> within the html block as described in the docs: https://www.home-assistant.io/integrations/smtp/#usage

Proposed change

Current behavior:

1. Plain text message (no html or images defined), is sent as plain text
2. Plain text message with images defined (no html defined) is sent as html with images in-line
3. Messages with html defined are sent as html with images in-line (if any)

Proposed behavior:

1. Plain text message (no html or images defined), is sent as plain text (No change)
2. Plain text message with images defined (no html defined) is sent as plain text with images as separate attachments
3. Messages with html defined are sent as html with images in-line (if any) (No change)

Only proposed change is to the second type listed above. Currently, if the user drafts a plain text message, but adds an image, it is force converted to html and sent that way. That is not the user's intention as they could have drafted an html message if that were the intention. This change respects the user's choice with regards to plain text vs html.

I personally consider this a bugfix, because currently a plain text email is force converted to html. Most notable issue it causes is when we try to use an email-sms gateway to send a text message to a cell phone (ie. email to 5555555555@msg.fi.google.com). Without an image, email is sent plain text and is successfully delivered to the mobile device as a text/sms message. With an image, the mobile device displays the message body as raw html, and the inline images are entirely missing.

With the proposed changes, a plain text email with images get delivered to the mobile device as a proper plain text mms, along with the pictures as mms attachments.

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

• Link to documentation pull request: Update info on image fields for SMTP home-assistant.io#27524

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• I have followed the perfect PR recommendations
• The code has been formatted using Black (black --fast homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

Hi @aptalca

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

[davet2001]

@aptalca I think it would be helpful to get consensus on whether your proposal is preferred behaviour.

It looks like this feature was added way back in #2738 by @partofthething. So might be worth reaching out to see if there was another reason for the original choice.

I expect many users don't realise that inline images mean an email is html, so this change might force them to

1. Edit YAML
2. learn basic html
   ...in order to restore previous behaviour.

If that's the case an option flag for inline images might be better?

[aptalca]

Thanks @davet2001

It looks like when the image attachment functionality was PRed in, there was no support for html emails. It was all plain text. So it seems the decision of sending them as inline html was just a personal choice by the dev. There was no plain text vs html choice given to the user.

Later, when html was added as an option to the integration, the image addition forcing plain text to html probably should have been changed as it became problematic. Because at that point plain text vs html became a deliberate choice given to the users, and the forced html behavior goes against that choice.

As far as consensus goes, not sure how to get that. This integration does not have a maintainer and apart from a few forum posts asking how to add images as attachments rather than inline, that end with no, you can't do that, there hasn't been any discussion that I can see anywhere. I feel like this PR is just going to sit for a long time. Please let me know if you have any ideas. Should I try discord?

[davet2001]

Discord or user forums could help.

https://analytics.home-assistant.io/integrations/ gives the approx number of people using this integration (currently >=3536).

I think there's quite a high chance that some of them would not want the behaviour to change.

If it did change, I suggest you would need to provide some instructions on how to restore old behaviour. For example, if a user does want inline images, what should they change their YAML to?

• To avoid this being a user facing problem you could make it selectable and make HTML the default for existing users.

But given that new YAML configuration options are no longer allowed, the first step might be to convert it to be set up via the UI, then make the change. This is clearly more work and probably not what you were hoping for.

I genuinely don't know what the use cases are. It could be that no one wants html. But my experience is that if you change something that affects users who were already happy, they will expect immediate guidance on how to make it work like it used to.

[partofthething]

Hi. Thanks for reaching out. I am still using this, and am totally fine with this change. Seems like an improvement to me, and I'll have no problem adjusting my config as necessary.

[aptalca]

I genuinely don't know what the use cases are. It could be that no one wants html. But my experience is that if you change something that affects users who were already happy, they will expect immediate guidance on how to make it work like it used to.

That's a good point. I'll try and come up with a way to maintain the current behavior by default and add a flag for selecting attachments vs inline.

[github-actions]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

[aptalca]

I gave this more thought and I'm personally not in favor of keeping the current behavior because it would be very confusing. The email should be html when the user selects and inputs html. If not, it should be plain text. That's my opinion.

In any case, that's not a coding decision, but a behavior decision so it's not really up to me. HA team will have to decide since there is no maintainer for this core integration.

Thanks

[davet2001]

Ok, I suggest adding a description of what the user has to do if they want the images to stay inline. This can be added to the PR description. That can then be included in the breaking changes in the release notes.

I think an example YAML snippet is needed with HTML, ready for copy/paste.

[aptalca]

thanks @davet2001
Updated the PR description to add the breaking change, along with a link to the docs page that includes a detailed example.

Also improved attachment handling where it could add double headers if the file's MIME type could not be determined as image.

[davet2001]

@aptalca The CI is failing, please can you check and run it locally?

[aptalca]

@aptalca The CI is failing, please can you check and run it locally?

Fixed formatting. Thanks for the heads up.

[davet2001]

This looks good to me, the change is as described and the updated documentation is accurate.

Requested 2nd opinion to make sure others are happy with the user impact.

[emontnemery]

The change makes perfect sense and the behavior more logical.
Thanks @aptalca 👍