Bug 1699284 - Part 4: Continue to search/try next accesskey target if current target is not focusable; r=masayuki

For example, if the target element is disabled, we could not move focus to it,
then we cycle to next accesskey target.

Differential Revision: https://phabricator.services.mozilla.com/D109621

dom/events/EventStateManager.cpp

@@ -1102,16 +1102,19 @@ bool EventStateManager::LookForAccessKeyAndExecute(
           shouldActivate = false;
         }
 
-        while (shouldActivate && ++count <= length) {
-          Element* el = mAccessKeys[(start + count) % length];
+        // XXXedgar, Bug 1700646, maybe we could use other data structure to
+        // make searching target with same accesskey easier, and current setup
+        // could not ensure we cycle the target with tree order.
+        int32_t j = 0;
+        while (shouldActivate && ++j < length) {
+          Element* el = mAccessKeys[(start + count + j) % length];
           if (IsAccessKeyTarget(el, accessKey)) {
             shouldActivate = false;
           }
         }
 
-        bool focusChanged =
-            element->PerformAccesskey(shouldActivate, aIsTrustedEvent);
-        if (focusChanged && aIsTrustedEvent) {
+        if (element->PerformAccesskey(shouldActivate, aIsTrustedEvent)) {
+          if (aIsTrustedEvent) {
             // If this is a child process, inform the parent that we want the
             // focus, but pass false since we don't want to change the window
             // order.
@@ -1122,11 +1125,11 @@ bool EventStateManager::LookForAccessKeyAndExecute(
               child->SendRequestFocus(false, CallerType::System);
             }
           }
-
           return true;
         }
       }
     }
+  }
   return false;
 }
 

dom/html/nsGenericHTMLElement.cpp

@@ -2418,11 +2418,8 @@ bool nsGenericHTMLElement::PerformAccesskey(bool aKeyCausesActivation,
     fm->SetFocus(this, nsIFocusManager::FLAG_BYKEY);
 
     // Return true if the element became the current focus within its window.
-    //
-    // FIXME(emilio): Shouldn't this check `window->GetFocusedElement() == this`
-    // based on the above comment?
     nsPIDOMWindowOuter* window = OwnerDoc()->GetWindow();
-    focused = window && window->GetFocusedElement();
+    focused = window && window->GetFocusedElement() == this;
   }
 
   if (aKeyCausesActivation) {

dom/xul/nsXULElement.cpp

@@ -543,7 +543,7 @@ bool nsXULElement::PerformAccesskey(bool aKeyCausesActivation,
 
         // Return true if the element became focused.
         nsPIDOMWindowOuter* window = OwnerDoc()->GetWindow();
-        focused = (window && window->GetFocusedElement());
+        focused = (window && window->GetFocusedElement() == elementToFocus);
       }
     }
   }

dom/xul/test/test_accesskey.xhtml

@@ -12,7 +12,8 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1699284
 <label id="label1" accesskey="a">Label 1</label><button id="button1" accesskey="a">Button 1</button>
 <label id="label2" accesskey="a" tabindex="0">Label 2</label><button id="button2">Button 2</button>
 <label id="label3">Label 3</label><button id="button3" accesskey="a">Button 3</button>
-<label id="label4" accesskey="a" control="button4">Label 4</label><button id="button4">Button 4</button>
+<label id="label4" accesskey="a" control="button4">Label 4</label><button id="button4" disabled="true">Button 4</button>
+<label id="label5" accesskey="a" control="button5">Label 5</label><button id="button5">Button 5</button>
 <!-- Tests code -->
 <script type="application/javascript">
 <![CDATA[
@@ -25,11 +26,11 @@ function PerformAccessKey(aKey) {
 };
 
 add_task(async function test_accesskey() {
-  let [label1, label2, label3, label4] = document.querySelectorAll("label");
-  let [button1, button2, button3, button4] = document.querySelectorAll("button");
+  let [label1, label2, label3, label4, label5] = document.querySelectorAll("label");
+  let [button1, button2, button3, button4, button5] = document.querySelectorAll("button");
 
   [
-    label1, label2, label3, label4, button1, button2, button3, button4
+    label1, label2, label3, label4, label5, button1, button2, button3, button4, button5
   ].forEach(function(ele) {
     ele.addEventListener("click", function(e) {
       ok(false, `${e.target.id} should not receive click event`);
@@ -43,7 +44,7 @@ add_task(async function test_accesskey() {
   is(document.activeElement.id, button3.id, `focus should move to ${button3.id}`);
 
   PerformAccessKey("a");
-  is(document.activeElement.id, button4.id, `focus should move to ${button4.id}`);
+  is(document.activeElement.id, button5.id, `focus should move to ${button5.id}`);
 
   // Cycle back to first element
   PerformAccessKey("a");