<table><tr><td style="">aheinecke added a subscriber: emanuel.<br />aheinecke added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D3140" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>Great, I think it's nice to finally have an application/pgp-keys handler. But as written in the inline comments I think that if we have a handler we need to offer the import action.</p>

<p>Regarding the WKS Handler I think the "This key is already published" message / check if the key is already in the WKD is probably wrong, there might be use cases (e.g. after changing the expiry date of a key or adding / removing subkeys) where a user wants to update the published key even if a key with the same fingerprint was already published.<br />
I would have thought we could handle this like invitations. So that after accepting an invitation the actual invitation mail is deleted. (Here after replying to a confirmation request we would delete the request). This would remove the need here problem to check if the key is already published.</p>

<p>Regarding sending, you use SMTPJob. I am concerned that this might be a bit too low level.  It looks to me for example that if you are offline when trying to reply the SMTPJob would just error out where I expected that the response stays in the outbox and is sent the next time you are online. Again invitations could be the example how to handle this.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D3140#inline-12180" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dvratil</span> wrote in <span style="color: #4b4d51; font-weight: bold;">gnupgwksmemento.cpp:47</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p style="padding: 0; margin: 8px;">Perhaps it will good to detect if we have gpg2 in system no ?</p></blockquote>

<p style="padding: 0; margin: 8px;">It's a bit more difficult than that. Some distros have GnuPG2 binaries called "gpg", while some others have GnuPG2 binaries called "gpg2" (and GnuPG1 binaries called "gpg", possibly installed at the same time). We need to use GnuPG2 for this, so we try "gpg2" first, and if we get <tt style="background: #ebebeb; font-size: 13px;">QProcess::FailedToStart</tt> error we try "gpg". If we would get error with "gpg", it means that either there's no "gpg" binary, or it's GnuPG1 binary (which does not understand the arguments).</p>

<p style="padding: 0; margin: 8px;">I think it's easier and simpler to just try, rather than locating the binaries and parsing their "--version" output.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Ultimately I think we need API in gpgme for this. But in the meantime its better to use GpgME engine info:</p>

<p style="padding: 0; margin: 8px;">GpgME::engineInfo(GpgME::GpgEngine).fileName();</p>

<p style="padding: 0; margin: 8px;">There is also GpgME::engineInfo(GpgME::GpgEngine).engineVersion() that you could use for version checks like:</p>

<p style="padding: 0; margin: 8px;">if (!(GpgME::engineInfo(GpgME::GpgEngine).engineVersion() < "2.1.13"))<br />
...</p>

<p style="padding: 0; margin: 8px;">(2.1.13 is the first version that fully supports wkd lookup)</p>

<p style="padding: 0; margin: 8px;">But as written in the general comment I don't think we should do the check here but rather delete the confirmation request mail once we have sent a response.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D3140#inline-12188" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">pgpkeyformatter.cpp:93</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">    <span style="color: #aa4000">const</span> <span style="color: #aa4000">auto</span> <span class="n">key</span> <span style="color: #aa2211">=</span> <span class="n">mp</span><span class="p">.</span><span class="n">key</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">    <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">key</span><span class="p">.</span><span class="n">isNull</span><span class="p">())</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">        <span class="n">block</span><span class="p">.</span><span class="n">setProperty</span><span class="p">(</span><span style="color: #766510">"uid"</span><span class="p">,</span> <span class="n">mp</span><span class="p">.</span><span class="n">userID</span><span class="p">());</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">In this case we should offer to import the key. So that we handle the case where someone attaches their PGP key to a message and the recipient imports it. QGpgME::ImportJob</p>

<p style="padding: 0; margin: 8px;">This would also mean to store the raw data in the PgpKeyMessagePart. I think without this we might have a regression because application/pgp-keys was previously associated with kleopatra so you could import an attached public key by "Opening the attachment in Kleopatra".</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>rKDEPIMADDONS KDE PIM Addons</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D3140" rel="noreferrer">https://phabricator.kde.org/D3140</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>dvratil, bjoernbalazs, mlaurent, aheinecke<br /><strong>Cc: </strong>emanuel, mlaurent, kde-pim, KDE PIM, spencerb, dvasin, winterz, vkrause, knauss, dvratil<br /></div>