1609928 - Pills created from an address retrieved via LDAP appear in orange

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect, P2)

Description

[Jorg K (CEST = GMT+2)]

Pills created from an address retrieved via LDAP appear in orange.

Comment 1

[Alessandro Castellani [:aleca]]

So, the orange pill happens if an address is not already saved in the Address Book.
This works for local AB, but it doesn't work for LDAP directories as it seems we're not able to check the existence of an email address synchronously.
We need to do it synchronously since we need to style the pill on creation and we can't have a delay.

An easy solution would be do drop the whole "orange if not saved" class and only mark invalid emails in red.
This removes an extra condition that is not accurate, and prevents this visual error.

Also, as a follow up in bug 1601745, we can implement a couple of menu items in the context menu, like "Edit Saved Contact" and "Add to Address Book".

What do you think?

Comment 2

[Alessandro Castellani [:aleca]]

Attachment: 1609928-pill-colour.diff

Tiny diff to fix this issue.

Comment 3

[Jorg K (CEST = GMT+2)]

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

So, the orange pill happens if an address is not already saved in the Address Book.
This works for local AB, but it doesn't work for LDAP directories as it seems we're not able to check the existence of an email address synchronously.

Can you simply remember that the address came from an autocomplete and must therefore have come from some address book? How does this work in TB 68 and before?

What do you think?

Removing the warning colour if the address is not in any AB is not a good idea (if this is what the patch does).

Comment 4

[Alessandro Castellani [:aleca]]

Can you simply remember that the address came from an autocomplete and must therefore have come from some address book?

I can, but that doesn't cover the scenario where a user types or pastes the entire address and hits enter.
That action doesn't pass through the ontextentered listener of the autocomplete, even if the email address was part of the autocomplete since the user typed it or pasted it in its entirety.
We still might have false negatives.

How does this work in TB 68 and before?

There's no validation against AB in the composer header, it only checks if an address is valid or not.
The same issue is present in the message header, as addresses saved in a LDAP directory are not getting the "blue star".

Comment 5

[Alessandro Castellani [:aleca]]

I'd suggest landing this to remove the false negative and get rid of the orange variation, which doesn't quite work well.

After that, I can deal with finding a better UI and workflow to asynchronously validate the pills against LDAP directories in bug 1601745, alongside implementing the ability to edit saved contact cards, as those things are related.

Comment 6

[Richard Marti (:Paenglab)]

Comment on attachment 9127383 [details] [diff] [review]
1609928-pill-colour.diff

This fixes the orange pill issue.

When you re-implement the feature, maybe use a less "dangerous" colour because sending to an address which isn't in a AB isn't as bad as the colour could imply. With the default settings, the next time the address is no more "dangerous" because it's in the Collected Addresses AB. Would the address then be better than the first time?

(In reply to Alessandro Castellani (:aleca) from comment #1)
> Also, as a follow up in bug 1601745, we can implement a couple of menu items in the context menu, like "Edit Saved Contact" and "Add to Address Book".

This was my expectation to synch with the message header Address pill.

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 9127383 [details] [diff] [review]
1609928-pill-colour.diff

Review of attachment 9127383 [details] [diff] [review]:
-----------------------------------------------------------------

Let's take it. r=mkmelin

Comment 8

[Alessandro Castellani [:aleca]]

When you re-implement the feature, maybe use a less "dangerous" colour...

Indeed, I will probably implement a tiny flag, like a coloured dot or something a bit more subtle.
I'll do some mock-ups to upload on bug 1601745 to kick-off the discussion.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7a9663e64dc8
Fix pill colour of addresses retrieved via LDAP. r=mkmelin