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

RESOLVED FIXED in Thunderbird 53.0

Status

Thunderbird
Mail Window Front End
RESOLVED FIXED
7 months ago
3 months ago

People

(Reporter: David E. Ross, Assigned: Jorg K (GMT+2))

Tracking

({regression})

Trunk
Thunderbird 53.0
regression

Thunderbird Tracking Flags

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

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

7 months ago
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>.
(Reporter)

Comment 1

7 months ago
Created attachment 8812609 [details]
source of message with preference variable true
(Reporter)

Comment 2

7 months ago
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?
(Assignee)

Comment 3

7 months ago
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)
(Assignee)

Comment 4

7 months ago
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.
(Assignee)

Comment 5

7 months ago
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.
(Reporter)

Comment 6

7 months ago
While I wrote this bug report for the Mail Window Front End component, it might instead belong in the Message Reader UI component.
(Assignee)

Comment 7

7 months ago
Created attachment 8812797 [details] [diff] [review]
Possible solution (v1)

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)
(Assignee)

Comment 8

7 months ago
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.

Comment 9

7 months ago
I do not understand the fix. How does 'undisclosed recipients' get displayed? Which variable contains it now?
(Assignee)

Comment 10

7 months ago
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?

Comment 11

7 months ago
Strangely, I do not get any To: line in such a message source (where I only put bcc recipient). What am I doing wrong?

Comment 12

7 months ago
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?

Comment 13

7 months ago
Well, it is translated by the receiving client, not the sending. That seems strange to me.
(Assignee)

Comment 14

7 months ago
(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 15

7 months ago
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+

Updated

7 months ago
OS: Windows → All
Hardware: Unspecified → All
Version: unspecified → Trunk
(Assignee)

Comment 16

7 months ago
Created attachment 8812905 [details] [diff] [review]
1318964-undisclosed-recipients.patch (v2)

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 17

7 months ago
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+
(Assignee)

Comment 18

7 months ago
Created attachment 8812912 [details] [diff] [review]
1318964-undisclosed-recipients.patch (v3)

(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 19

7 months ago
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+
(Assignee)

Comment 20

7 months ago
https://hg.mozilla.org/comm-central/rev/3f40fe5e5b2799f9273293f1777f86121b97f027
Pushed with extra parenthesis.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 53.0
(Assignee)

Comment 21

7 months ago
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+
(Reporter)

Comment 22

7 months ago
(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.
(Assignee)

Comment 23

7 months ago
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.
(Assignee)

Updated

7 months ago
Attachment #8812912 - Flags: approval-comm-beta?
(Assignee)

Comment 24

7 months ago
https://hg.mozilla.org/releases/comm-aurora/rev/d89432306b5bb687f5975797452b994d6518299b
status-thunderbird52: --- → fixed
status-thunderbird53: --- → fixed
status-thunderbird_esr45: --- → affected
tracking-thunderbird_esr45: --- → ?
(Assignee)

Comment 25

7 months ago
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+
(Assignee)

Comment 26

7 months ago
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
(Assignee)

Comment 27

7 months ago
Beta (TB 51):
https://hg.mozilla.org/releases/comm-beta/rev/ba2e07098d064608cf0e900d9fa9f409eaf20e07
status-thunderbird51: --- → fixed
(Assignee)

Comment 28

7 months ago
(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.

Updated

7 months ago
tracking-thunderbird_esr45: ? → -

Comment 29

6 months ago
I believe my tracking - earlier was a typo.
tracking-thunderbird_esr45: - → +
(Assignee)

Comment 30

5 months ago
https://hg.mozilla.org/releases/comm-esr45/rev/3c8f0c9bae33d227b523200df412fd4de8ecb4f3
(Kent forgot this one)
status-thunderbird_esr45: affected → fixed
tracking-thunderbird_esr45: + → 51+
(Assignee)

Updated

5 months ago
Attachment #8812912 - Flags: approval-comm-esr45? → approval-comm-esr45+

Comment 31

3 months ago
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.

Comment 32

3 months ago
Oh - the tests were on Windows 7 Pro 64bit, TB 52 32bit release.
(Assignee)

Comment 33

3 months ago
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.

Comment 34

3 months ago
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.