Closed Bug 1318964 Opened 5 years ago Closed 5 years ago

Thunderbird fails to display "undisclosed-recipients" in thread/header pane when message was sent to BCC recipient only (likely JS Mime regression)

Categories

(Thunderbird :: Mail Window Front End, defect)

defect
Not set
normal

Tracking

(thunderbird_esr4551+ fixed, thunderbird51 fixed, thunderbird52 fixed, thunderbird53 fixed)

RESOLVED FIXED
Thunderbird 53.0
Tracking Status
thunderbird_esr45 51+ fixed
thunderbird51 --- fixed
thunderbird52 --- fixed
thunderbird53 --- fixed

People

(Reporter: david, Assigned: jorgk-bmo)

Details

(Keywords: regression)

Attachments

(2 files, 2 obsolete files)

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.5.0

With the preference variable mail.compose.add_undisclosed_recipients set to "true" and all destination addresses set for Bcc, Thunderbird fails to display "undisclosed-recipients" for the To: header field.  In the message source, I see
   To: undisclosed-recipients: ;
However, the displays of both the as-sent and the as-received message, I see only
   To:  
(that is, "To:" with the rest of the line blank).  Yes, I tested this while running Thunderbird in Safe Mode.  

This has been the topic of a discussion in the mozilla.support.thunderbird newsgroup in news.mozilla.org, subject "Undisclosed Recipients missing from SENT folder".  See <mailman.1083.1479405090.16819.support-thunderbird@lists.mozilla.org>.
Note that the primary issue in the newsgroup discussion is that the phrase "undisclosed-recipients" should display without the user having to view the message source.  After all, what would prompt the user to request the message source?
The discussion on support-thunderbird@lists.mozilla.org can be found here:
https://groups.google.com/d/msg/mozilla.support.thunderbird/4KOB4v-BsiM/Iqq7cP44BwAJ

This thread started with:

Just updated to TB45.4 and noticed that in all of my SENT folders the
RECIPIENT space is blank ONLY for email sent using only BCC.   The
RECIPIENT space had "UNDISCLOSED RECIPIENT" before I updated.
Is there a fix, is this a bug, was this feature removed, is there a
config setting?

https://groups.google.com/d/msg/mozilla.support.thunderbird/4KOB4v-BsiM/3mKb3mYhCAAJ
says:
Version 24.0 displays "Undisclosed Recipients". Version 38.0.1 not working.

So the bug is: When message is sent with BCC recipients only, resulting |To: undisclosed-recipients: ;| is displayed as blank in thread and message/header pane.

This is most likely a JS Mime regression between TB 31 and TB 38. I confirmed that TB 31 displays "undisclosed-recipients" and TB 38 does not.
Keywords: regression
Summary: Thunderbird Fails to Display "undisclosed-recipients" → Thunderbird fails to display "undisclosed-recipients" in thread/header pane when message was sent to BCC recipient only (likely JS Mime regression)
I've looked into this issue a little. It appears that the To: header
  To: undisclosed-recipients: ;
is covered by many tests, see
https://dxr.mozilla.org/comm-central/search?q=undisclosed-recipients&redirect=false

It gets parsed into an *empty* group named "undisclosed-recipients", see for example here:
https://dxr.mozilla.org/comm-central/rev/09ec3012ff3d5d3ea84338b2d7707046046bdcbc/mailnews/mime/jsmime/test/test_structured_headers.js#108
See just the line below for an example of a non-empty group:
"world: a@example.invalid, b@example.invalid;"

This is even documented here:
https://dxr.mozilla.org/comm-central/rev/09ec3012ff3d5d3ea84338b2d7707046046bdcbc/mailnews/mime/public/nsIMsgHeaderParser.idl#27

There is also another test
https://dxr.mozilla.org/comm-central/rev/09ec3012ff3d5d3ea84338b2d7707046046bdcbc/mailnews/mime/test/unit/test_nsIMsgHeaderParser2.js#51
and there was bug 549931 relating to "undisclosed-recipients".

Sadly there is no test for "undisclosed-recipients" in the emitter tests:
https://dxr.mozilla.org/comm-central/rev/09ec3012ff3d5d3ea84338b2d7707046046bdcbc/mailnews/mime/jsmime/test/test_header_emitter.js#59

