We need a new UI for all of the new capabilities coming to the Address Book.

I'll start this bug with a patch to remove the old UI, then Nicolai will be implementing the new design.

I'm not sure what's happening here but when loading the full stack of patches I get this error on startup:

JavaScript error: chrome://messenger/content/msgMail3PaneWindow.js, line 581:
ReferenceError: addressBookTabType is not defined

The app is blank and doesn't finish loading.

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

I'm not sure what's happening here but when loading the full stack of patches I get this error on startup:

JavaScript error: chrome://messenger/content/msgMail3PaneWindow.js, line 581:
ReferenceError: addressBookTabType is not defined

The app is blank and doesn't finish loading.

Okay sorry, I think it is this deleted line from me:

content/messenger/addressbook/addressBookTab.js (content/addressBookTab.js)
phab link

Seems to be important for macOS but not for linux!
The phab rev D142678 is updated to contain the line.

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/07841155090b
Delete button for a contact. r=aleca

Wait, what? This is not fixed.

Oops.

Depends on D143115Differential Revision: https://phabricator.services.mozilla.com/D142678

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/206296511101
Various CSS improvements for the Address Book view and edit card. r=darktrojan

Also fixes "organisation" if no department is specified.

I found a very annoying issue.
It seems that I'm unable to save a new contact:

STR:

• Be in the "All Address Book" list
• Click "New Contact"
• Insert "First Name" and "email"
• Click "Save"

Result:

• A new "contact entry" shows up, but none of the typed info were saved and the contact is completely empty.

Additional issue:

• Be in the "Personal Address Book" list
• Repeat the steps to create a new contact

Result:

• Same empty contact created, but this time the empty list item shows up with a wrong height.

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

I found a very annoying issue.
It seems that I'm unable to save a new contact:
Result:

• A new "contact entry" shows up, but none of the typed info were saved and the contact is completely empty.

I had seen that during my testing last week, but then it was only showing Name fields for contact editing in Daily, so I figured it's still WIP. Now more fields are available, and the problem has remained.

This isn't intended to add tests for all of the new UI, just adjust the tests for the old UI to work with the new UI.

Depends on D146790

Heads up for a needed change, which I think it's a simple fix, but the "Default" radio button shouldn't be visible if only one email address field is currently used.

I looked at the email addresses input table, and the design seems slightly broken.

Currently it is

<!-- These labels seem to be duplicates, and very far away structurally from where they are used. -->
<label id="vcard-email-table-header-type-label" hidden="">Email address</label>
<label id="vcard-email-table-header-email-label" hidden="">Email address</label>
<!-- This label shouldn't have both text content and an aria-label. Moreover, the aria-label is not an appropriate label for a checkbox. -->
<label id="vcard-email-table-header-primary" hidden="" aria-label="Choose your primary email address">Default</label>
...
<table>
  <thead>
    <tr>
      <th scope="col">Type</th>
      <th scope="col">Email address</th>
      <!-- Same issue with the aria-label as above. Also, this is hidden when creating a new contact. -->
      <th scope="col" aria-label="Choose your primary email address" hidden="">Default</th>
    </tr>
  </thead>
  <tbody>
    <!-- I'm not sure why a role of "row" was set. -->
    <tr is="vcard-email" slot="v-email" role="row">
      <td>
        <select class="vcard-type-selection" aria-labelledby="vcard-email-table-header-type-label">
        </select>
      </td>
      <td>
        <input type="email" aria-labelledby="vcard-email-table-header-email-label" />
      </td>
      <td>
        <input type="checkbox" aria-labelledby="vcard-email-table-header-primary" />
      </td>
    </tr>
  </tbody>
</table>

