Bug 1190882: If the focused accessible is removed from the tree, fire a11y focus on the document. r=eeejay
authorJames Teh <jteh@mozilla.com>
Wed, 15 May 2019 00:31:16 +0000
changeset 532706 28b9af9efee6a0317e21fa3a1971f039f53a25ed
parent 532705 294fb9eba22edda7dd2f8c3691364eb8b6a2a2d0
child 532707 61f3f19ee0de3b05060506337f8fb71a08c2e07b
push id11270
push userrgurzau@mozilla.com
push dateWed, 15 May 2019 15:07:19 +0000
treeherdermozilla-beta@571bc76da583 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerseeejay
bugs1190882, 559561
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1190882: If the focused accessible is removed from the tree, fire a11y focus on the document. r=eeejay If the DOM focus is removed before something else is focused, the document gets DOM focus, but no blur event is fired (bug 559561). This means that no a11y focus event is fired, so clients aren't notified. This is particularly problematic for screen readers when dismissing some ARIA dialogs, as the screen reader doesn't know that focus has returned to the top level document. Differential Revision: https://phabricator.services.mozilla.com/D31024
accessible/base/FocusManager.cpp
accessible/base/FocusManager.h
accessible/generic/DocAccessible.cpp
accessible/tests/mochitest/events/a11y.ini
accessible/tests/mochitest/events/test_focus_removal.html
--- a/accessible/base/FocusManager.cpp
+++ b/accessible/base/FocusManager.cpp
@@ -94,16 +94,20 @@ FocusManager::FocusDisposition FocusMana
     if (child == focus) return eContainedByFocus;
 
     child = child->Parent();
   }
 
   return eNone;
 }
 
