Closed Bug 1372928 Opened 2 years ago Closed 2 years ago

In "message body as plaintext" mode, invitation are not detected as so and do not have .ics file attached

Categories

(MailNews Core :: MIME, defect)

defect
Not set

Tracking

(thunderbird_esr5257+ fixed, thunderbird57 fixed, thunderbird58 fixed)

RESOLVED FIXED
Thunderbird 58.0
Tracking Status
thunderbird_esr52 57+ fixed
thunderbird57 --- fixed
thunderbird58 --- fixed

People

(Reporter: sexxxenator, Assigned: jorgk)

References

Details

(Whiteboard: TB 57 beta => TB 52.5 ESR)

Attachments

(3 files, 1 obsolete file)

Attached image b1+2.png
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170524174214

Steps to reproduce:

Preliminary note: since I don't want anybody to execute unknown code on my machine (who wants that????), my ThB is configured to "View" "Message Bod[ies]" "As Plain Text"

I received an email that is an invitation to a meeting.


Actual results:

1 - email is not detected as an invitation and Thb+lightning does not propose to Accept/refuse
2 - the invitation is not shown (not even as an attached .ics)


Expected results:

- Invitation text should appear in the body
- An ics file should appear as attachments
- Lightning detect that it's an invitation and propose to accept/reject it

Might be related to bug XXX
bug XXX == https://bugzilla.mozilla.org/show_bug.cgi?id=1372935

PS: the sender was an Apple user. That's what makes me think the problem lies in the ("Plain Text" View mode) vs. ("Orig HTML" View mode)
Can you attach such a message for inspection here: Save/drag to the desktop, edit resulting .eml file with editor to remove personal information, then attach here, "Attach File" above.
Duplicate of this bug: 1409668
(In reply to Jorg K (GMT+2) from comment #2)
> Can you attach such a message for inspection here: Save/drag to the desktop,
> edit resulting .eml file with editor to remove personal information, then
> attach here, "Attach File" above.

There are reports on irc that this has been working in 45.8. I have done some tests with TB52.4: for messages with a single text/calendar part this seems to work also in plain text mode, while for multipart messages the calendar part is not detected. In any of the HTML modes, it works like expected.

To reproduce in Daily yourself you can take any mulktipart message with an text/calendar part, regardless whether this is multipart/alternative or multipart/mixed. Just switch in TB menu View->Message Body As to Plain Text. This is also an issue if you codewise effectively disable the display changes feature, so it's not related to bug 1360155, but seems to be a mime issue / regression.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to [:MakeMyDay] from comment #4)
> To reproduce in Daily yourself you can take any mulktipart message with an
> text/calendar part, regardless whether this is multipart/alternative or
> multipart/mixed.
OK, I used attachment 8901591 [details] from bug 1360155 and it works just fine when viewing as plaintext in a current Daily.

So kindly attach an example that doesn't work.
Flags: needinfo?(makemyday)
Sure. After some further testing, it seems to be limited to a multipart/alternative within a multipart/mixed with an additional ics attachment.
Flags: needinfo?(makemyday)
Hmm, that message really has a quite unfortunate MIME structure:

multipart/mixed
  multipart/alternative
    text/plain
    text/calendar
attachment.

Since bug 253830 we prioritise the text part higher than anything else when in plaintext mode:
https://dxr.mozilla.org/comm-central/rev/952762964f408e33d060dd1110cac0626ae5fa51/mailnews/mime/src/mimemalt.cpp#432
text/plain gets PRIORITY_HIGHEST and text/calendar gets PRIORITY_NORMAL.

Before that bug which landed on TB 49, so not in TB 45 but in TB 52, all text parts had the same priority, so the last one won, maybe good for calendar, but not good if the last part was HTML that then got converted to plaintext, ignoring the dedicated plaintext part.

The simple fix is to move text/calendar up in priority if that's what's desired.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Component: Lightning Only → MIME
Product: Calendar → MailNews Core
Version: Lightning 5.4.1.1 → 52
I guess you've heard about the Mailnews misery, so kindly try this for me and approve.
Attachment #8924334 - Flags: review?(makemyday)
Yes, I'll fix the trailing space.
(v1) wasn't right. You need more context than the patch contains to see that in plaintext mode both text/plain and text/calendar receive the highest priority.
Attachment #8924334 - Attachment is obsolete: true
Attachment #8924334 - Flags: review?(makemyday)
Attachment #8924373 - Flags: review?(makemyday)
Comment on attachment 8924373 [details] [diff] [review]
1372928-plaintext-invite.patch (v2)

Or maybe Aceman also wants to take a look.
Attachment #8924373 - Flags: review?(acelists)
Why do we need to play with priorities here? This shouldn't be a case of choosing one of many different variants of the text.
If there is a calendar, shouldn't it be shown in any case (regardless of plain/html display)? Can there be multiple calendars in a message? Shouldn't the calendar handled/shown in addition a the chosen body part, regardless of priorities?
The decision of which part to display is done via priorities. The part with the highest priority wins, or the last part if two parts have the same priority. We won't change this design here.

In plaintext view mode, the plaintext part receives the highest priority to ensure it is displayed and not the HTML part (downgraded to plaintext) which typically comes later.

What the author of bug 253830, Terje, and the reviewer, myself, overlooked is that when text/calendar follows text/plain in plaintext mode, text/calendar should receive the same (highest) priority that text/plain receives. So if Lightning is installed, the last part with that high priority, so the text/calendar part is displayed, or better, passed to Lightning via some complicated process to display in its own special way.

The patch corrects that oversight with one simple if-clause. That the 100% correct fix.
Actually: That *is* the 100% correct fix.
Comment on attachment 8924373 [details] [diff] [review]
1372928-plaintext-invite.patch (v2)

Review of attachment 8924373 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, it works like expected with this patch. Too me, this approach seems reasonable, so r+ from my side (Although I'm not a peer here, so please decide yourself whether this is sufficient for landing).

If there is more then one event in the email or other parts should be displayed additionally, the email should have been composed not using multipart/alternate alone but just multipart/mixed (for which displaying the events already works). For providing an option to switch between a text/calendar or text/plain resp. text/html for multipart/alternate messages, there is with bug 1406868 an enhancement request on file, so we should keep things separate here.
Attachment #8924373 - Flags: review?(makemyday) → review+
(In reply to Jorg K (GMT+2) from comment #13)
> So if Lightning is installed, the last part with that high
> priority, so the text/calendar part is displayed, or better, passed to
> Lightning via some complicated process to display in its own special way.

I just do not understand why the calendar part has to compete with the plain/html parts of messages to get displayed. I think priority should be computed for the message parts, but calendar part should be sent to Lightning in any case and that decides how to display it.
Feel free to rework it and deal with the fallout :-(

This is the second regression from bug 253830 and its friend, bug 574989, there was also bug 1334937.

Currently all parts compete, that's the architecture, and if the winning part is text/calendar, we ship it off to Lightning to determine its display.
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/5014946f75dc
prioritise text/calendar the same as text/plain in plaintext mode so inviation is displayed. r=MakeMyDay
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Attachment #8924373 - Flags: review?(acelists)
Attachment #8924373 - Flags: review+
Attachment #8924373 - Flags: approval-comm-esr52?
Attachment #8924373 - Flags: approval-comm-beta+
Attachment #8924373 - Flags: review+
Target Milestone: --- → Thunderbird 58.0
Whiteboard: TB 57 beta => TB 52.5 ESR
Attachment #8924373 - Flags: approval-comm-esr52? → approval-comm-esr52+
You need to log in before you can comment on or make changes to this bug.