Whereas a simpler design would be (following https://webaim.org/techniques/forms/advanced#multiple)

<!-- No extra labels. -->
<table>
  <thead>
    <tr>
      <th scope="col" id="vcard-email-table-header-type-label">Type</th>
      <th scope="col" id="vcard-email-table-header-email-label">Email address</th>
      <!-- "Default" or "Default email" is a fine label for the checkbox. -->
      <th scope="col" id="vcard-email-table-header-primary">Default</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <td>
        <!-- Labelled by the heading. -->
        <select class="vcard-type-selection" aria-labelledby="vcard-email-table-header-type-label">
        </select>
      </td>
      <td>
        <input type="email" aria-labelledby="vcard-email-table-header-email-label" />
      </td>
      <td>
        <input type="checkbox" aria-labelledby="vcard-email-table-header-primary" />
      </td>
    </tr>
  </tbody>
</table>

Please let me know if there is some specific problem that motivated the current design and maybe I can give a solution.

Also, this bug introduced the class "screen-readers-only" (with an "s" after "reader") and defines it exactly the same in two places https://searchfox.org/comm-central/search?q=screen-readers-only&path=css&case=false&regexp=false

But we also have the class "screen-reader-only" (with no "s" after "reader") with the same rules https://searchfox.org/comm-central/rev/cfb1ede00a8093b57b1643743f84edddb9001a43/mail/themes/shared/mail/messageHeader.css#554

We should just have one class that we share.

(In reply to Henry Wilkes [:henry] from comment #40)

I looked at the email addresses input table, and the design seems slightly broken.
...

Also mentioned in bug 1717632 comment 5, so this can be addressed there instead.

I've kept the title, role, department and organisation fields grouped together. This needed a bit of special handling but it works.

I saw some issues with the URL field.

First: I can't enter thunderbird.net or www.thunderbird.net, it has to be https://thunderbird.net

Second: Save a website for a contact, go to another contact, go back to the first contact and open the edit form. The URL is now changed to https://thunderbird.net with an additional backslash \
I could only reproduce this with a CardDAV addressbook (in my case Google), not with a local one.

Pushed by thunderbird@calypsoblue.org:
https://hg.mozilla.org/comm-central/rev/22c44a417a21
Remove aria-label for addressbook primary email label. r=aleca

Comment on attachment 9278788 [details]
Bug 1762126 - Remove aria-label for addressbook primary email label. r=aleca

[Triage Comment]
Approved for 102.0b1.

Thunderbird 102.0beta1:
https://hg.mozilla.org/releases/comm-beta/rev/de511f8862d5

Wayne mentioned on Matrix that this bug is at least meta-ish. I agree.
We've landed a thousand things here, and we're still creating more satellite bugs which we wouldn't want on the top-level (tb-new-addrbook) bug.
Let's give this bug some more visibility and make it more obvious that lots of (patches and) AB bugs are linked up (only) on this one.

(In reply to Andreas from comment #51)

I saw some issues with the URL field.

Please file a new bug. It's expected that only a real url will be allowed though, not just "www.example.com".

(In reply to Henry Wilkes [:henry] from comment #40)

I looked at the email addresses input table, and the design seems slightly broken.

Currently it is

<!-- These labels seem to be duplicates, and very far away structurally from where they are used. -->
<label id="vcard-email-table-header-type-label" hidden="">Email address</label>
<label id="vcard-email-table-header-email-label" hidden="">Email address</label>
<!-- This label shouldn't have both text content and an aria-label. Moreover, the aria-label is not an appropriate label for a checkbox. -->
<label id="vcard-email-table-header-primary" hidden="" aria-label="Choose your primary email address">Default</label>
...
<table>
  <thead>
    <tr>
      <th scope="col">Type</th>
      <th scope="col">Email address</th>
      <!-- Same issue with the aria-label as above. Also, this is hidden when creating a new contact. -->
      <th scope="col" aria-label="Choose your primary email address" hidden="">Default</th>
    </tr>
  </thead>
  <tbody>
    <!-- I'm not sure why a role of "row" was set. -->
    <tr is="vcard-email" slot="v-email" role="row">
      <td>
        <select class="vcard-type-selection" aria-labelledby="vcard-email-table-header-type-label">
        </select>
      </td>
      <td>
        <input type="email" aria-labelledby="vcard-email-table-header-email-label" />
      </td>
      <td>
        <input type="checkbox" aria-labelledby="vcard-email-table-header-primary" />
      </td>
    </tr>
  </tbody>
</table>

Whereas a simpler design would be (following https://webaim.org/techniques/forms/advanced#multiple)

<!-- No extra labels. -->
<table>
  <thead>
    <tr>
      <th scope="col" id="vcard-email-table-header-type-label">Type</th>
      <th scope="col" id="vcard-email-table-header-email-label">Email address</th>
      <!-- "Default" or "Default email" is a fine label for the checkbox. -->
      <th scope="col" id="vcard-email-table-header-primary">Default</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <td>
        <!-- Labelled by the heading. -->
        <select class="vcard-type-selection" aria-labelledby="vcard-email-table-header-type-label">
        </select>
      </td>
      <td>
        <input type="email" aria-labelledby="vcard-email-table-header-email-label" />
      </td>
      <td>
        <input type="checkbox" aria-labelledby="vcard-email-table-header-primary" />
      </td>
    </tr>
  </tbody>
</table>

Please let me know if there is some specific problem that motivated the current design and maybe I can give a solution.

Yeah it's currently broken.
We agreed on your point.

My issue occured because the slot element is a problem for a table and stuff like

<select>
    <slot></slot>
</select

It's not flattening the HTML tree. Currently it's XML compliant but not HTML. The role="row" is added to for the accessibility tree with a broken table layout.

For more information see:
https://github.com/WICG/webcomponents/issues/59#issuecomment-202330706
https://github.com/WICG/webcomponents/issues/404#issuecomment-202290604

I'd like to get the slot element out. I will file a bug for it with possible solutions and add you if its fine for you.

Edit:

The labels on the top are there because of an issue with the shadow dom borders.
Without the slot element there's no need for the shadow dom and therefore no need for these additional labels.
They should be removed.

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/83ea13ba136d
Hide the border in contact edit fieldset and align the textfields. r=aleca

Comment on attachment 9278948 [details]
Bug 1762126 - Hide the border in contact edit fieldset and align the textfields. r=aleca

[Approval Request Comment]
User impact if declined: On Windows unneeded borders and on all platforms not well aligned textboxes
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): low

(In reply to Nicolai Kasper from comment #59)

I'd like to get the slot element out. I will file a bug for it with possible solutions and add you if its fine for you.

ok, sounds good. The slots also made it a lot harder to understand the area because it splits the DOM into a different order. This also made it harder to access in the Browser Toolbox.

Please also note, from bug 1717632 comment 5:

The "Websites" and "Phone Numbers" similarly have multiple entries, but are structured differently to the "Email Addresses" section. They should be consistent and use a table as well.

I think they also use slots, so maybe you can address this in the same bug as well.

Calling this done.

Comment on attachment 9278948 [details]
Bug 1762126 - Hide the border in contact edit fieldset and align the textfields. r=aleca

[Triage Comment]
Approved for beta

Thunderbird 102.0beta6:
https://hg.mozilla.org/releases/comm-beta/rev/09d200e063d7