| Summary: | KMail doesn't honor the "always sign with GPG" flag in idendity when replying | ||
|---|---|---|---|
| Product: | [Applications] kmail2 | Reporter: | Christian Boltz <kde-bugs> |
| Component: | crypto | Assignee: | kdepim bugs <kdepim-bugs> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | gjditchfield, montel, sknauss |
| Priority: | NOR | ||
| Version: | 5.1 | ||
| Target Milestone: | --- | ||
| Platform: | Compiled Sources | ||
| OS: | Linux | ||
| Latest Commit: | http://commits.kde.org/kdepim/385638c6b459fc93845cafb2b336b16ae8c0623e | Version Fixed In: | |
| Sentry Crash Report: | |||
| Attachments: |
patch
A test for this bug |
||
|
Description
Christian Boltz
2016-01-03 18:10: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
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. 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. 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.
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 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. Created attachment 96943 [details]
A test for this bug
Please add this patch to you build and run kmcommandtest. Does it fail?
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 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... 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 ;-) 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 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! |