Bug 1612369 Comment 3 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

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

::: mail/components/compose/content/addressingWidgetOverlay.js
@@ +125,5 @@
>      // Populate all the recipients with the proper values.
>      // We need to force the focus() on each input to trigger the attachment
>      // of the autocomplete mController.
>      if (msgReplyTo) {
> +      if (msgReplyTo.length) {

Why this double condition?
If the first condition is not null it means that we have addresses to add.
There's not need for this.

@@ +566,5 @@
>  function recipientAddPill(element, automatic = false) {
> +  let container = element.closest(".address-container");
> +  for (let pill of container.getElementsByTagName("mail-address-pill")) {
> +    pill.remove();
> +  }

This shouldn't be here as the recipientAddPill method is called whenever a new pill is created.
Adding this method here removes all the pills whenever a new is typed/created/pasted, which makes the entire area unusable.
Also, calling this method for every added pill is very taxing.

Since this issue is related to the CompFields2Recipients method not clearing previous pills, we could add this at the beginning of that method:

for (let pill of document.querySelectorAll("mail-address-pill")) {
    pill.remove();
}
Review of attachment 9123983 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/compose/content/addressingWidgetOverlay.js
@@ +125,5 @@
>      // Populate all the recipients with the proper values.
>      // We need to force the focus() on each input to trigger the attachment
>      // of the autocomplete mController.
>      if (msgReplyTo) {
> +      if (msgReplyTo.length) {

Why this double condition?
If the first condition is not null it means that we have addresses to add.
There's no need for this.

@@ +566,5 @@
>  function recipientAddPill(element, automatic = false) {
> +  let container = element.closest(".address-container");
> +  for (let pill of container.getElementsByTagName("mail-address-pill")) {
> +    pill.remove();
> +  }

This shouldn't be here as the recipientAddPill method is called whenever a new pill is created.
Adding this method here removes all the pills whenever a new is typed/created/pasted, which makes the entire area unusable.
Also, calling this method for every added pill is very taxing.

Since this issue is related to the CompFields2Recipients method not clearing previous pills, we could add this at the beginning of that method:

for (let pill of document.querySelectorAll("mail-address-pill")) {
    pill.remove();
}

Back to Bug 1612369 Comment 3