Bug 3889

Summary: address and quoted message inconsistent in reply
Product: Claws Mail (GTK 2) Reporter: Michal Suchánek <msuchanek>
Component: UI/ActionsAssignee: users
Status: RESOLVED FIXED    
Severity: normal CC: kardan
Priority: P3    
Version: 3.13.2   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
workaround for missing text conversion
none
potential fix
none
Another proposed fix
none
Updated patch
none
Another better patch none

Description Michal Suchánek 2017-09-01 20:00:04 UTC
In split view it is possible that the highlighted message in the folder view is different from the message shown in the message view.

Hitting reply quotes the message shown to the author of the message selected in folder view.
Comment 1 Chad Wallace 2017-09-01 20:40:55 UTC
I'm not seeing this in 3.14.1.  If I have a message shown in message view, and move the selection up to a different message in the message list, and hit Reply, the reply address and quoting are both based on the message shown in the view.
Comment 2 Michal Suchánek 2017-09-01 22:08:42 UTC
Indeed. It only happens with reply-all and possibly only when there is more than one address.
Comment 3 Paul 2017-09-02 11:20:52 UTC
I cannot reproduce this. If you can reproduce this with the latest release (currently 3.15.1) then re-open with specific instructions for triggering the behaviour.
Comment 4 Michal Suchánek 2018-05-28 19:07:56 UTC
Created attachment 1875 [details]
workaround for missing text conversion

This is the patch that fixes the issue for me.

However, there is no way to reply to multiple messages because there is missing text conversion. It never worked to start with.
Comment 5 Paul 2018-09-12 12:30:58 UTC
I believe that this was caused by a patch added by suse to their package, Do-not-use-msginfo_list-for-compose.patch, which breaks things.

This breakage was pointed out to them and the patch has been dropped since.
Comment 6 Michal Suchánek 2018-09-12 18:07:00 UTC
no, the breakage is fixed by the patch. without that patch the messages selected in the message list are used for reply address but the message shown in message view for text.

Because these need not be in sync the reply address need not correspond to the message.
Comment 7 Paul 2018-09-12 19:23:26 UTC
with that patch you have to open a msg to reply to it, you can't just right-click a msg to reply. also with that patch you can't select more than 1 msg (in order to open more than one compose window)and reply to them all.

