If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Selected text from unrelated message is quoted when right-click replying to another message, cunningly with correct attribution line

RESOLVED FIXED in Thunderbird 56.0

Status

MailNews Core
Composition
--
critical
RESOLVED FIXED
8 years ago
a month ago

People

(Reporter: Jan, Assigned: Jorg K (GMT+2))

Tracking

(Blocks: 1 bug, {privacy})

Trunk
Thunderbird 56.0
privacy
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird_esr5255+ fixed, thunderbird56 fixed, thunderbird57 fixed)

Details

(Whiteboard: [DUPEME][bogus-focus])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.0.12) Gecko/2009070611 Firefox/3.0.12 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.1.1) Gecko/20090715 Thunderbird/3.0b3

In TB3 beta 2 and 3 (at least), if you select a piece of text in one e-mail (in the message preview pane), then right-click on another e-mail (in the subject list above) and click reply, the selected text from the open mail (i.e. not the one you are replying to!) is quoted. Additionally, the attribution line ("on date, xy wrote") contains the name of the author of the mail you are replying to, NOT the one the quoted text is from.

Reproducible: Always

Steps to Reproduce:
1. Open a folder with at least 2 mails from different authors. I call them mail 1 from author A and mail 2 from author B.
2. Click on mail 1 so you can see the text in the message pane.
3. Select some of the text
4. Right-Click mail 2, select reply
Actual Results:  
You will see:
On (date), B wrote:
> (text by A from selected mail)

Expected Results:  
Quote complete mail that was right-clicked. Maybe refresh the preview pane before any actions are performed, immediately when the context menu is displayed. As the blue "this message is selected" marking switches immediately, the preview pane should reflect this to avoid confusion.

Comment 1

8 years ago
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.3pre) Gecko/20090730 SeaMonkey/2.0b2pre, no duplicate found.

This is indeed irritating, the quote and the attribution doesn't match, neither does the subject. Either the context-menu item should be disabled if a quote is selected, or the full other message quoted after selecting it.
Status: UNCONFIRMED → NEW
Component: General → Composition
Ever confirmed: true
Product: Thunderbird → MailNews Core
QA Contact: general → composition
Version: unspecified → Trunk

Updated

8 years ago
Whiteboard: DUPEME

Comment 2

4 years ago
I've seen several bugs where bogus-focus when right-clicking messages in message list is a problem.
I know some people are saying it's by design to preserve multiple selections (where you can right-click on single msgs without losing such selection). However, such behaviour is nowhere to be found in any other windows application, and hence I doubt many people are using it that way. Otoh, not moving focus on right-click violates basic focus principles, and causes a lot of confusion and bugs. I don't think the following workflow is very intuitive and frequent:
- Select a bunch of messages (so you're intending to act on the bunch, right?),
- then suddenly act by right-click on a single message without seeing the preview of that msg,
- while expecting to keep your selection?

Updated

4 years ago
Whiteboard: DUPEME → [DUPEME][bogus-focus]

Updated

4 years ago
Duplicate of this bug: 539648

Updated

4 years ago
See Also: → bug 181028

Updated

6 months ago
Duplicate of this bug: 729053

Comment 5

6 months ago
Does this still happen? (can't test right now)
If yes:

This is a serious and cunning violation of user privacy, and should be fixed asap.
The cunning part here is that the correct and expected attribution line gets stuck on top the wrong quote from an unrelated message, so without double-checking the quote, user may not even notice sending the wrong thing.
We should really kill those bugs where stuff from unrelated emails silently gets sent to people.
This is among the worst things a mailer can do.

Removing the deviant selection behaviour described in my comment 2 would be the safest way of avoiding bugs like this one, as right-click would first select the new message before doing any command.
The selection behaviour is reminiscent of the other selection bug on contacts side bar, maybe even the same thing because of poor tree implementation. I wouldn't call this a feature, but others might find it useful, as it allows acting differently on a single message while you select multiple messages. See comment 2.
Severity: normal → critical
Flags: needinfo?(jan-bugreport)
OS: Windows XP → All
Hardware: x86 → All
Summary: Wrong quoting/attribution line with selected text and right-click reply to other mail → Selected text from unrelated message is quoted when right-click replying to another message, cunningly with correct attribution line

Updated

6 months ago
Keywords: privacy
(Reporter)

Comment 6

6 months ago
Still reproduces on 45.8.0 using the original repro steps. Make sure to select enough text in step 3, apparently no quote is made if only a single word is selected.
Flags: needinfo?(jan-bugreport)
(Assignee)

Comment 7

6 months ago
Reproducible in Daily TB 55, quoted text is taken from the wrong message. Interesting.
(Assignee)

Comment 8

6 months ago
Fix needs to be somewhere in mailCommands.js (ComposeMessage()) or nsMsgComposeService::OpenComposeWindow() which calls nsMsgComposeService::GetOrigWindowSelection(). We need to check that the selection belongs to the message that's being replied to. That's easier said than done. Maybe we should just reset the selection if replying to a different message.
Blocks: 742721
Duplicate of this bug: 742721
(Assignee)

Comment 10

2 months ago
Created attachment 8891701 [details] [diff] [review]
507541-selection-reply.patch - WIP (v1).

This bit of code tests whether the displayed message is being replied to or another message.

In the latter case we should not quote the selection from the displayed message.

Full patch to follow.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
(Assignee)

Comment 11

2 months ago
Created attachment 8891702 [details] [diff] [review]
507541-selection-reply.patch - WIP (v2).

Better like this. Clearing the selection doesn't work, but we can't be far off.
Attachment #8891701 - Attachment is obsolete: true
(Assignee)

Comment 12

2 months ago
Created attachment 8891729 [details] [diff] [review]
507541-selection-reply.patch - (v3).

This works, also tested on stand-alone message window, replies can still quote.
Attachment #8891702 - Attachment is obsolete: true
Attachment #8891729 - Flags: review?(acelists)

Comment 13

2 months ago
Comment on attachment 8891729 [details] [diff] [review]
507541-selection-reply.patch - (v3).

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

It is ugly that this can't be solved in the UI, but the composeService grabs into the window and gets the selected text on its own.

::: mail/base/content/mailCommands.js
@@ +163,5 @@
>    let match = /.+#(\d+)/.exec(uri);
>    return (match) ? match[1] : null;
>  }
>  
>  // type is a nsIMsgCompType and format is a nsIMsgCompFormat

Please convert this to the modern format while here ;)

