Stop removing the html format option if mime_mail is not enabled [#1371084]

See title.

Two reasons:

a) Right now, we need to explicitly check for each html module we support. We do that now for mimemail and html_mail, but it just one occasation, which is this setting. But there are a number of places like watchdog messages, where add something if mimemail is installed (without knowing if it's really used).

b) It makes testing harder. Right now, we'd need to work around this limitation, for example by inventing a new hook and then creating dummy test helper module which implements that hook and exposes itself as a html mail implementation.

So, I'd suggest to remove this check (and probably most of the others as well, like different watchdog messages) and instead simply display a message that if you want to send html newsletters, you need to install and configure a module like mimemail or html_mail, including links to the project page.

Any objections?

Comment	File	Size	Author
#1	remove_mimemail_checks.patch	6.63 KB	berdir

Comments

Comment #1

berdir

German

Switzerland

commented 14 December 2011 at 21:02

Status	File	Size
new	remove_mimemail_checks.patch	6.63 KB

Isn't this a nice patch?

I also removed the mail system setting completely. Mime Mail now depends on http://drupal.org/project/mailsystem, which does a better job at controlling that than we do.

Comment #2

14 December 2011 at 21:03

Status:

Needs review

» Needs work

The last submitted patch, remove_mimemail_checks.patch, failed testing.

Comment #3

miro_dietiker

Switzerland

commented 14 December 2011 at 22:02

The patch with cleaning up that stale parts is a great first step. The core mailsystem also was a great idea.

Now we have core mailsystem and contrib mailsystem module. ;-)
The contrib mailsystem tries to cover many things and originally was only a configuration module.
This led us to the idea of defining mailwire. A plain API module. Now mailsystem started to adopt our ideas of APIs and does a little everything.
Mailwire is still in very early stage and needs a lot of work. However we should have two completely separate discussions about API and UI.
The mailsystem is still inflexible and intransparent...
BTW: The introduction like a hook_mailsystem_info() would be subject of mailwire...
http://drupal.org/project/mailwire
#1179438: comparision mailwire vs mailsystem module
#1363428: Allow mailengines to define html capability

To know if HTML / Multipart mails are supported by the underlying mailsystem is something we should really know... However with e.g. mailsystem, you never know if for a specific mailkey we really use a transport that supports HTML.

Creating the multipart mail might add a lot weight to newsletter creation (slow performance) as long as you only send finally only plaintext.

My intention was to rely on this hook once it's defined as an optional resource and then switch non-HTML things off (or vice versa). However i don't want to depend on it.

=> Please commit the cleanup, generally looks fine to me, and add some followup issues / feature requests to cover the topics. Prio low.

Feature followup:
Note that (still with modern) newsletters, users need the capability to switch to plaintext mode only... Also for HTML newsletters. To show this option if a system even doesn't support HTML (or no one ever wants to create HTML mails) is really strange.

Comment #4

berdir

German

Switzerland

commented 15 December 2011 at 15:43

As discussed, we can hide user options if there is no category with html format and we can only generate the html version if the format is set to html of the related category.

Note that the patch is built on the caching issue and will only apply after that one was commited.