So it breaks more than it attempts to fix.
Comment 8 Michal Suchánek 2018-09-12 21:07:50 UTC
(In reply to comment #7)
> with that patch you have to open a msg to reply to it, you can't just
> right-click a msg to reply. 

Yes, you have to do that. Since the message view is the only component that can convert message to text you won't get a message to reply to otherwise.

> also with that patch you can't select more than
> 1 msg (in order to open more than one compose window)and reply to them all.

To reply to a number of messages quoting completely unrelated message? No, thanks.

It would be awesome if the code to convert a message to text was ripped out of the messageview so you actually could quote arbitrary message and not only the one displayed but that's a feature to be implemented.

> 
> So it breaks more than it attempts to fix.

AFAICT it does not break anything what was not broken to start with.
Comment 9 Paul 2018-09-13 00:13:09 UTC
(In reply to comment #8)
> Yes, you have to do that. Since the message view is the only component that
> can convert message to text you won't get a message to reply to otherwise.

You are absolutely wrong here. Try it (without the aforementioned patch) and you will see. Enter a folder without automatically opening a message. Right-click any message and choose 'reply'.
 
> To reply to a number of messages quoting completely unrelated message? No,
> thanks.

No. To reply to a number of messsages, each quoting the respective message you are replying to, each being addressed to the sender of the respective message. Try it (without the aforementioned patch) and you will see.

> AFAICT it does not break anything what was not broken to start with.

Keep trying and you will see.
Comment 10 Paul 2018-09-13 14:39:37 UTC
The content of attachment 1875 [details] has been deleted by
    Paul <paul@claws-mail.org>
who provided the following reason:

breaks more than it fixes

The token used to delete this attachment was generated at 2018-09-13 14:39:22 CEST.
Comment 11 Paul 2018-09-13 14:40:55 UTC
Created attachment 1913 [details]
potential fix

Michal, try this patch.
Comment 12 Colin Leroy 2018-09-13 22:41:28 UTC
Hi!

Apparently that patch would break replying to selected part of the displayed body.

The problem, I think, is that the menubar "Reply" and the main menu's Reply items use the displayed message, and the summaryview's contextual menu "Reply" uses the same callback as the main menu.

For the two first cases, I'd say that's logical. For the last one, it's not.
Comment 13 Colin Leroy 2018-09-13 22:42:50 UTC
We could change the behaviour so that all "Reply" from mainwindow (bar and menu) would reply to the selected message instead of the opened one if the selected one is different from the opened one ?
Comment 14 Colin Leroy 2018-09-13 23:12:27 UTC
Created attachment 1914 [details]
Another proposed fix

May I suggest that patch?

- Using the toolbar buttons, no change
- Using the menus (main and summary): 
  - If the message opened in the messageview is part of the summaryview selection, no change.
  - Else, skip using the messageview information altogether.
Comment 15 Michal Suchánek 2018-09-14 14:36:30 UTC
(In reply to comment #14)
> Created attachment 1914 [details]
> Another proposed fix
> 
> May I suggest that patch?
> 
> - Using the toolbar buttons, no change
> - Using the menus (main and summary): 
>   - If the message opened in the messageview is part of the summaryview
> selection, no change.
>   - Else, skip using the messageview information altogether.

This is broken. The messageview is the most prominent target for reply. So when you click reply any place other than a popup from summaryview it should reply to the message from messageview.

The selection in the summaryview may not even be visible when you trigger reply anywhere else.
Comment 16 Colin Leroy 2018-09-14 14:55:42 UTC
That's true. I'll see if I can put in the extra work to make it work.
Comment 17 Michal Suchánek 2018-09-14 15:08:55 UTC
Also your patch does not remove the part that merges addressees from different messages so it will probably not fix the issue.
Comment 18 Michal Suchánek 2018-09-18 13:46:45 UTC
(In reply to comment #11)
> Created attachment 1913 [details]
> potential fix
> 
> Michal, try this patch.

I cannot reproduce the problem with this patch applied.
Comment 19 Paul 2018-09-18 14:47:06 UTC
(In reply to comment #18)
> 
> I cannot reproduce the problem with this patch applied.

Thanks for the test, but I spotted a couple problems with it after posting it here. Colin's patch is some steps in the right direction, although not yet without problems - e.g. main menu reply acts differently from toolbar button reply.
Comment 20 Colin Leroy 2018-09-24 10:16:43 UTC
Created attachment 1924 [details]
Updated patch

Hi,

Update of the previous patch, making main toolbar buttons behave the same as the menu items (both main and popup), so everything works the same:

- If the message opened in the messageview is part of the summaryview 
  selection, no change.
- Else, skip using the messageview information altogether.

Separate messageviews are unaffected of course: their buttons apply to their displayed message.
Comment 21 Paul 2018-09-25 11:43:27 UTC
*** Bug 4064 has been marked as a duplicate of this bug. ***
Comment 22 Michal Suchánek 2018-09-25 11:52:44 UTC
(In reply to comment #20)
> Created attachment 1924 [details]
> Updated patch
> 
> Hi,
> 
> Update of the previous patch, making main toolbar buttons behave the same as
> the menu items (both main and popup), so everything works the same:
> 
> - If the message opened in the messageview is part of the summaryview 
>   selection, no change.

This is bogus. If more than one message is selected using the messageview info for messages not shown in the messageview will produce the broken results.

> - Else, skip using the messageview information altogether.

This is also bogus. When a message is shown in the messageview any reply should go to the message in messageview unless very precisely specified otherwise (ie rightclicking a particular message in summary and picking reply from the context menu).

As said earlier the selection in summaryview need not be visible due to scrolling but the messageview is always visible. When both are visible the messageview is way more prominent. Skipping the messageview when replying does not make any sense from UI viewpoint.
Comment 23 Colin Leroy 2018-09-27 10:47:00 UTC
Created attachment 1925 [details]
Another better patch

Following more IRC chat, I think we can say that:

WE HAVE:
 
Message A is displayed, message A is selected
    Mainwindow replies => To message A
    Toolbar replies => To message A
    SummaryPopup on message A replies => To message A
    SummaryPopup on other message replies => To message A (IT SHOULDN'T)
   
Message A is displayed, messages A B C are selected
    Mainwindow replies => To A, B, C
    Toolbar replies => To A, B, C
    SummaryPopup on message A, B or C replies => To A, B, C
    SummaryPopup on other message (D) replies => To message A (IT SHOULDN'T)
 
Message A is displayed, but one other only, B, is selected
    Mainwindow replies => to message A with some corner-case bug in address selection (https://www.colino.net/tmp/reply.ogv)
    Toolbar replies => to message A with some corner-case bug in address selection (https://www.colino.net/tmp/reply.ogv)
    SummaryPopup on message A replies => to message A with some corner-case bug in address selection (https://www.colino.net/tmp/reply.ogv)
    SummaryPopup on message B replies => to message A, with message B To address (IT SHOULDN'T)
    SummaryPopup on other message (D) replies => to message A, with message D To address (IT SHOULDN'T)
 
Message A is displayed, but others only (B C D) are selected
    Mainwindow replies => to selected messages B C D
    Toolbar replies => to selected messages B C D
    SummaryPopup on one of the selected messages replies => B C D
    SummaryPopup on another message (E) replies => to message A, with message E To address (IT SHOULDN'T)
 
WE WANT:
 
Message A is displayed, message A is selected
    Mainwindow replies => To message A
    Toolbar replies => To message A
    SummaryPopup on message A replies => To message A
    SummaryPopup on other message B (selects B) replies => To message B
   
Message A is displayed, messages A B C are selected
    Mainwindow replies => To A, B, C
    Toolbar replies => To A, B, C
    SummaryPopup on message A, B or C replies => To A, B, C
    SummaryPopup on other message (D) (selects D) replies => To message D
 
Message A is displayed, but one other only, B, is selected
    Mainwindow replies => to message A
    Toolbar replies => to message A
    SummaryPopup on message A (selects A) replies => to message A
    SummaryPopup on message B replies => to message B
    SummaryPopup on other message (D) (selects D) replies => to message D
 
Message A is displayed, but others only (B C D) are selected
    Mainwindow replies => to message A
    Toolbar replies => to message A
    SummaryPopup on one of the selected messages replies => B C D
    SummaryPopup on another message (E) (selects E) replies => to message E
    
This doesn't describe precisely the "only quote selected part of the mail" feature,
nor the "reply to attached message/rfc822 when it is the currently selected part" feature,
which don't change and only applies when we reply to displayed message A and only message A is selected.

The attached patch implements that "WE WANT" part.
Comment 24 Michal Suchánek 2018-09-28 10:12:02 UTC
(In reply to comment #23)
> Created attachment 1925 [details]
> Another better patch

Thank you for the patch. I am unable to reproduce the issue with this patch applied and it indeed preserves the ability to open multiple reply windows at once.
Comment 25 users 2018-09-28 10:35:05 UTC
Changes related to this bug have been committed.
Please check latest Git and update the bug accordingly.
You can also get the patch from:
http://git.claws-mail.org/

++ ChangeLog	2018-09-28 10:35:04.733301259 +0200
http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=9b0970f01e48727d53915965ae42c27368874b69
Merge: 3d2d178 5d3761f
Author: Colin Leroy <colin@colino.net>
Date:   Fri Sep 28 10:35:03 2018 +0200

    Merge branch 'master' of file:///home/git/claws

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=5d3761fe8f2dafd4eb30fd9322b00f5e6bdbaf4f
Author: Colin Leroy <colin@colino.net>
Date:   Fri Sep 28 10:33:20 2018 +0200

    Fix bug #3889.
    * Fix right-click replying to messages in summaryview
    * Fix corner-case when selected and opened messages are different,
      and one of them is to a mailing-list.

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=d15c76ad8978cea678fe5a262b558e6eda2753be
Author: Colin Leroy <colin@colino.net>
Date:   Fri Sep 28 10:29:27 2018 +0200

    Fix possible null-dereference (thanks Coverity)
Comment 26 users 2018-09-28 10:46:03 UTC
Changes related to this bug have been committed.
Please check latest Git and update the bug accordingly.
You can also get the patch from:
http://git.claws-mail.org/

++ ChangeLog	2018-09-28 10:46:03.843470201 +0200
http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=e758e47ef9aadef1a8b38d38891f0fe737b74575
Merge: 9b0970f d285bdf
Author: Colin Leroy <colin@colino.net>
Date:   Fri Sep 28 10:46:02 2018 +0200

    Merge branch 'master' of file:///home/git/claws

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=d285bdfc245a9e9ed8dc12a4ff8539dd21544d82
Author: Colin Leroy <colin@colino.net>
Date:   Fri Sep 28 10:33:20 2018 +0200

    Fix bug #3889, "Address and quoted message inconsistent in reply"
    * Fix right-click replying to messages in summaryview
    * Fix corner-case when selected and opened messages are different,
      and one of them is to a mailing-list.