Comments

claudiu.cristea’s picture

Category: feature » task
claudiu.cristea’s picture

Status: Active » Needs review
StatusFileSize
new15.96 KB

Here's a first patch.

This feature doesn't add any visual enhancement but it make all the process more clean and robust.

I ask everyone to test this patch. I need some bug checking before committing to CVS. Thanks!

claudiu.cristea’s picture

Status: Needs review » Fixed
StatusFileSize
new15.96 KB

Committed in #381384. Final patch attached.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

roball’s picture

Category: task » bug
Priority: Normal » Critical
Status: Closed (fixed) » Needs work

With this patch applied, mails set to the "Full HTML" Message format are no longer actually sent as HTML (when Mime mail is installed). Had to revert this patch and reinstall Views Send and it worked fine again.

scubaguy’s picture

I'm having the problem of html emails not being send as html also.

claudiu.cristea’s picture

It may be something in _views_send_prepare_mail() function, something around the $plain variable. Can somebody investigate this?

roball’s picture

Do you think you can solve this problem so that we have a working version of Views Send?

claudiu.cristea’s picture

We will fix the issue, of course. In holiday right now :)

roball’s picture

Wish you a nice holiday!

Maybe you could revert the patch from #3 until there is a working fix because with this patch being in dev that version can't be used unpatched.

roball’s picture

Just wanted to ask if there is any progress update on this critical issue - thanks.

claudiu.cristea’s picture

I cannot reproduce the error. What version of Mime Mail module do you use?

roball’s picture

Mime Mail 6.x-1.0-alpha5, together with SMTP Authentication Support 6.x-1.0-beta5.

roball’s picture

Title: Move all message processing before spooling » Full HTML Messages are garbaged

I have the same problem on another site, where I don't use the "SMTP Authentication Support" module - just Mime Mail 6.x-1.0-alpha5. After reverting the patch from #3 via

[root@me modules]# cd views_send
[root@me views_send]# wget http://drupal.org/files/issues/views_send-process-before-spool-808058-1-D6.patch
[root@me views_send]# patch -R -p0 < views_send-process-before-spool-808058-1-D6.patch

HTML mails are fine. Have you now been able to reproduce the problem?

roball’s picture

Title: Full HTML Messages are garbaged » HTML messages are sent as plain-text including (visible) HTML-tags

The same problem still exists with Mime Mail 6.x-1.0-alpha6. Please, could you care about this? I don't want to pressure, but I'm patiently waiting for this bug to be fixed since months.

delty’s picture

Bump

roball’s picture

Claudiu, so at least 3 people here are observing this problem. Could you finally find time to look into it? This is really frustrating.

anawillem’s picture

Me too! This is a great module, and I'd like to see it really ready for production soon.

roball’s picture

