The right-click menu for offline contacts will include an option to add the known fingerprint for a contact's OTR key.

The initial implementation will silently filter out all keystrokes that aren't hex characters.

Patrick thinks this approach might not be confusing, and has suggested to work on an improvement, he said:

"I would expected something like the box going red and an error appearing below it (immediately when an invalid letter is typed) saying that the data must be hexadecimal.

I did not suggest an alert box, especially not on every incorrect input. I do not think waiting for the user to press enter makes sense either if you can display something immediately. Maybe Alex has some ideas of how to make this UI better? This can be fixed in a follow-up."

We could use the occasion to start implementing the same paradigm that is getting introduced into the new Account Creation Dialog.
For input fields that need an helper and validation text, we can use the "info" icon with a descriptive tooltip, and when an error is detected we can swap the icon and permanently trigger the tooltip until the error is resolved.

I can quickly implement this if you like this workflow.
Thoughts?

That's pretty much what I was going for, yeah. It seems reasonable to me.

A little screenshot of the upcoming patch

Here's the updated patch with the improved dialog.
I'd like Magnus to chime in as I've used the occasion to implement a base folder to handle shared assets. Since this style is very much identical to the new account hub and email wizard, and hopefully we'll keep reusing the same style and paradigm, I thought it would be good to set things up right off the bat.

You're suggesting to show a "human fingerprint" image in this dialog.

I have seen the "human fingerprint" icon being shown on Android devices, in apps that allow logging in with the real human fingerprint, and on devices with a sensor for reading physical fingerprints.

In our context, the term fingerprint is used as a metaphor for something that is difficult to fake (like a fingerprint that used to be difficult to fake).

Aren't you worried that by showing an icon that depicts a human fingerprint, users might get confused and might believe we're talking about a real human finger?

Maybe the screen should display a sentence that explains what the fingerprint metaphor means in our context, like:

"The term fingerprint is used as a metaphor for uniquely identifying information."

On my Linux system, the buttons aren't visible (just a few top pixels). It seems the dialog doesn't resize to include all elements.

