Correctly wrap inline attachments.

This switches the behaviour from:

multipart/related
  multipart/alternative
    text/plain
    text/html
  attachment (disposition: inline, image1)
  attachment (disposition: attachment, file1)
  attachment (disposition: attachment, file2)

to:

multipart/mixed
  multipart/related
    multipart/alternative
      text/plain
      text/html
    attachment (disposition: inline, image1)
  attachment (disposition: attachment, file1)
  attachment (disposition: attachment, file2)

Fixes #2686.

Thanks to clemens and eGust for reviewing.

actionmailer/lib/action_mailer/base.rb

@@ -859,6 +859,7 @@ module ActionMailer
       @_mail_was_called = true
 
       create_parts_from_responses(message, responses)
+      wrap_inline_attachments(message)
 
       # Setup content type, reapply charset and handle parts order
       message.content_type = set_content_type(message, content_type, headers[:content_type])
@@ -889,7 +890,7 @@ module ActionMailer
         when user_content_type.present?
           user_content_type
         when m.has_attachments?
-          if m.attachments.detect(&:inline?)
+          if m.attachments.all?(&:inline?)
             ["multipart", "related", params]
           else
             ["multipart", "mixed", params]
@@ -990,6 +991,27 @@ module ActionMailer
         end
       end
 
+      def wrap_inline_attachments(message)
+        # If we have both types of attachment, wrap all the inline attachments
+        # in multipart/related, but not the actual attachments
+        if message.attachments.detect(&:inline?) && message.attachments.detect { |a| !a.inline? }
+          related = Mail::Part.new
+          related.content_type = "multipart/related"
+          mixed = [ related ]
+
+          message.parts.each do |p|
+            if p.attachment? && !p.inline?
+              mixed << p
+            else
+              related.add_part(p)
+            end
+          end
+
+          message.parts.clear
+          mixed.each { |c| message.add_part(c) }
+        end
+      end
+
       def create_parts_from_responses(m, responses)
         if responses.size == 1 && !m.has_attachments?
           responses[0].each { |k, v| m[k] = v }

actionmailer/test/base_test.rb

@@ -179,6 +179,20 @@ class BaseTest < ActiveSupport::TestCase
     assert_equal("logo.png", email.parts[1].filename)
   end
 
+  test "can embed an inline attachment and other attachments" do
+    email = BaseMailer.inline_and_other_attachments
+    # Need to call #encoded to force the JIT sort on parts
+    email.encoded
+    assert_equal(2, email.parts.length)
+    assert_equal("multipart/mixed", email.mime_type)
+    assert_equal("multipart/related", email.parts[0].mime_type)
+    assert_equal("multipart/alternative", email.parts[0].parts[0].mime_type)
+    assert_equal("text/plain", email.parts[0].parts[0].parts[0].mime_type)
+    assert_equal("text/html",  email.parts[0].parts[0].parts[1].mime_type)
+    assert_equal("logo.png", email.parts[0].parts[1].filename)
+    assert_equal("certificate.pdf", email.parts[1].filename)
+  end
+
   # Defaults values
   test "uses default charset from class" do
     with_default BaseMailer, charset: "US-ASCII" do

actionmailer/test/fixtures/base_mailer/inline_and_other_attachments.html.erb

@@ -0,0 +1,5 @@
+<h1>Inline Image</h1>
+
+<%= image_tag attachments['logo.png'].url %>
+
+<p>This is an image that is inline</p>

actionmailer/test/fixtures/base_mailer/inline_and_other_attachments.text.erb

@@ -0,0 +1,4 @@
+Inline Image
+
+No image for you
+

actionmailer/test/mailers/base_mailer.rb

@@ -39,6 +39,12 @@ class BaseMailer < ActionMailer::Base
     mail
   end
 
+  def inline_and_other_attachments
+    attachments.inline["logo.png"] = "\312\213\254\232"
+    attachments["certificate.pdf"] = "This is test File content"
+    mail
+  end
+
   def attachment_with_content(hash = {})
     attachments["invoice.pdf"] = "This is test File content"
     mail(hash)