I would bet that the new display logic displays an empty group as empty without including the group name or special casing "undisclosed-recipients". I'd have to dig through JS Mime to confirm this assumption.
A header of |To: world: a@a.b;|, so a group called "world" with one member |a@a.b|, displays as |a@a.b|, so the group name doesn't show in the UI. Since I can't see special casing for undisclosed-recipients, the empty group will thus result in an empty display as observed.

Needs more digging into.
While I wrote this bug report for the Mail Window Front End component, it might instead belong in the Message Reader UI component.
Attached patch Possible solution (v1) (obsolete) — Splinter Review
This went bust in TB 38 and no one ever noticed until now. Great!
Thanks David for persisting. We'll fix it for TB 52 due out in March 2017.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8812797 - Flags: review?(acelists)
BTW, most likely JS Mime changed parseHeadersWithArray()
https://dxr.mozilla.org/comm-central/rev/d4e762ef561de865b6e95befb61beafaf914866d/mailnews/mime/src/mimeJSComponents.js#386
or underlying functions like MimeParser.parseHeaderField().

I don't think it's valid to stuff the "undisclosed-recipients" into an name/address since then you can send e-mail to this address which makes no sense. Try it in TB 31, you get undisclosed-recipients in the From field. By just adding it as a display, I avoid this behaviour, it's really only for display.
I do not understand the fix. How does 'undisclosed recipients' get displayed? Which variable contains it now?
emailAddresses contains the header value |undisclosed-recipients: ;| that is successfully parsed into an empty group so nothing is displayed. In this case, we know it must be an empty group and display the group name from the raw header. Have you tried it?
Strangely, I do not get any To: line in such a message source (where I only put bcc recipient). What am I doing wrong?
I mean the outgoing message. In the received message there is To: undisclosed recipients (even translated) in the message source. Is that the expected behaviour?
Well, it is translated by the receiving client, not the sending. That seems strange to me.
(In reply to :aceman from comment #12)
> I mean the outgoing message. In the received message there is To:
> undisclosed recipients (even translated) in the message source. Is that the
> expected behaviour?
I think so. In the message that is copied to the Sent folder, the BCC is maintained.
The "undisclosed-recipients: ;" is inserted into the message that is really sent.

The patch is simply to display the (raw) To: header in case no addresses were extracted.
Comment on attachment 8812797 [details] [diff] [review]
Possible solution (v1)

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

Ok, I can now see the string is produced by the sending agent (I forgot I sent multiple message from multiple language versions of TB).
The patch works with all the translated versions of the "undisclosed-recipients". Displays what is in the msg source.

::: mail/base/content/msgHdrViewOverlay.js
@@ +1616,5 @@
>    fields.newsgroups = addressNode.getAttribute("newsgroup");
>    if (addressNode.hasAttribute("fullAddress")) {
>      let addresses = MailServices.headerParser.makeFromDisplayAddress(
>        addressNode.getAttribute("fullAddress"), {});
> +    if (addresses.length)

addresses.length > 0
Attachment #8812797 - Flags: review?(acelists) → review+
OS: Windows → All
Hardware: Unspecified → All
Version: unspecified → Trunk
Sorry, this wasn't quite right yet. Firstly I got this on the console:
UpdateEmailNodeDetails failed: TypeError: aEmailAddress is undefined

That's because now I'm processing an entry with an empty e-mail address which isn't expected.

Secondly I did some reading to see where parseHeadersWithArray() was used elsewhere. I found a spot which discarded any addresses with a colon in them. That's pre-JS Mime code. Now groups will be processed and no e-mail address with a colon will ever be returned. So that processing can go as it only confuses. Do you agree?

However, from that I took the idea to check for a colon in my new code.
Attachment #8812797 - Attachment is obsolete: true
Attachment #8812905 - Flags: review?(acelists)
Comment on attachment 8812905 [details] [diff] [review]
1318964-undisclosed-recipients.patch (v2)

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

Looks more tight now ;)

But I still get: 
22:30:05.584 ReferenceError: reference to undefined property "emailAddress" 1 mailWidgets.xml:615:13
Attachment #8812905 - Flags: review?(acelists) → review+
(In reply to :aceman from comment #17)
> But I still get: 
> 22:30:05.584 ReferenceError: reference to undefined property "emailAddress"
> 1 mailWidgets.xml:615:13

Right. This should fix it.
Attachment #8812905 - Attachment is obsolete: true
Attachment #8812912 - Flags: review?(acelists)
Comment on attachment 8812912 [details] [diff] [review]
1318964-undisclosed-recipients.patch (v3)

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

Thanks.

::: mail/base/content/mailWidgets.xml
@@ +630,5 @@
>              }
>  
>              try
>              {
> +              if ("UpdateEmailNodeDetails" in top && aAddress.emailAddress)

Please put brackets around the ("" in top) when pushing.
Attachment #8812912 - Flags: review?(acelists) → review+
https://hg.mozilla.org/comm-central/rev/3f40fe5e5b2799f9273293f1777f86121b97f027
Pushed with extra parenthesis.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 53.0
Comment on attachment 8812912 [details] [diff] [review]
1318964-undisclosed-recipients.patch (v3)

Let's uplift this to TB 52 in due course. Low risk, only affects the display of messages sent to BCCs only.
Attachment #8812912 - Flags: approval-comm-aurora+
(In reply to Jorg K (GMT+1) from comment #14)
> (In reply to :aceman from comment #12)
> > I mean the outgoing message. In the received message there is To:
> > undisclosed recipients (even translated) in the message source. Is that the
> > expected behaviour?
> I think so. In the message that is copied to the Sent folder, the BCC is
> maintained.
> The "undisclosed-recipients: ;" is inserted into the message that is really
> sent.
> 
> The patch is simply to display the (raw) To: header in case no addresses
> were extracted.

NOTE WELL:  With the preference variable mail.compose.add_undisclosed_recipients set to "true" and all destination addresses set for Bcc, both the sources of the as-sent and as-received messages in the Send and Inbox folders respectively should indeed have "To: undisclosed-recipients: ;".  This is the way Thunderbird worked when this bug report was submitted.  The problem is that the displayed messages did not do this.  Please make sure that the fix addresses both the as-sent and as-received messages.
I checked: In the sent message there is also the |To: undisclosed-recipients:;| header. It works like TB 31 now. You can verify this with today's Daily build available from about 14:00 GMT.
Attachment #8812912 - Flags: approval-comm-beta?
Comment on attachment 8812912 [details] [diff] [review]
1318964-undisclosed-recipients.patch (v3)

[Approval Request Comment]
Regression caused by (bug #): JS Mime and friends
User impact if declined: completely blank To field in UI, not nice.
Testing completed (on c-c, etc.): Manual.
Risk to taking this patch (and alternatives if risky):
Low, only affects code path when message sent to BCCs only.
Attachment #8812912 - Flags: approval-comm-esr45?
Attachment #8812912 - Flags: approval-comm-beta?
Attachment #8812912 - Flags: approval-comm-beta+
Oops, I forgot to write me standard comment, so here we go again:
Aurora (TB 52):
https://hg.mozilla.org/releases/comm-aurora/rev/d89432306b5bb687f5975797452b994d6518299b
(In reply to :aceman from comment #15)
> The patch works with all the translated versions of the
> "undisclosed-recipients". Displays what is in the msg source.
Yep, in a message from a German sender I can see "Verborgene_Empfaenger;" now.
I believe my tracking - earlier was a typo.
Attachment #8812912 - Flags: approval-comm-esr45? → approval-comm-esr45+
Someone on the Thunderbird support list just complained that this still wasn't fixed in TB 52, and I must confirm this issue still exists.

To confirm, I sent a test email from one TB/account BCC only to another TB/account, and here is what I see in the different places:

Note: I use the ToggleHeaders Addon.

1. In the Sent folder of the sending system I see:

  a) in the thread pane, the Recipient column is blank,
  b) in the message pane header with headers collapsed I see:
     'To Recipient@Example.com',
  c) in the message pane header with headers expanded, I see:
     'Bcc Recipient@Example.com'

2. In the recipient Inbox, I see:

  a) in the message pane header with headers collapsed, I see:
     'To' (it is blank)
  b) in the message pane header with headers expanded, I see:
     'To Undisclosed recipients;'

So, the ONLY place I ever see 'To Undisclosed recipients;' is in the receivers Thunderbird, and only when the headers are not collapsed.
Oh - the tests were on Windows 7 Pro 64bit, TB 52 32bit release.
I know nothing about the ToggleHeaders add-on.

You're right, TB doesn't show "undisclosed recipients;" in the thread pane as TB 31 did. I filed bug 1354822 for that.
Sorry, my bad, I meant the Compact Headers Addon. Hopefully you're familiar with that one.

ToggleHeaders just adds the ability to use a keyboard shortcut to toggle them just tap the 'H' key).
You need to log in before you can comment on or make changes to this bug.