(In reply to Kai Engert (:kaie:) from comment #9)

I think we should do one of the following:

• either allow the user to enter any combination of spaces into the dialog,
  and ignore all whitespace when doing the verification
• or tell the user in an onscreen message that whitespace should be omitted
  when entering the fingerprint.

Please strip spaces automatically.

Florian mentioned to me on IRC that he had some feedback on this.

Patch updated.

• I'm automatically stripping away all the blank spaces before validating and counting, easier for the user to paste or type the 40 char string.
• I re-positioned the elements to reduce the dialog size and remove extra strings.

(Nit: maybe rename "otr-add-finger-tooltip-error" to "otr-add-finger-popup-error", because it's a popup, not tooltip.)

I'd suggest we keep those as -tooltip- because the markup element is actually a <tooltip>. The method that triggers them in JS is openPopup()

(In reply to Kai Engert (:kaie:) from comment #14)

Thanks, I like the key icon better. You're still including the fingerprint
icon in the patch, should this be removed? (Or do you anticipate we'll need
it later?)

It slipped away, sorry, I'll remove it.

The cancel button says "skip". Looking at bug 1518185 attachment 9035039 [details]
[details] it was set to "skip" in the previous code already. But I don't
understand why. Maybe we should use the standard "cancel" instead.

I agree with replacing it with "Cancel".

You are adding many new CSS files. You have 12 files
{linux,osx,windows}/base/{colors,icons,inputs,tooltips} ... Or do you consider it very likely
that you'll soon need individual platform specific code for other purposes?

This structure will be used by the new Account hub with some per-platform CSS variations. But you're right in suggesting to use only the shared as those variations won't be used soon. I got carried away in preparing files for the future.

I'll push an updated patch soon.

Sorry, I forgot to update the "Cancel" button string.

Right on.
Waiting for feedback from Magnus and Florian before marking it for check-in.

(In reply to Magnus Melin [:mkmelin] from comment #20)

Maybe we should also just drop input (while entering) that is not HEX.

Magnus, this is how the original implementation behaved, but Patrick didn't like it.

Ok, I was just thinking about the error message. It's pretty obscure. I'd imagine not even 1% of users could tell you what HEX is. Any non-HEX is not going to be accepted anyway, so why not just drop those typos?

(In reply to Patrick Cloke [:clokep] from comment #11)

Florian mentioned to me on IRC that he had some feedback on this.

I don't really remember, I think I mostly said that this looks fine if it's triggered by a user action (eg. a click on a button in the UI) but should never be put in the user's face as a reaction to something received on the network.

(In reply to Magnus Melin [:mkmelin] from comment #22)

Ok, I was just thinking about the error message. It's pretty obscure. I'd imagine not even 1% of users could tell you what HEX is. Any non-HEX is not going to be accepted anyway, so why not just drop those typos?

Because it is a confusing user experience to just drop erroneous input arbitrarily. Spaces are not errors, they're expected in situations, but unnecessary for the calculation. I don't feel super strongly about this though.

(In reply to Magnus Melin [:mkmelin] from comment #20)

I think spaces should be allowed, and then you can trim them later.

White spaces are allowed in the field. The trim happens only to properly count the 40 cha for validation, without affecting what the user wrote.

It would actually be nice to use the spaces on input also: when the input
changes, add the space where "needed" so that it shows as expected during
input too whether the user inputs the spaces or not. That way it's clear to
a user whether he is supposed to input space separators or not.

Nice, I like it. I can implement it oninput and onblur and add a blank space once ever 10 cha.

Maybe we should also just drop input (while entering) that is not HEX.

I think we should avoid it.
Usually, preventing specific characters and drop them oninput is fine when handling short strings or number values.
In this case, we're dealing with a 40 char long string, and we can't assume the user will properly type it on the first try. Here's a couple of case scenarios:

1. The user manually types the fingerprint from a piece of paper and looks at the keyboard while doing it, consequentially not noticing that some characters are not accepted.
2. The user paste the fingerprint directly in the field and we strip away non hex characters. At that point it'll be hard to identify which character was removed, and even notice at first glance if the fingerprint is not 39 or 38 characters instead of 40.

    let features =
      "chrome,modal,centerscreen,resizable=no,minimizable=no";

why this change, the original looks < 80ch

Sorry, that's a leftover for when I was defining width and height.

I do think the info signs etc. look very nice (the invalid message - the title - is not so nice though, and probably not workable as it is now, since it would be too long for some localizations), but I'd like to preferably avoid much custom code for showing the error messages, and rely on browser in-built functionality as much as possible.

I agree in general with this approach, but there are some cases like this one where the browser built-in functionality is not enough, causing a bad experience, and risking to confuse the user.

I think it should be fine to have the ok button always active, and on ok, the validation message pops up if you hadn't filled it all.

I disagree with this. We shouldn't allow users to go down a path if that path is not available. That's extra work for us and a misleading and frustrating workflow for the user. Imagine if the user inputs 39 characters instead of 40, keeps clicking on OK, and we keep returning an error message. We're forcing the user to visually count the characters trying to identify where he missed one.

Enabling the OK button only when the input is valid removes that blocking experience. I think we should leverage a "smart" and adaptable tooltip to guide the user in properly typing a complex and long string.

We should also work with toolkit to improve how these things are handled if they do not fit our needs.

Totally agree with this, but in the meantime we should not accept a not so great user experience with the promise of some future improvements.

Thoughts?

(In reply to Alessandro Castellani (:aleca) from comment #26)

Nice, I like it. I can implement it oninput and onblur and add a blank space once ever 10 cha.

Please make that "every 8 characters", that's what's used in other places.

Here's an updated patch with all the suggestions and updates.

• The dialog's title is a simple "Add Fingerprint"
• The title inside the dialog has the contact's email to remind the user which contact he's adding the fingerprint to.
• The description is shorter and more direct.
• The input field has both title and placeholder text

Here's the recap of what happens on typing

• A blank space is added once every 8 characters
• Text is validated at every input, triggering an error message once an invalid character is detected.
• The tooltip with the error message remains visible until the error has been fixed.
• The OK button is disabled until the fingerprint is valid and 40 character in total, excluding blank spaces.

(In reply to Alessandro Castellani (:aleca) from comment #26)

In this case, we're dealing with a 40 char long string, and we can't assume
the user will properly type it on the first try. Here's a couple of case
scenarios:

1. The user manually types the fingerprint from a piece of paper and looks
   at the keyboard while doing it, consequentially not noticing that some
   characters are not accepted.

The current approach doesn't help if he's not looking, since it doesn't way what char. In any case, for both versions, the input is invalid and will need to be re-entered or examined.

2. The user paste the fingerprint directly in the field and we strip away
   non hex characters. At that point it'll be hard to identify which character
   was removed, and even notice at first glance if the fingerprint is not 39 or
   38 characters instead of 40.

If pasting something invalid, that's not something we can help with. He'll have to write a new valid input in there.
For length validation, you'd get that automatically with proper message for html5 input type validation.

I think it should be fine to have the ok button always active, and on ok, the validation message pops up if you hadn't filled it all.

I disagree with this. We shouldn't allow users to go down a path if that
path is not available. That's extra work for us and a misleading and
frustrating workflow for the user. Imagine if the user inputs 39 characters
instead of 40, keeps clicking on OK, and we keep returning an error message.
We're forcing the user to visually count the characters trying to identify
where he missed one.

Well how is he to know why the button can't be clicked? He might very well give up on the dialog since there is no error message shown anywhere in the UI and no way to go ahead. If you let him click the button, then an explanation is easily shown.

Thanks for the feedback and review, here's the updated dialog with some additional changes.

For the dialog description, I merged together title and description to avoid the repetition and the odd feeling you pointed out. With this long description the dialog feels more balanced and plays well with the left icon.

I updated the placeholder, and added a : for the input label.

I'd like to try to convince you in keeping the OK button disabled until the input is valid.
You raised a good point about the user not knowing why the button is disabled, that's why I implemented a character count which updates at every input. That visual hint should remove any confusion and help the user in reviewing the length of the string. What do you think?

Also, and I'm sorry if this is a stupid question, isn't the HTML input validation triggered only on form submit? We're not submitting a form here, so I'm not sure if that would work.

If you still think the button should always be enabled, I will do that and take care of the validation and error messages.

More screenshots for the different states and a patch are coming.

FYI, after last night's OTR commits: If you edit your patch file, and remove
type="application/javascript"
the patch applies.

Thanks Kai for the feedback, I updated the patch.
Let's wait for the green light from Magnus

Here's an updated patch.

Like I wrote in bug 1551133, the HTML5 form validation doesn't currently work in Thunderbird as expected. No error messages are being visualized and the validation needs to be triggered in JS in order to make sense, otherwise it renders the field invalid at first keystroke, or when you dismiss the dialog.

The current patch uses some built-in HTML5 attributes for validation, alongside JS error handling.
The icons now use tooltiptext which is accessible.
The error is visualized underneath the input field if a wrong character is typed.

All the style is self contained into a single CSS, and the UX has been greatly improved thanks to the word counter.

We could revisit this once the HTML5 native form validation features are usable.
(Screenshot to follow)

Sorry Kai, I won't request further UI reviews for this patch.
Magnus, this one doesn't need rebasing as it applies normally to the current trunk.

Kai, does this need to be uplifted to beta or esr68?

Yes, please.
This patch has mostly minor UX and UI changes, not affecting any chat functionality.

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/4a0cb4b952fa
OTR: improve UX of 'Add Finderprint' dialog. r=mkmelin,kaie

TB 69 beta:
https://hg.mozilla.org/releases/comm-beta/rev/fa74c9e9b21e6cdce902e8ab0cb2fb6d57479a3a

TB 68.0 ESR:
https://hg.mozilla.org/releases/comm-esr68/rev/2671a7fcbaeffd2bd80aa3a5af6260768a5e831b