Bug 1634889 - Fix composition's fast focus ring (Ctrl+[Shift+]+Tab / [Shift+]F6]). r=aleca
authorThomas Duellmann <bugzilla2007@duellmann24.net>
Fri, 01 May 2020 08:52:55 +0200
changeset 39078 1defa8e63d750243cd404c379fa780980bf59364
parent 39077 3edf0643a1d84d2ee647912078e5a236a8abc6c2
child 39079 ac2e45bcc600711a6871f81d4781158816e69e1b
push id402
push userclokep@gmail.com
push dateMon, 29 Jun 2020 20:48:04 +0000
reviewersaleca
bugs1634889
Bug 1634889 - Fix composition's fast focus ring (Ctrl+[Shift+]+Tab / [Shift+]F6]). r=aleca
mail/base/content/mailWidgets.js
mail/components/compose/content/MsgComposeCommands.js
mail/test/browser/composition/browser_focus.js
--- a/mail/base/content/mailWidgets.js
+++ b/mail/base/content/mailWidgets.js
@@ -2402,17 +2402,17 @@
           pill.rowInput.focus();
           break;
 
         case "Tab":
           event.preventDefault();
           for (let item of this.getSiblingPills(pill)) {
             item.removeAttribute("selected");
           }
-          if (event.shiftKey) {
+          if (event.shiftKey && !event.ctrlKey) {
             this.moveFocusToPreviousElement(pill);
             return;
           }
           pill.rowInput.focus();
           break;
 
         case "a":
           if (!(event.ctrlKey || event.metaKey) || event.repeat) {
--- a/mail/components/compose/content/MsgComposeCommands.js
+++ b/mail/components/compose/content/MsgComposeCommands.js
@@ -7549,63 +7549,77 @@ function GetMsgHeadersToolbarElement() {
     gMsgHeadersToolbarElement = document.getElementById("MsgHeadersToolbar");
   }
 
   return gMsgHeadersToolbarElement;
 }
 
 /**
  * Determine which element of the fast-track focus ring has focus.
- * Note that only elements of the fast-track focus ring will be returned.
+ * Note that mostly elements of the fast-track focus ring will be returned.
  *
- * @return an element node of the fast-track focus ring if the node or one of
- *         its descendants has focus, otherwise null.
+ * @return {HTMLElement | null} An element node of the fast-track focus ring if
+ *   the node or one of its descendants has focus, sometimes other focused
+ *   elements, otherwise null.
  */
 function WhichElementHasFocus() {
-  let msgAttachmentElement = GetMsgAttachmentElement();
-  let abContactsPanelElement = sidebarDocumentGetElementById("abContactsPanel");
-
-  if (top.document.commandDispatcher.focusedWindow == window.content) {
-    return window.content;
+  // Special-case message body
+  if (document.activeElement == document.getElementById("content-frame")) {
+    return document.getElementById("content-frame");
   }
 
   let currentNode = top.document.commandDispatcher.focusedElement;
+
+  // Special-case Contacts Side Bar's peopleSearchInput so that iteration on
+  // currentNode.parentNode doesn't get stuck on Shadow Root of anonymous input.
+  let peopleSearchInput = sidebarDocumentGetElementById(
+    "peopleSearchInput",
+    "abContactsPanel"
+  );
+  if (
+    currentNode.flattenedTreeParentNode &&
+    currentNode.flattenedTreeParentNode == peopleSearchInput
+  ) {
+    currentNode = peopleSearchInput;
+  }
+
   while (currentNode) {
     if (
       currentNode == document.getElementById("msgIdentity") ||
       currentNode == document.getElementById("toAddrInput") ||
       currentNode == document.getElementById("ccAddrInput") ||
       currentNode == document.getElementById("bccAddrInput") ||
       currentNode == document.getElementById("replyAddrInput") ||
       currentNode == document.getElementById("newsgroupsAddrInput") ||
       currentNode == document.getElementById("followupAddrInput") ||
       currentNode == document.getElementById("msgSubject") ||
+      currentNode == document.getElementById("attachmentBucket") ||
       currentNode == document.getElementById("extraRecipientsLabel") ||
       currentNode == document.getElementById("addr_bcc") ||
       currentNode == document.getElementById("addr_cc") ||
-      currentNode == msgAttachmentElement ||
-      currentNode == abContactsPanelElement
+      currentNode == sidebarDocumentGetElementById("abContactsPanel")
     ) {
       return currentNode;
     }
+    // Iterate parent nodes until we find one that matches.
+    // Applicable for Contacts Sidebar with focus on search input or a contact.
     currentNode = currentNode.parentNode;
   }
 
   return null;
 }
 
 /**
  * Fast-track focus ring: Switch focus between important (not all) elements
  * in the message compose window. Ctrl+[Shift+]Tab | [Shift+]F6 on Windows.
  *
  * The default element to switch to when going in either direction (with or
  * without shift key pressed) is the ToRecipientElement.
  *
- * The only exception is when the MsgHeadersToolbar is collapsed,
- * then the focus will always be on the body of the message.
+ * @param {Event} event - A DOM keyboard event of a fast focus ring shortcut key
  */
 function SwitchElementFocus(event) {
   let focusedElement = WhichElementHasFocus();
 
   if (!focusedElement) {
     // None of the pre-defined focus ring elements has focus: This should never
     // happen with the default installation, but might happen with add-ons.
     // In that case, default to focusing the address widget as the first element
@@ -7640,32 +7654,31 @@ function SwitchElementFocus(event) {
         // otherwise focus message body.
         if (sidebar_is_hidden() || !focusContactsSidebarSearchInput()) {
           SetMsgBodyFrameFocus();
         }
         break;
       case sidebarDocumentGetElementById("abContactsPanel"):
         SetMsgBodyFrameFocus();
         break;
-      case window.content: // message body
-        // Only set focus to the attachment element if it is shown.
+      case document.getElementById("content-frame"): // message body
+        // Focus attachment bucket if shown, otherwise message subject.
         if (!document.getElementById("attachments-box").collapsed) {
           SetMsgAttachmentElementFocus();
         } else {
           SetMsgSubjectElementFocus();
         }
         break;
       case gMsgAttachmentElement:
         SetMsgSubjectElementFocus();
         break;
       case document.getElementById("msgSubject"):
         SetFocusOnPreviousAvailableElement(focusedElement);
         break;
       default:
-        // focus on '#msgIdentity'
         SetMsgToRecipientElementFocus();
         break;
     }
 
     return;
   }
 
   // Forwards focus ring: e.g. Ctrl+Tab | F6
@@ -7684,27 +7697,27 @@ function SwitchElementFocus(event) {
       break;
     case document.getElementById("followupAddrInput"):
       SetFocusOnNextAvailableElement(focusedElement);
       break;
     case document.getElementById("newsgroupsAddrInput"):
       SetFocusOnNextAvailableElement(focusedElement);
       break;
     case document.getElementById("msgSubject"):
-      // Only set focus to the attachment element if it is shown.
+      // Focus attachment bucket if shown, otherwise message body.
       if (!document.getElementById("attachments-box").collapsed) {
         SetMsgAttachmentElementFocus();
       } else {
         SetMsgBodyFrameFocus();
       }
       break;
     case gMsgAttachmentElement:
       SetMsgBodyFrameFocus();
       break;
-    case window.content:
+    case document.getElementById("content-frame"): // message body
       // Focus the search input of contacts side bar if that's available,
       // otherwise focus "From" selector.
       if (sidebar_is_hidden() || !focusContactsSidebarSearchInput()) {
         SetMsgIdentityElementFocus();
       }
       break;
     case sidebarDocumentGetElementById("abContactsPanel"):
       SetMsgIdentityElementFocus();
--- a/mail/test/browser/composition/browser_focus.js
+++ b/mail/test/browser/composition/browser_focus.js
@@ -30,34 +30,34 @@ function check_element_cycling(controlle
   // Make sure the accessibility tabfocus is set to 7 to enable normal Tab
   // focus on non-input field elements. This is necessary only for macOS as
   // the dafault value is 2 instead of the default 7 used on Windows and Linux.
   Services.prefs.setIntPref("accessibility.tabfocus", 7);
 
   let addressingElement = controller.e("toAddrInput");
   let subjectElement = controller.e("msgSubject");
   let attachmentElement = controller.e("attachmentBucket");
-  let contentElement = controller.window.content;
+  let editorElement = controller.e("content-frame");
   let identityElement = controller.e("msgIdentity");
   let extraRecipientsLabel = controller.e("extraRecipientsLabel");
   let bccLabel = controller.e("addr_bcc");
   let ccLabel = controller.e("addr_cc");
 
   let key = ctrlTab ? "VK_TAB" : "VK_F6";
 
   // We start on the addressing widget and go from there.
 
   controller.keypress(null, key, { ctrlKey: ctrlTab });
   Assert.equal(subjectElement, controller.window.WhichElementHasFocus());
   if (attachmentsExpanded) {
     controller.keypress(null, key, { ctrlKey: ctrlTab });
     Assert.equal(attachmentElement, controller.window.WhichElementHasFocus());
   }
   controller.keypress(null, key, { ctrlKey: ctrlTab });
-  Assert.equal(contentElement, controller.window.WhichElementHasFocus());
+  Assert.equal(editorElement, controller.window.WhichElementHasFocus());
   controller.keypress(null, key, { ctrlKey: ctrlTab });
   Assert.equal(identityElement, controller.window.WhichElementHasFocus());
   controller.keypress(null, key, { ctrlKey: ctrlTab });
   mc.sleep(0); // Focusing the addressing element happens in a timeout...
   Assert.equal(addressingElement, controller.window.WhichElementHasFocus());
 
   controller.keypress(null, key, { ctrlKey: ctrlTab, shiftKey: true });
 
@@ -71,17 +71,17 @@ function check_element_cycling(controlle
     controller.keypress(null, key, { shiftKey: true });
     Assert.equal(ccLabel, controller.window.WhichElementHasFocus());
 
     controller.keypress(null, key, { shiftKey: true });
   }
 
   Assert.equal(identityElement, controller.window.WhichElementHasFocus());
   controller.keypress(null, key, { ctrlKey: ctrlTab, shiftKey: true });
-  Assert.equal(contentElement, controller.window.WhichElementHasFocus());
+  Assert.equal(editorElement, controller.window.WhichElementHasFocus());
   if (attachmentsExpanded) {
     controller.keypress(null, key, { ctrlKey: ctrlTab, shiftKey: true });
     Assert.equal(attachmentElement, controller.window.WhichElementHasFocus());
   }
   controller.keypress(null, key, { ctrlKey: ctrlTab, shiftKey: true });
   Assert.equal(subjectElement, controller.window.WhichElementHasFocus());
   controller.keypress(null, key, { ctrlKey: ctrlTab, shiftKey: true });
   mc.sleep(0); // Focusing the addressing element happens in a timeout...