+bool FocusManager::WasLastFocused(const Accessible* aAccessible) const {
+  return mLastFocus == aAccessible;
+}
+
 void FocusManager::NotifyOfDOMFocus(nsISupports* aTarget) {
 #ifdef A11Y_LOG
   if (logging::IsEnabled(logging::eFocus))
     logging::FocusNotificationTarget("DOM focus", "Target", aTarget);
 #endif
 
   mActiveItem = nullptr;
 
@@ -335,16 +339,17 @@ void FocusManager::ProcessFocusEvent(Acc
   // Reset cached caret value. The cache will be updated upon processing the
   // next caret move event. This ensures that we will return the correct caret
   // offset before the caret move event is handled.
   SelectionMgr()->ResetCaretOffset();
 
   RefPtr<AccEvent> focusEvent = new AccEvent(nsIAccessibleEvent::EVENT_FOCUS,
                                              target, aEvent->FromUserInput());
   nsEventShell::FireEvent(focusEvent);
+  mLastFocus = target;
 
   // Fire scrolling_start event when the document receives the focus if it has
   // an anchor jump. If an accessible within the document receive the focus
   // then null out the anchor jump because it no longer applies.
   DocAccessible* targetDocument = target->Document();
   Accessible* anchorJump = targetDocument->AnchorJump();
   if (anchorJump) {
     if (target == targetDocument) {
--- a/accessible/base/FocusManager.h
+++ b/accessible/base/FocusManager.h
@@ -65,16 +65,28 @@ class FocusManager {
 
   /**
    * Return whether the given accessible is focused or contains the focus or
    * contained by focused accessible.
    */
   enum FocusDisposition { eNone, eFocused, eContainsFocus, eContainedByFocus };
   FocusDisposition IsInOrContainsFocus(const Accessible* aAccessible) const;
 
+  /**
+   * Return true if the given accessible was the last accessible focused.
+   * This is useful to detect the case where the last focused accessible was
+   * removed before something else was focused. This can happen in one of two
+   * ways:
+   * 1. The DOM focus was removed. DOM doesn't fire a blur event when this
+   *    happens; see bug 559561.
+   * 2. The accessibility focus was an active item (e.g. aria-activedescendant)
+   *    and that item was removed.
+   */
+  bool WasLastFocused(const Accessible* aAccessible) const;
+
   //////////////////////////////////////////////////////////////////////////////
   // Focus notifications and processing (don't use until you know what you do).
 
   /**
    * Called when DOM focus event is fired.
    */
   void NotifyOfDOMFocus(nsISupports* aTarget);
 
@@ -119,15 +131,16 @@ class FocusManager {
 
   /**
    * Return DOM document having DOM focus.
    */
   dom::Document* FocusedDOMDocument() const;
 
  private:
   RefPtr<Accessible> mActiveItem;
+  RefPtr<Accessible> mLastFocus;
   RefPtr<Accessible> mActiveARIAMenubar;
 };
 
 }  // namespace a11y
 }  // namespace mozilla
 
 #endif
--- a/accessible/generic/DocAccessible.cpp
+++ b/accessible/generic/DocAccessible.cpp
@@ -1211,19 +1211,19 @@ void DocAccessible::BindToDocument(Acces
     }
   }
 }
 
 void DocAccessible::UnbindFromDocument(Accessible* aAccessible) {
   NS_ASSERTION(mAccessibleCache.GetWeak(aAccessible->UniqueID()),
                "Unbinding the unbound accessible!");
 
-  // Fire focus event on accessible having DOM focus if active item was removed
+  // Fire focus event on accessible having DOM focus if last focus was removed
   // from the tree.
-  if (FocusMgr()->IsActiveItem(aAccessible)) {
+  if (FocusMgr()->WasLastFocused(aAccessible)) {
     FocusMgr()->ActiveItemChanged(nullptr);
 #ifdef A11Y_LOG
     if (logging::IsEnabled(logging::eFocus))
       logging::ActiveItemChangeCausedBy("tree shutdown", aAccessible);
 #endif
   }
 
   // Remove an accessible from node-to-accessible map if it exists there.
--- a/accessible/tests/mochitest/events/a11y.ini
+++ b/accessible/tests/mochitest/events/a11y.ini
@@ -29,16 +29,17 @@ skip-if = os == 'win' || os == 'linux'
 [test_focus_contextmenu.xul]
 [test_focus_controls.html]
 [test_focus_doc.html]
 [test_focus_general.html]
 [test_focus_general.xul]
 [test_focus_listcontrols.xul]
 [test_focus_menu.xul]
 [test_focus_name.html]
+[test_focus_removal.html]
 [test_focus_selects.html]
 [test_focus_tabbox.xul]
 skip-if = webrender
 [test_focus_tree.xul]
 [test_fromUserInput.html]
 [test_label.xul]
 [test_menu.xul]
 [test_mutation.html]
new file mode 100644
--- /dev/null
+++ b/accessible/tests/mochitest/events/test_focus_removal.html
@@ -0,0 +1,82 @@
+<html>
+
+<head>
+  <title>Test removal of focused accessible</title>
+
+  <link rel="stylesheet" type="text/css"
+        href="chrome://mochikit/content/tests/SimpleTest/test.css" />
+
+  <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+
+  <script type="application/javascript"
+          src="../common.js"></script>
+  <script type="application/javascript"
+          src="../events.js"></script>
+
+  <script type="application/javascript">
+    async function setFocus(aNodeToFocus, aExpectedFocus) {
+      let expected = aExpectedFocus || aNodeToFocus;
+      let focused = waitForEventPromise(EVENT_FOCUS, expected);
+      info("Focusing " + aNodeToFocus.id);
+      aNodeToFocus.focus();
+      await focused;
+      ok(true, expected.id + " focused after " +
+        aNodeToFocus.id + ".focus()");
+    }
+
+    async function expectFocusAfterRemove(aNodeToRemove, aExpectedFocus) {
+      let focused = waitForEventPromise(EVENT_FOCUS, aExpectedFocus);
+      info("Removing " + aNodeToRemove.id);
+      aNodeToRemove.remove();
+      await focused;
+      let friendlyExpected = aExpectedFocus == document ?
+        "document" : aExpectedFocus.id;
+      ok(true, friendlyExpected + " focused after " +
+        aNodeToRemove.id + " removed");
+    }
+
+    async function doTests() {
+      info("Testing removal of focused node itself");
+      let button = getNode("button");
+      await setFocus(button);
+      await expectFocusAfterRemove(button, document);
+
+      info("Testing removal of focused node's parent");
+      let dialog = getNode("dialog");
+      let dialogButton = getNode("dialogButton");
+      await setFocus(dialogButton);
+      await expectFocusAfterRemove(dialog, document);
+
+      info("Testing removal of aria-activedescendant target");
+      let listbox = getNode("listbox");
+      let option = getNode("option");
+      await setFocus(listbox, option);
+      await expectFocusAfterRemove(option, listbox);
+
+      SimpleTest.finish();
+    }
+
+    SimpleTest.waitForExplicitFinish();
+    addA11yLoadEvent(doTests);
+  </script>
+</head>
+
+<body>
+
+  <p id="display"></p>
+  <div id="content" style="display: none"></div>
+  <pre id="test">
+  </pre>
+
+  <button id="button"></button>
+
+  <div role="dialog" id="dialog">
+    <button id="dialogButton"></button>
+  </div>
+
+  <div role="listbox" id="listbox" tabindex="0" aria-activedescendant="option">
+    <div role="option" id="option"></div>
+  </div>
+
+</body>
+</html>