Bug 357481 - KMail doesn't honor the "always sign with GPG" flag in idendity when replying
Summary: KMail doesn't honor the "always sign with GPG" flag in idendity when replying
Status: RESOLVED FIXED
Alias: None
Product: kmail2
Classification: Applications
Component: crypto (show other bugs)
Version: 5.1
Platform: Compiled Sources Linux
: NOR normal (vote)
Target Milestone: ---
Assignee: kdepim bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-03 18:10 UTC by Christian Boltz
Modified: 2019-09-21 14:33 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
patch (820 bytes, patch)
2016-01-03 19:40 UTC, Christian Boltz
Details
A test for this bug (12.43 KB, patch)
2016-01-31 14:23 UTC, Sandro Knauß
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Boltz 2016-01-03 18:10:03 UTC
KMail 4.14.10 on KDE 4.14.14 on openSUSE Tumbleweed

In one of my idendities, I have set my PGP key and enabled the "Automatically sign messages" option. Also, I have assigned that idendity for one IMAP account and selected a folder inside that account.

When writing a _new_ message, the editor opens with the "Sign message" button pre-selected - as expected. Good.

However, when I reply to a message, the editor always opens with the "Sign message" button _not_ selected. Bad. This even happens when replying to signed mails, which enables the "Sign message" button in other folders if using an idendity without the "Automatically sign message" setting. Bad again.

Reproducer:
- setup an idendity with "Automatically sign messages" and your GPG key
- assign that idendity to an IMAP account (assigning to a folder might be enough)
- store at least two mails in that folder, one GPG-signed, one not signed.
- reply to both mails
- in the mail editor, check if the "Sign message" button is pre-selected

Expected behaviour:
The mail editor should have the "Sign message" button pre-selected because of the idendity settings (remember: the idendity has "Automatically sign message" set)
Comment 1 Christian Boltz 2016-01-03 19:40:03 UTC
Created attachment 96433 [details]
patch

This patch seems to work - "Sign message" is now always enabled in the composer when starting a new mail or a reply in a folder using an idendity with "Automatically sign messages". Also, when replying to a signed mail, "Sign message" still (and now always) gets enabled.

Notes:
- I tested this patch with various combinations (folder using an idendity with / without "Automatically sign messages", signed / unsigned mails, new mail / reply), and it seems to do the expected thing everywhere
- I'm not a C or C++ developer, so please double-check the patch - even if it's short and seems to work, it's still copy&paste-programming ;-)
- I don't have (and don't want, sorry) write access to the KDE git, so please someone take the patch and apply it
Comment 2 Sandro Knauß 2016-01-30 21:29:21 UTC
The patch looks like straight forward.
Please prepare a patch for current kf5 version and apply he syntax for that and upload it to phabricator/reviewboard.
If you have any problems, don't hasitate to ask.
Comment 3 Christian Boltz 2016-01-30 23:36:55 UTC
For the records: KMail 5.1.1 is more consistent - it never enables the GPG signing ;-)
Therefore this patch is still needed in KMail 5.1.1 - and still works with it.

As I said on IRC, I'm not familiar with phabricator etc. and am too busy to learn it (sorry!), so I'd prefer if you can push this patch.
Comment 4 Laurent Montel 2016-01-31 07:10:26 UTC
Two line after your patch there is :
    mLastSignActionState = ident.pgpAutoSign();

so what is the diff between your patch and actual code ?

But indeed we need a patch from reviewboard or codereview so we will see these lines.

Regards.
Comment 5 Christian Boltz 2016-01-31 12:42:35 UTC
The line you mentioned is line 1310, but my patch applies around line 1644 *). I'm not sure what exactly happens, but I'd guess something is overwriting/setting mLastSignActionState to false somewhere in between.

That said - the difference between my patch and the actual code is simple: With my patch, it works - without, it doesn't ;-)  (tested with KMail 5.1.1 aka kdepim-15.12.1.tar.xz)


*) line numbers fro git as shown on https://quickgit.kde.org/?p=kdepim.git&a=blob&h=dd45b50a50934ac22c05c240d883f3acfcd2a477&hb=1af4bde89d080692b7dbb90ce00e4f5136af2144&f=kmail%2Feditor%2Fkmcomposewin.cpp
Comment 6 Sandro Knauß 2016-01-31 14:12:29 UTC
Unfortunatelly the problem is not that easy to track down than I thought *sigh*
I created now a test to have a way to reproduce the issue with a automated test, but there the autoSign value is taken correctly.

So there must be an additonal thing hapepen, that triggers your bug. Can you please test:

in the setMessage function:
qDebug() << newMsg->head();

and additionally the question what command you are using (kmail/kmcommands.cpp).

slotIdentityChanged(mId, true /*initalChange*/); should actually set everything correctly. But maybe we have a wrong uoid and that's why everything is screwed up.
This would be great.
Comment 7 Sandro Knauß 2016-01-31 14:23:35 UTC
Created attachment 96943 [details]
A test for this bug

Please add this patch to you build and run kmcommandtest. Does it fail?
Comment 8 Christian Boltz 2016-01-31 20:48:44 UTC
Just for the records: a debugging session on IRC (thanks for doing that!) showed that
- the test works (fails without my patch, and succeeds with it)
- the test needs GPG signing enabled in the default idendity to work
- my patch fixes the issue
Comment 9 Sandro Knauß 2016-02-01 14:30:38 UTC
Okay I now managed to create a fully working test. I use another way to fix this. slotIdentityChange already sets the correct value for signature, but this value is not honord later one. So I save the state after changing the identity.

https://phabricator.kde.org/D892
and cleanup around
https://phabricator.kde.org/D891

these are for master, not for 5.1.1. in 5.1.1 there is also the sticky Identity handling, that makes the patch apply harder. But if you wan't to test on base of 5.1.1 - feel free to ask...
Comment 10 Christian Boltz 2016-02-01 18:41:21 UTC
I don't want to force you to do superfluous backporting work (better spend your time on fixing other bugs;-) )  so I'll trust you that you got everything right. 

Just tell me which version will have the fixed code, and I'll report back as soon as it is released and packaged for openSUSE Tumbleweed ;-)
Comment 11 Sandro Knauß 2016-02-10 13:55:16 UTC
Git commit 385638c6b459fc93845cafb2b336b16ae8c0623e by Sandro Knauß.
Committed on 10/02/2016 at 13:35.
Pushed by knauss into branch 'master'.

Fixes 357481: honor the "always sign with GPG" flag in idendity

added testcase

Differential revision: https://phabricator.kde.org/D892

M  +2    -1    kmail/autotests/CMakeLists.txt
A  +242  -0    kmail/autotests/kmcommandstest.cpp     [License: GPL (v2+)]
A  +45   -0    kmail/autotests/kmcommandstest.h     [License: GPL (v2+)]
M  +3    -2    kmail/editor/composer.h
M  +4    -0    kmail/editor/kmcomposewin.cpp
M  +3    -1    kmail/kmmainwin.h
M  +3    -1    kmail/kmreadermainwin.h
M  +3    -1    kmail/kmreaderwin.h

http://commits.kde.org/kdepim/385638c6b459fc93845cafb2b336b16ae8c0623e
Comment 12 Christian Boltz 2016-06-05 16:34:27 UTC
I finally upgraded to KMail 5.2.1 (from KDE Applications 16.04.1), and the signing defaults work as expected :-)

Thanks again for fixing this!