Unfortunately, this module seems to be dead already before even seeing any working release :-(

franz’s picture

StatusFileSize
new1.12 KB

Well, at least I made a patch to address the issue. What happens is that in mimemail's current version, there is a function for sending the mail that does everything, so we can clean up de code for sending. I just tested and it worked fine.

The only thing still not addressed is plain-only messages (there is an argument for mimemail()), because the format is not saved on the database.

franz’s picture

Status: Needs work » Needs review

Forgot to change Status, please test this!

gabrielu’s picture

This works for me.
I believe we should consider saving and retrieving $plaintext from the database and comit it.

roball’s picture

Status: Needs review » Reviewed & tested by the community

Yeah, finally a solution!! Working fine for me - thanks a lot franz!
Also seems to fix another bug that truncated the message's Sender's name at the first whitespace.
Suggestion from gabrielu seems logic. Would like to post the corresponding patch?

franz’s picture

StatusFileSize
new2.13 KB

This could be implemented simply by placing a "fake" header in the message, with a boolean value. Of course, the header should be removed before sending, or rather comply to e-mail standards (which I'm not familiar with). I tried to address both issues with a new patch:

franz’s picture

Status: Reviewed & tested by the community » Needs review

Ops, changing state

franz’s picture

I did not test this new patch, please do it if you can.

roball’s picture

Status: Needs review » Needs work

I tested the patch at #24: It always sends HTML mails as HTML even if a user has set his profile preference to receive plain text only.

franz’s picture

I was not even aware of this option. Tweaking the patch shouldn't be hard, thought... But I don't have the time for it right now...

roball’s picture

StatusFileSize
new5.62 KB

Here you can see how the "Email settings" fieldset at user/[UID]/edit look like:

franz’s picture

Could this be addressed by another issue?

Is the maintainer currently active?

franz’s picture

Status: Needs work » Needs review
StatusFileSize
new2.46 KB

That was Mimemail's option, easier than I thought. Instead of setting $plaintext as FALSE when content is set to HTML, I had to set it to NULL, so that mimemail would respect user's preferences

roball’s picture

Thanks for your work on this. However, also with your latest patch I am getting HTML mails regardless of the plain text user preference. Not really a problem for me since the big problem has been solved with the patch.

SeanBannister’s picture

Just confirming that the patch in #31 fixes the problem of HTML e-mails being sent as plain text. But I haven't tested the plain text user preference.

roball’s picture

Status: Needs review » Reviewed & tested by the community

I think we should change this status to RTBTC since the #31 patch does solve the original problem. The remaining small issue is another issue.

claudiu.cristea’s picture

Status: Reviewed & tested by the community » Needs work
+++ views_send.module	19 Nov 2010 11:36:19 -0000
@@ -601,27 +607,17 @@ function views_send_deliver($message) {
-  // Mime Mail has no separate rutines for sending and preparing.
-  // We are using pieces of code from mimemail() function to send the message.
   if (VIEWS_SEND_MIMEMAIL) {
-    $mail = array(
-      'address' => $to,
-      'subject' => mime_header_encode($message->subject),
-      'body' => $message->body,
-      'sender' => $from,
-      'headers' => $headers,
-    );
-
-    $engine = variable_get('mimemail_engine', 'mimemail') .'_mailengine';
-    if (!function_exists($engine)) {
-      return FALSE;
+    // Retrieve header for Plaintext option
+    // And remove it
+    if (isset($headers['X-Views-Send-Plaintext'])) {
+      $plaintext = TRUE;
+      unset($headers['X-Views-Send-Plaintext']);
     }
-
-    // Workaround a Mime Mail inclusion issue...
-    if ($engine == 'mimemail_mailengine') {
-      require_once(drupal_get_path('module', 'mimemail') .'/mimemail.inc');
+    else {
+      $plaintext = NULL;
     }
-    return $engine('send', $mail);
+    return mimemail($from, $to, $message->subject, $message->body, $plaintext, $headers);

Views Send does not process messages when sending. Instead it performs all processing before spooling (see http://drupal.org/node/1065840). This I consider an "added value" of the module. That's why we cannot simply call mimemail() because mimemail() is preparing again the message (and we did that when spooling). This is design Mimemail issue that I addressed here #808518: API change: Split mail preparation from sending. Till that is fixed we have to fork the Mimemail code.

+++ views_send.module	19 Nov 2010 11:36:19 -0000
@@ -340,6 +340,12 @@ function views_send_mail_action($object,
+  // If dealing with Plaintext format along with mimemail, we set a fake header ¶
+  // to be removed later
+  if ($context['views_send_message_format'] == VIEWS_SEND_FORMAT_PLAIN && VIEWS_SEND_MIMEMAIL) {
+    $headers['X-Views-Send-Plaintext'] = TRUE;
+  }
+

Clever :) But I think that we have to fix the issue before spooling in _views_send_prepare_mail(). I think there's a bug there when mail is prepared.

Powered by Dreditor.

claudiu.cristea’s picture

Status: Needs work » Needs review
StatusFileSize
new969 bytes

Please apply the attached patch against 6.x-1.x-dev and see if it fixes the issue.

roball’s picture

No, patch #36 does not work, unfortunately.

roball’s picture

Status: Needs review » Needs work
claudiu.cristea’s picture

@roball, Did not apply or is not fixing the issue? Remember, you have to apply it against the latest 6.x-1.x-dev and uset git am (see http://drupal.org/node/1054616 or better instal latest 6.x-1.x-dev and apply it manually - there's a single word to be added: "_prepare_message" instead of "_prepare"

roball’s picture

I installed the latest currently available 6.x-1.x-dev (2010-Mar-01) and manually applied the patch from #36:

[root@www views_send]# wget http://drupal.org/files/issues/views_send-808058.patch
[root@www views_send]# patch -p0 < views_send-808058.patch
File to patch: views_send.module
patching file views_send.module
Hunk #1 succeeded at 703 (offset -144 lines).

So the patch successfully applied, but it did not change anything on the described problem. Reverting back to a "views_send.module" file having the patch applied from #31 did solve the problem. Thus #36 misses something essential from #31.

claudiu.cristea’s picture

@roball, This patch works for me. Without this it behaves like in the ticket.

Please, double check all above and make sure you didn't turn on "Plaintext email only" on Mimemail settings (admin/settings/mimemail), while this patch is aware of that.

Thanks.

franz’s picture

@claudiu.cristea, not sure if I'm missing something, but patch at #36 only changes one line. I believe you mentioned another issue, along with forking mimemail. Could that be working? We need to test this single patch on a fresh install then, to isolate it.

claudiu.cristea’s picture

Yes @franz, please test in this way:

  1. Install a fresh Views Send from latest 6.x-1.x-dev
  2. Apply the patch from #36. Use git am (see http://drupal.org/node/1054616) OR apply it manually - there's a single word to be added: "_prepare_message" instead of "_prepare"
  3. Go to Mimemail Settings at admin/settings/mimemail and assure that "Plaintext email only" checkbox IS NOT checked
  4. Test with a HTML formatted message.

Messages should be delivered formatted with markup.

roball’s picture

As already mentioned, this won't work for me. But with the patch from Franz (#31) it does.

silaslin’s picture

To claudiu.cristea,
Thank you !
This patch worked perfectly~

claudiu.cristea’s picture

Status: Needs work » Reviewed & tested by the community

Works for me too. RTBC

claudiu.cristea’s picture

Status: Reviewed & tested by the community » Fixed

Commited

roball’s picture

Status: Fixed » Needs work

I am sorry to say, but - as already mentioned - this does *not* work for me although it does seem to work for you (claudiu.cristea) and silaslin. Just upgraded to the 6.x-1.x-dev (2011-Mar-16) snapshot (and of course cleared the cache) - problem went in again. Replacing the "views_send.module" file to the one from 2010-11-27, with the patch from Franz applied again solved the problem.

Franz can you please let us know your experience? Maybe you could provide a patch against the current dev which solves the problem for you?

Thanks.

claudiu.cristea’s picture

@roball, what Mimemail version do you have? Can you provide us and your Views Send (admin/settings/views_send) & Mimemail (admin/settings/mimemail) settings?

roball’s picture

Mime Mail 6.x-1.0-alpha7: Settings - see screenshot (same result when the Input format is set to "Full HTML").

Views Send 6.x-1.x-dev (2011-Mar-16): Settings - see screenshot

claudiu.cristea’s picture

Status: Needs work » Needs review
StatusFileSize
new1.68 KB

Fortunately I found that you are using SMTP as mailer for Mimemail. Indeed there is a bug in my module causing failure when using other mail engine than Mimemail (like SMTP, PHPMailer, etc.).

Please test this patch against latest from 6.x-1.x-dev

roball’s picture

Status: Needs review » Reviewed & tested by the community

Yeah - finally this works on my environment as well :-)

claudiu.cristea’s picture

Status: Reviewed & tested by the community » Fixed

Committed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.