::: mailnews/compose/public/nsIMsgComposeParams.idl
@@ +39,5 @@
>       */
>      const long Redirect                 = 14;
> +
> +    /**
> +     * Add this value to a reply type so suppress quoting the selection.

"*to* suppress"? I'd expand the description to "suppress quoting the current text selection".
Attachment #8891729 - Flags: review?(acelists) → review+

Comment 14

2 months ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/5dcb4353fcec
Ignore selection when not replying to displayed message. r=aceman
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → FIXED
(Assignee)

Updated

2 months ago
Target Milestone: --- → Thunderbird 56.0
(Assignee)

Comment 15

2 months ago
Comment on attachment 8891729 [details] [diff] [review]
507541-selection-reply.patch - (v3).

The bug is old enough to deserve an uplift.
Attachment #8891729 - Flags: approval-comm-esr52?
Thanks. Yeah, not cool. (although you would hope in most cases the user would notice this)
(Assignee)

Comment 17

2 months ago
(In reply to :aceman from comment #13)
> Please convert this to the modern format while here ;)
Done.
 
> "*to* suppress"? I'd expand the description to "suppress quoting the current
> text selection".
Done, I haven't used the word "text" since the selection is HTML in general.

I've also taken the liberty for a further small optimisation:
https://hg.mozilla.org/comm-central/rev/5dcb4353fcec#l1.52
since we already have the message key.
Blocks: 1385727

Comment 18

2 months ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/8530044d4d8c
Follow-up: cater for case where no message is displayed. r=me
(Assignee)

Comment 19

2 months ago
Sadly that tripped up a test (test-reply-multipart-charset.js) which was doing some stuff while the preview pane was turned off. Being a bit more careful with accessing |gMessageDisplay.displayedMessage| fixes the issue.
(Assignee)

Comment 20

2 months ago
Created attachment 8891828 [details] [diff] [review]
507541-follow-up.patch
(Assignee)

Updated

a month ago
Attachment #8891729 - Flags: approval-comm-esr52? → approval-comm-esr52+
(Assignee)

Comment 21

a month ago
TB 52.3 ESR:
https://hg.mozilla.org/releases/comm-esr52/rev/61341e35f732
https://hg.mozilla.org/releases/comm-esr52/rev/7ae40637aa82
status-thunderbird56: --- → fixed
status-thunderbird57: --- → fixed
status-thunderbird_esr52: --- → fixed
tracking-thunderbird_esr52: --- → 56+
(Assignee)

Updated

a month ago
tracking-thunderbird_esr52: 56+ → 55+
You need to log in before you can comment on or make changes to this bug.