1038836 - [email/backend] Mailing list text/plain footers added (with a multipart/mixed container) to text/html parts are detected as unnamed attachments

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Andrew Sutherland [:asuth] (he/him)]

In some fallout from bug 851963 that demonstrates even I have trouble getting through my really long comments (https://bugzilla.mozilla.org/show_bug.cgi?id=851963#c5), the logic I suggested went too generic and is improperly treating a text part as an attachment because of the presence of an explicit disposition of inline.

The glodebug tree looks like this:
  1 Container (34029 bytes): multipart/mixed
    1 Container (33888 bytes): multipart/alternative
      1 Body: text/plain (948 bytes)
      2 Body: text/html (32940 bytes)
    2 Body: text/plain (141 bytes)

The footer part headers looks like this:

Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline


The good news is that things like this because much more obvious when you have test cases, and now we have a test case to add!

The simplest/most obvious change is to weaken the 'id' requirement
  if (partInfo.id) {
to only happen for non-text parts.  Like:
  // Displaying text-parts inline is not a problem for us, but we need a
  // content id for other embedded content.  (Currently only images are
  // supported, but that is enforced in a subsequent check.)
  if (partInfo.type === 'text' || partInfo.id)

It's probably worth skimming https://bugzilla.mozilla.org/show_bug.cgi?id=851963#c5 again when fixing to make sure there's not anything else obvious I missed.  And certainly if any real-world test cases can be inferred, it'd be good to add them too.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Andrew Sutherland [:asuth] from comment #0)
> The good news is that things like this because much more obvious when you
> have test cases, and now we have a test case to add!

Rather, things like this *become* much more obvious.  Brain got ahead of my fingers and their memory doesn't have ECC.

Comment 2

[Alex Wissmann [:awiss]]

Attachment: asuth's fix with test case

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8456482 [details] [review]
asuth's fix with test case

Test pass clean locally after rebasing to master.  r=asuth, will merge and propagate to gaia now.

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

landed on gaia-email-libs-and-more/master:
https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/322
https://github.com/mozilla-b2g/gaia-email-libs-and-more/commit/0df84c73c89c1ce4afaa9622825c7187049b672f

landed on gaia/master:
https://github.com/mozilla-b2g/gaia/pull/22088
https://github.com/mozilla-b2g/gaia/commit/0128f0de5105f71462c0eff99194abc6763763b6