Bug 1602431 - Fix and handle focus and selection when removing recipient pills with DEL, BACKSPACE, context delete, or cut. r=mkmelin, ui-r=aleca
☠☠ backed out by a4e036ddf033 ☠ ☠
authorThomas Duellmann <bugzilla2007@duellmann24.net>
Mon, 27 Jan 2020 12:39:04 +0200
changeset 37137 3568d9ea04a66019a373e662ce2a411cb781cfcd
parent 37136 1706fa6a364a9364cfd9e764d4a738350d0b4705
child 37138 b35a643cf2dd5630d8876d85b864d27b006516ce
push id2552
push userclokep@gmail.com
push dateMon, 10 Feb 2020 21:24:16 +0000
treeherdercomm-beta@f95a6f4408a3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmkmelin, aleca
bugs1602431
Bug 1602431 - Fix and handle focus and selection when removing recipient pills with DEL, BACKSPACE, context delete, or cut. r=mkmelin, ui-r=aleca
mail/base/content/mailWidgets.js
mail/components/compose/content/addressingWidgetOverlay.js
--- a/mail/base/content/mailWidgets.js
+++ b/mail/base/content/mailWidgets.js
@@ -1798,17 +1798,17 @@
         `input[is="autocomplete-input"][recipienttype]`
       );
     }
 
     /**
      * Check if the pill is currently in "Edit Mode", meaning the label is
      * hidden and the html:input field is visible.
      *
-     * @returns {boolean} True if the pill is currently being edited.
+     * @return {boolean} true if the pill is currently being edited.
      */
     get isEditing() {
       return !this.emailInput.hasAttribute("hidden");
     }
 
     get fragment() {
       if (!this.constructor.hasOwnProperty("_fragment")) {
         this.constructor._fragment = MozXULElement.parseXULToFragment(`
@@ -1956,16 +1956,46 @@
       this.style.removeProperty("max-width");
       this.style.removeProperty("min-width");
       this.classList.remove("editing");
       this.pillLabel.removeAttribute("hidden");
       this.emailInput.setAttribute("hidden", "hidden");
       this.rowInput.focus();
     }
 
+    /**
+     * Get the nearest sibling pill which is not selected.
+     *
+     * @param {("next"|"previous")} [siblingsType="next"] - Iterate next or
+     *   previous siblings.
+     * @return {HTMLElement} - The nearest unselected sibling element, or null.
+     */
+    getUnselectedSiblingPill(siblingsType = "next") {
+      if (siblingsType == "next") {
+        // Check for next siblings.
+        let element = this.nextElementSibling;
+        while (element) {
+          if (!element.hasAttribute("selected")) {
+            return element;
+          }
+          element = element.nextElementSibling;
+        }
+        return null;
+      }
+      // Check for previous siblings.
+      let element = this.previousElementSibling;
+      while (element) {
+        if (!element.hasAttribute("selected")) {
+          return element;
+        }
+        element = element.previousElementSibling;
+      }
+      return null;
+    }
+
     removeObserver() {
       Services.obs.removeObserver(
         this.inputObserver,
         "autocomplete-did-enter-text"
       );
     }
   }
 
@@ -1992,17 +2022,17 @@
         setupAutocompleteInput(input, this.highlightNonMatches);
       }
     }
 
     /**
      * Create a new recipient row container with the input autocomplete.
      *
      * @param {Array} recipient - The unique identifier of the email header.
-     * @returns {XULElement} - The newly created recipient row.
+     * @return {Element} - The newly created recipient row.
      */
     buildRecipientRows(recipient) {
       let row = document.createXULElement("hbox");
       row.setAttribute("id", recipient.row);
       row.classList.add("addressingWidgetItem", "address-row");
 
       let firstCol = document.createXULElement("hbox");
       firstCol.classList.add("aw-firstColBox");
@@ -2172,35 +2202,46 @@
       params.type = element.getAttribute("recipienttype");
       pill.emailInput.setAttribute(
         "autocompletesearchparam",
         JSON.stringify(params)
       );
     }
 
     /**
-     * Move the focus on the first pill from the same .address-container.
+     * Handle keypress event on a pill in the mail-recipients-area.
      *
-     * @param {XULElement} pill - The mail-address-pill element.
+     * @param {Element} pill - The mail-address-pill element where Event fired.
      * @param {Event} event - The DOM Event.
      */
     handleKeyPress(pill, event) {
       if (pill.isEditing) {
         return;
       }
 
       switch (event.key) {
         case "Enter":
         case "F2": // For Windows users
           this.startEditing(pill, event);
           break;
 
         case "Delete":
         case "Backspace":
-          this.removePills(pill);
+          // We must never delete a focused pill which is not selected.
+          // If no pills selected, just select the focused pill.
+          // For rapid repeated deletions (esp. from holding BACKSPACE),
+          // stop before selecting another focused pill for deletion.
+          if (!this.hasSelectedPills() && !event.repeat) {
+            pill.setAttribute("selected", "selected");
+            break;
+          }
+          // Delete selected pills, handle focus and select another pill
+          // where applicable.
+          let focusType = event.key == "Delete" ? "next" : "previous";
+          this.removeSelectedPills(pill, focusType, true);
           break;
 
         case "ArrowLeft":
           if (pill.previousElementSibling) {
             pill.previousElementSibling.focus();
             this.checkKeyboardSelected(event, pill.previousElementSibling);
           }
           break;
@@ -2244,17 +2285,17 @@
           if (event.ctrlKey || event.metaKey) {
             copyEmailNewsAddress(pill);
           }
           break;
 
         case "x":
           if (event.ctrlKey || event.metaKey) {
             copyEmailNewsAddress(pill);
-            deleteAddressPill(pill);
+            this.removeSelectedPills(pill);
           }
           break;
       }
     }
 
     /**
      * Handle the selection and focus of the pill elements on mouse events.
      *
@@ -2293,17 +2334,20 @@
         if (this.hasAttribute("selected") && element.hasAttribute("selected")) {
           this.removeAttribute("selected");
           return;
         }
 
         this.setAttribute("selected", "selected");
         element.setAttribute("selected", "selected");
       } else if (!event.ctrlKey) {
+        // Non-modified navigation keys must select the target pill and deselect
+        // all others. Also some other keys like Backspace from rowInput.
         this.clearSelected();
+        element.setAttribute("selected", "selected");
       }
     }
 
     clearSelected() {
       for (let pill of this.getAllPills()) {
         pill.removeAttribute("selected");
       }
     }
@@ -2319,33 +2363,71 @@
         event.stopPropagation();
         return;
       }
 
       pill.startEditing();
     }
 
     /**
-     * When a "Delete" action is triggered, we need to check if other pills are
-     * currently selected and delete them all.
+     * Delete all selected pills and handle focus and selection smartly as needed.
      *
-     * @param {XULElement} pill - The mail-address-pill element.
+     * @param {Element} pill - The mail-address-pill element where Event fired.
+     * @param {("next"|"previous")} [focusType="next"] - How to move focus after
+     *   removing pills: try to focus one of the next siblings (for DEL etc.)
+     *   or one of the previous siblings (for BACKSPACE).
+     * @param {boolean} [select=false] - After deletion, whether to select the
+     *   focused pill where applicable.
      */
-    removePills(pill) {
-      // Store the addressing input that needs to receive focus before deleting
-      // the pill.
+    removeSelectedPills(pill, focusType = "next", select = false) {
+      // We'll look hard for an appropriate element to focus after the removal.
+      let focusElement = null;
+      // Get addressContainer and rowInput now as pill might be deleted later.
+      let addressContainer = pill.closest(".address-container");
       let rowInput = pill.rowInput;
-      for (let item of this.getAllSelectedPills()) {
-        item.remove();
+      let unselectedSourcePill = false;
+      if (pill.hasAttribute("selected")) {
+        // Find focus (1): Focused pill is selected and will be deleted;
+        // try nearest sibling, observing focusType direction.
+        focusElement = pill.getUnselectedSiblingPill(focusType);
+      } else {
+        // The source pill isn't selected; keep it focused ("satellite focus").
+        unselectedSourcePill = true;
+        focusElement = pill;
       }
 
-      rowInput.focus();
-      // Manually remove the pill in case it wasn't part of the selected array.
-      pill.remove();
+      // Remove selected pills.
+      let selectedPills = this.getAllSelectedPills();
+      for (let sPill of selectedPills) {
+        sPill.remove();
+      }
 
+      // Find focus (2): When deleting backwards, if no previous sibling found,
+      // this means that the first pill was deleted. Try the first remaining pill,
+      // but don't auto-select it because it's in the opposite direction.
+      if (!focusElement && focusType == "previous") {
+        focusElement = addressContainer.querySelector("mail-address-pill");
+      } else if (
+        select &&
+        focusElement &&
+        selectedPills.length == 1 &&
+        !unselectedSourcePill
+      ) {
+        // If select = true (DEL or BACKSPACE), and we found a pill to focus in
+        // round (1), and we have removed a single pill only, and it's not a
+        // case of "satellite focus" (see above):
+        // Conveniently select the nearest pill for rapid consecutive deletions.
+        focusElement.setAttribute("selected", "selected");
+      }
+      // Find focus (3): If all else fails (no pills left in addressContainer,
+      // or last pill deleted forwards): Focus rowInput.
+      if (!focusElement) {
+        focusElement = rowInput;
+      }
+      focusElement.focus();
       onRecipientsChanged();
       calculateHeaderHeight();
     }
 
     /**
      * Move the focus on the first pill from the same .address-container.
      *
      * @param {XULElement} pill - The mail-address-pill element.
@@ -2389,12 +2471,21 @@
     /**
      * Return all the selected pills currently available in the address area.
      *
      * @return {Array} Array of selected mail-address-pill elements.
      */
     getAllSelectedPills() {
       return this.querySelectorAll("mail-address-pill[selected]");
     }
+
+    /**
+     * Check if any pill in the addressing area is selected.
+     *
+     * @return {boolean} true if any pill is selected.
+     */
+    hasSelectedPills() {
+      return Boolean(this.querySelector("mail-address-pill[selected]"));
+    }
   }
 
   customElements.define("mail-recipients-area", MailRecipientsArea);
 }
--- a/mail/components/compose/content/addressingWidgetOverlay.js
+++ b/mail/components/compose/content/addressingWidgetOverlay.js
@@ -683,35 +683,25 @@ function copyEmailNewsAddress(element) {
  *   opened.
  */
 function cutEmailNewsAddress(element) {
   copyEmailNewsAddress(element);
   deleteAddressPill(element);
 }
 
 /**
- * Delete the selected pill/pills.
+ * Delete the selected pill(s).
  *
  * @param {XULElement} element - The element from which the context menu was
  *   opened.
  */
 function deleteAddressPill(element) {
-  // We need to store the input location before removing the pills.
-  let input = element
-    .closest(".address-container")
-    .querySelector(`input[is="autocomplete-input"][recipienttype]`);
-
-  for (let pill of document
-    .getElementById("recipientsContainer")
-    .getAllSelectedPills()) {
-    pill.remove();
-  }
-
-  input.focus();
-  onRecipientsChanged();
+  // element is the pill's <label>, get the pill.
+  let pill = element.closest("mail-address-pill");
+  document.getElementById("recipientsContainer").removeSelectedPills(pill);
 }
 
 /**
  * Handle the keypress event on the labels to show the container row
  * of an hidden recipient (Cc, Bcc, etc.).
  *
  * @param {Event} event - The DOM keypress event.
  * @param {XULelement} label - The clicked label to hide.