In Outlook it is not easy to see whether encryption is enabled (grey background). It is too easy to accidentally click ion the icon and thus send a mail in the clear. A better visual feedback is needed. For example to use a different icon if encryption is not enable (replace the blue by grey or at least remove the small badge.
Description
Revisions and Commits
Related Objects
- Mentioned Here
- T7144: GpgOL: Escalate errors when setting crypto flag fails
Event Timeline
Changing the icon is unusual and does not match a native look and feel in Outlook where toggle icons are there for a reason, to be toggled or not. This is also the way how Outlooks native encrypt & sign works and Microsoft will probably have thought about this a bit.
Technically I also do not think that this is as easy as just changing the code in getIcon to isButtonPressed (or what the callback was called) because it will require invalidating the UI in all windows, that might cause flickers and you can't do it in the code where the UI updates as this would then loop. While toggling buttons is a supported concept, changing the icons in the Ribbon depending on the state is a hack which I thought of to have a UI element for signature display. And there I had all the problems that I needed to ensure an invalidation was done. Triggering invalidations was also often times associated with crashes. So there is a lot of fragility involved in the Invalidate UI code where we collect "pending invalidations" and fire them asynchronusly after udating the crypto status since too many invalidations in a short time period appeared to cause crashes.
So my strong recommendation would be to leave that alone, as I do not agree that this is a confusing or strange element, i have not gotten much bad feedback about this and changing the interface of something where users are used to is often not well received. Additionally I think that you will encounter problems with this, which has been my experience of changing the icons based on the decrypt / verify result.
In gpgoladdin:
/* RibbonUi elements are created on demand but they are reused in different inspectors. So far and from all documentation I could find RibbonUi elments are never deleted. When they are created the onLoad callback is called to register them. The callbacks registered in the XML description are only executed on Load. So to have different information depending on the available mails we have to invalidate the UI ourself. This means that the callbacks will be reevaluated and the UI Updated. Sadly we don't know which ribbon_ui needs updates so we have to invalidate everything. */ void gpgoladdin_invalidate_ui () { GpgolAddin::get_instance ()->invalidateRibbons(); }
And further in windowmessages.h
/** Block ui invalidation. The idea here is to further reduce UI invalidations because depending on timing they might crash. So we try to block invalidation for as long as it is a bad time for us. */ void blockInv (); /** Unblock ui invalidation */ void unblockInv (); DWORD WINAPI delayed_invalidate_ui (LPVOID minsleep_ms = 0);
A better solution might be to use categories to have that element "this message will be signed / this message will be encrypted" above the edit window. But what I find more important and so much more a high priority is that in cases we have a failure saving the draft info flags an error message should come up. This happened for a customer and in the logs I could see that MAPI returned an error. the button was not toggled in this case but the mail also was not marked for encryption. T7144 is the task for that so I'd suggest to start with that one.
I added some comments to the commit. But
To repeat:
- Toggle buttons indicate what will be done -> In this case a mail will be either secured or not.
- Users are used to our toggle button so there is absolutely no need to reopen such an issue right now.
- Changing anything here, everyone (at least at some federal agencies) will have an opinion.
- If the Icon should convey information we would need a new variant that works with different accessibility things like light and dark. e.g. in Dark mode you cannot really see if the lock is locked.
- If we want the button to convey state, the only logical way in my opinion would be to use the crypto status icons to indicate what will happen with that mail. E.g. if it is not signed and not encrypted it should read "inescure" with the question mark, instead of displaying an open lock.
- People might be using S/MIME Encryption in Outlook for 20 years or so, and they use toggle buttons.
If the text is not changed it also makes not much sense to change the icon. If the text changes it will become even more confusing.
So apologies marcus, nothing against the code. But I completely reject this change if it only changes the icon. It is just deeply saddening that your first assignment was to do something which i refused to do. If there was one part on Gpg4win where I would have said that I had the last word it would have been GpgOL and I feel like I am wasting my time but I cannot shrug this off since this button _is_ the GpgOL UI.
Note: The code for this is in the work/mmontkowski branch but has not yet been merged with master. Before we take this bug up again, we need to look closer at the ribbon UI events as remarked by Andre on July 29.