1771575 - Restore "Required Information Missing" dialog to prevent saving empty contacts

Status: RESOLVED FIXED

(Thunderbird :: Address Book, defect, P1)

Description

[Thomas D. (:thomas8)]

Attachment: Screenshot 1: 'Required Information Missing' dialog on TB 91 (when trying to save empty contact)

TB 91 won't allow you to save an empty contact, which makes sense.
Also, empty contacts look extremely truncated in TB 102.

Expected

• Prevent saving empty contacts
• Restore "Required Information Missing" dialog to prevent saving empty contacts

Just disabling the Save button would have the disadvantage that the user may not know why the button is disabled. The dialog is good as it calls out the fields of which one must be filled before the contact can be saved. The list of fields make sense to me.

Needs strings!

Comment 1

[Thomas D. (:thomas8)]

Attachment: Screenshot 2: Ugly and useless empty contacts in TB 102

Empty contacts don't look nice on 102.0a1 (2022-05-27) (64-bit), Win10.

Comment 2

[Thomas D. (:thomas8)]

Comment on attachment 9278587 [details]
Screenshot 1: 'Required Information Missing' dialog on TB 91 (when trying to save empty contact)

.

Comment 3

[Alessandro Castellani [:aleca]]

Rather than showing a dialog, the form should have some default required fields to leverage the built-in HTML5 form validation, and go through that before saving the contact.
We might also consider disabling the "Save" button until the form is valid.
I'll take care of this.

Comment 4

[Thomas D. (:thomas8)]

Mass-adding regression keyword to applicable AB bugs.

#20220530ABRegressionKeyword01

Comment 5

[Thomas D. (:thomas8)]

(In reply to Alessandro Castellani [:aleca] from comment #3)

Rather than showing a dialog, the form should have some default required fields to leverage the built-in HTML5 form validation, and go through that before saving the contact.

I don't think that's a good solution, unless you have a way of natively requiring that from a range of fields, only one must be filled. We don't want to force users to enter data which they may not have or are not willing to add at this point.

As the current error message suggests (see screenshot above), it looks totally sufficient if any one of the following fields is filled:
Email Address, First Name, Last Name, Display Name, Organization.

https://searchfox.org/comm-central/rev/3a39d40958129613df4353604f04c9d32519e848/mail/locales/en-US/chrome/messenger/addressbook/addressBook.properties#28

cardRequiredDataMissingMessage=You must enter at least one of the following items:\nEmail Address, First Name, Last Name, Display Name, Organization.

We might also consider disabling the "Save" button until the form is valid.

While this might look like a superior solution, imo it is not. Again assuming that required isn't what we want here because it's way too strict, it's actually much clearer to to the user to just show an explicit error message. I think this error message won't happen very often - saving an empty contact is really just a rare accident, maybe when you got interrupted and don't realize you have entered nothing.
I think we're casting the error message so that you don't end up in a false belief that you have actually saved a contact with data.
The error message is much clearer as it explicitly tells the user what's wrong and how to fix it. I don't think any required-markup like red borders on fields could do this, because how would you indicate that only one out of several marked fields must be filled?

I'll take care of this.

Thank you.

Comment 6

[Thomas D. (:thomas8)]

^^ comment 5 fyi before you start fixing this

Comment 8

[Alessandro Castellani [:aleca]]

Can you please stop NI me on every single bug I'm already part of?
I'm already CC'ed on these and I get a notification anywhere.
My inbox is getting spammed by hundreds of repeated AB bugs for no reason and it's getting disrupting.

I don't think that's a good solution, unless you have a way of natively requiring that from a range of fields, only one must be filled. We don't want to force users to enter data which they may not have or are not willing to add at this point.

That's exactly how the native HTML5 form validation works.

I think this error message won't happen very often - saving an empty contact is really just a rare accident

Exactly for this reason, an entire alert dialog is not really necessary.
Native form validation will take care of highlighting the needed fields (or only 1 if we only care about it), and using that will better integrate with assistive technologies being able to handle the very simple "email address field is required" kind of built-in message.

Comment 9

[Martin Giger [:freaktechnik]]

Attachment: Bug 1771575 - Make sure contact items always have the full height. r=#thunderbird-reviewers

Contacts might not have any name and email address set, leading to an empty row for the contact. Not only can you currently create such contacts, but remote sources or imported address books might also contain such contacts.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b7dea7e10840
Make sure contact items always have the full height. r=aleca

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a9a9b9b1cba5
follow-up - Fix a broken test. rs=bustage-fix

Comment 12

[Alessandro Castellani [:aleca]]

Comment on attachment 9280077 [details]
Bug 1771575 - Make sure contact items always have the full height. r=#thunderbird-reviewers

[Approval Request Comment]
User impact if declined: Wrong UI alignment of contact lists with only one row of data.
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): low

IMPORTANT
This needs to land alongside this bustage fix follow up: https://hg.mozilla.org/comm-central/rev/a9a9b9b1cba555dd7394059087e2b1efb4fb8848

Comment 13

[Wayne Mery (:wsmwk)]

Comment on attachment 9280077 [details]
Bug 1771575 - Make sure contact items always have the full height. r=#thunderbird-reviewers

[Triage Comment]
Approved for beta

Rob, note comment 12

Comment 14

[Rob Lemley [:rjl]]

Thunderbird 102.0b5:
https://hg.mozilla.org/releases/comm-beta/rev/aabeb14f6fbf
https://hg.mozilla.org/releases/comm-beta/rev/4525ddb7a1ae

Comment 15

[Alessandro Castellani [:aleca]]

Attachment: Bug 1771575 - Restore "Required Information Missing" dialog to prevent saving empty contacts. r=darktrojan

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8d46927a1990
Restore "Required Information Missing" dialog to prevent saving empty contacts. r=darktrojan

Comment 17

[Rob Lemley [:rjl]]

Does this need to go on beta?

(In reply to Pulsebot from comment #16)

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8d46927a1990
Restore "Required Information Missing" dialog to prevent saving empty contacts. r=darktrojan

Comment 18

[Alessandro Castellani [:aleca]]

Comment on attachment 9281343 [details]
Bug 1771575 - Restore "Required Information Missing" dialog to prevent saving empty contacts. r=darktrojan

[Approval Request Comment]
User impact if declined: Users can accidentally save an empty contact
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): low

Comment 19

[Alessandro Castellani [:aleca]]

(In reply to Rob Lemley [:rjl] from comment #17)

Does this need to go on beta?

Yup, just requested the uplift.

Comment 20

[Wayne Mery (:wsmwk)]

Comment on attachment 9281343 [details]
Bug 1771575 - Restore "Required Information Missing" dialog to prevent saving empty contacts. r=darktrojan

[Triage Comment]
Approved for beta

Comment 21

[Rob Lemley [:rjl]]

Thunderbird 102.0beta7:
https://hg.mozilla.org/releases/comm-beta/rev/e1b91f9020ae