Bug 1280360 - markupview: a11y fix tabindex when modifying attributes;r=pbro draft
authorJulian Descottes <jdescottes@mozilla.com>
Fri, 01 Jul 2016 23:34:21 +0200
changeset 389468 e03f187326b6b42f7e575712137118e3adcb6a6f
parent 389463 4cf559be7ff0d7a57bfca355eef88ed890b86717
child 525763 5655d000999aa5557ef307302812d6dacf10dbe9
push id23418
push userjdescottes@mozilla.com
push dateTue, 19 Jul 2016 12:14:10 +0000
reviewerspbro
bugs1280360
milestone50.0a1
Bug 1280360 - markupview: a11y fix tabindex when modifying attributes;r=pbro Modify the template for markup view attributes to accept a tabindex attribute. The markup for attributes might be generated and inserted in an already focused markup view line. In this case the DOM element should be created with a tabindex of 0 instead of -1. MozReview-Commit-ID: AaMUS1h1o0C
devtools/client/framework/test/shared-head.js
devtools/client/inspector/markup/markup.js
devtools/client/inspector/markup/markup.xhtml
devtools/client/inspector/markup/test/browser.ini
devtools/client/inspector/markup/test/browser_markup_accessibility_navigation.js
devtools/client/inspector/markup/test/browser_markup_accessibility_navigation_after_edit.js
devtools/client/inspector/markup/test/helper_markup_accessibility_navigation.js
--- a/devtools/client/framework/test/shared-head.js
+++ b/devtools/client/framework/test/shared-head.js
@@ -1,12 +1,13 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
+/* eslint no-unused-vars: [2, {"vars": "local"}] */
 
 "use strict";
 
 // This shared-head.js file is used for multiple mochitest test directories in
 // devtools.
 // It contains various common helper functions.
 
 const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr, Constructor: CC}
@@ -469,8 +470,22 @@ function waitForContextMenu(popup, butto
  * @return {Promise} resolves when the preferences have been updated
  */
 function pushPref(preferenceName, value) {
   return new Promise(resolve => {
     let options = {"set": [[preferenceName, value]]};
     SpecialPowers.pushPrefEnv(options, resolve);
   });
 }
+
+/**
+ * Lookup the provided dotted path ("prop1.subprop2.myProp") in the provided object.
+ *
+ * @param {Object} obj
+ *        Object to expand.
+ * @param {String} path
+ *        Dotted path to use to expand the object.
+ * @return {?} anything that is found at the provided path in the object.
+ */
+function lookupPath(obj, path) {
+  let segments = path.split(".");
+  return segments.reduce((prev, current) => prev[current], obj);
+}
--- a/devtools/client/inspector/markup/markup.js
+++ b/devtools/client/inspector/markup/markup.js
@@ -3188,16 +3188,17 @@ ElementEditor.prototype = {
       attr.remove();
     }
   },
 
   _createAttribute: function (attribute, before = null) {
     // Create the template editor, which will save some variables here.
     let data = {
       attrName: attribute.name,
+      tabindex: this.container.canFocus ? "0" : "-1",
     };
     this.template("attribute", data);
     let {attr, inner, name, val} = data;
 
     // Double quotes need to be handled specially to prevent DOMParser failing.
     // name="v"a"l"u"e" when editing -> name='v"a"l"u"e"'
     // name="v'a"l'u"e" when editing -> name="v'a&quot;l'u&quot;e"
     let editValueDisplayed = attribute.value || "";
--- a/devtools/client/inspector/markup/markup.xhtml
+++ b/devtools/client/inspector/markup/markup.xhtml
@@ -71,17 +71,17 @@
  --></span>
 
     <span id="template-attribute"
           save="${attr}"
           data-attr="${attrName}"
           data-value="${attrValue}"
           class="attreditor"
           style="display:none"> <!--
-   --><span class="editable" save="${inner}" tabindex="-1"><!--
+   --><span class="editable" save="${inner}" tabindex="${tabindex}"><!--
      --><span save="${name}" class="attr-name theme-fg-color2"></span><!--
      -->=&quot;<!--
      --><span save="${val}" class="attr-value theme-fg-color6"></span><!--
      -->&quot;<!--
    --></span><!--
  --></span>
 
     <span id="template-text" save="${elt}" class="editor text"><!--
--- a/devtools/client/inspector/markup/test/browser.ini
+++ b/devtools/client/inspector/markup/test/browser.ini
@@ -29,16 +29,17 @@ support-files =
   doc_markup_toggle.html
   doc_markup_tooltip.png
   doc_markup_void_elements.html
   doc_markup_void_elements.xhtml
   doc_markup_xul.xul
   head.js
   helper_attributes_test_runner.js
   helper_events_test_runner.js
+  helper_markup_accessibility_navigation.js
   helper_outerhtml_test_runner.js
   helper_style_attr_test_runner.js
   lib_jquery_1.0.js
   lib_jquery_1.1.js
   lib_jquery_1.2_min.js
   lib_jquery_1.3_min.js
   lib_jquery_1.4_min.js
   lib_jquery_1.6_min.js
@@ -50,16 +51,18 @@ support-files =
   !/devtools/client/framework/test/shared-head.js
   !/devtools/client/shared/test/test-actor.js
   !/devtools/client/shared/test/test-actor-registry.js
 
 [browser_markup_accessibility_focus_blur.js]
 skip-if = os == "mac" # Full keyboard navigation on OSX only works if Full Keyboard Access setting is set to All Control in System Keyboard Preferences
 [browser_markup_accessibility_navigation.js]
 skip-if = os == "mac" # Full keyboard navigation on OSX only works if Full Keyboard Access setting is set to All Control in System Keyboard Preferences
+[browser_markup_accessibility_navigation_after_edit.js]
+skip-if = os == "mac" # Full keyboard navigation on OSX only works if Full Keyboard Access setting is set to All Control in System Keyboard Preferences
 [browser_markup_accessibility_semantics.js]
 [browser_markup_anonymous_01.js]
 [browser_markup_anonymous_02.js]
 skip-if = e10s # scratchpad.xul is not loading in e10s window
 [browser_markup_anonymous_03.js]
 [browser_markup_anonymous_04.js]
 [browser_markup_copy_image_data.js]
 subsuite = clipboard
--- a/devtools/client/inspector/markup/test/browser_markup_accessibility_navigation.js
+++ b/devtools/client/inspector/markup/test/browser_markup_accessibility_navigation.js
@@ -1,17 +1,18 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+/* import-globals-from helper_markup_accessibility_navigation.js */
 
 "use strict";
 
-/* global getContainerForSelector, openInspectorForURL */
+// Test keyboard navigation accessibility of inspector's markup view.
 
-// Test keyboard navigation accessibility of inspector's markup view.
+loadHelperScript("helper_markup_accessibility_navigation.js");
 
 /**
  * Test data has the format of:
  * {
  *   desc              {String}   description for better logging
  *   key               {String}   key event's key
  *   options           {?Object}  optional event data such as shiftKey, etc
  *   focused           {String}   path to expected focused element relative to
@@ -232,70 +233,45 @@ const TESTS = [
     focused: "docBody",
     activedescendant: "body.tagLine",
     key: "VK_UP",
     options: { },
     waitFor: "inspector-updated"
   },
 ];
 
+let containerID = 0;
 let elms = {};
-let containerID = 0;
 
 add_task(function* () {
   let { inspector } = yield openInspectorForURL(`data:text/html;charset=utf-8,
     <h1 id="some-id" class="some-class">foo<span>Child span<span></h1>`);
-  let markup = inspector.markup;
-  let doc = markup.doc;
-  let win = doc.defaultView;
 
   // Record containers that are created after inspector is initialized to be
   // useful in testing.
   inspector.on("container-created", memorizeContainer);
   registerCleanupFunction(() => {
     inspector.off("container-created", memorizeContainer);
   });
 
-  elms.docBody = doc.body;
-  elms.root = markup.getContainer(markup._rootNode);
+  elms.docBody = inspector.markup.doc.body;
+  elms.root = inspector.markup.getContainer(inspector.markup._rootNode);
   elms.header = yield getContainerForSelector("h1", inspector);
   elms.body = yield getContainerForSelector("body", inspector);
 
   // Initial focus is on root element and active descendant should be set on
   // body tag line.
-  testNavigationState(doc, elms.docBody, elms.body.tagLine);
+  testNavigationState(inspector, elms, elms.docBody, elms.body.tagLine);
 
   // Focus on the tree element.
   elms.root.elt.focus();
 
-  for (let {desc, waitFor, focused, activedescendant, key, options} of TESTS) {
-    info(desc);
-    let updated;
-    if (waitFor) {
-      updated = waitFor === "inspector-updated" ?
-        inspector.once(waitFor) : markup.once(waitFor);
-    } else {
-      updated = Promise.resolve();
-    }
+  for (let testData of TESTS) {
+    yield runAccessibilityNavigationTest(inspector, elms, testData);
+  }
 
-    EventUtils.synthesizeKey(key, options, win);
-    yield updated;
-    testNavigationState(doc, getElm(focused), getElm(activedescendant));
-  }
+  elms = null;
 });
 
 // Record all containers that are created dynamically into elms object.
 function memorizeContainer(event, container) {
   elms[`container-${containerID++}`] = container;
 }
-
-// Parse and lookup an element from elms object based on dotted path.
-function getElm(path) {
-  let segments = path.split(".");
-  return segments.reduce((prev, current) => prev[current], elms);
-}
-
-function testNavigationState(doc, focused, activedescendant) {
-  let id = activedescendant.getAttribute("id");
-  is(doc.activeElement, focused, `Keyboard focus should be set to ${focused}`);
-  is(elms.root.elt.getAttribute("aria-activedescendant"), id,
-    `Active descendant should be set to ${id}`);
-}
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/markup/test/browser_markup_accessibility_navigation_after_edit.js
@@ -0,0 +1,126 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+/* import-globals-from helper_markup_accessibility_navigation.js */
+
+"use strict";
+
+// Test keyboard navigation accessibility is preserved after editing attributes.
+
+loadHelperScript("helper_markup_accessibility_navigation.js");
+
+const TEST_URI = '<div id="some-id" class="some-class"></div>';
+
+/**
+ * Test data has the format of:
+ * {
+ *   desc              {String}   description for better logging
+ *   key               {String}   key event's key
+ *   options           {?Object}  optional event data such as shiftKey, etc
+ *   focused           {String}   path to expected focused element relative to
+ *                                its container
+ *   activedescendant  {String}   path to expected aria-activedescendant element
+ *                                relative to its container
+ *   waitFor           {String}   optional event to wait for if keyboard actions
+ *                                result in asynchronous updates
+ * }
+ */
+const TESTS = [
+  {
+    desc: "Select header container",
+    focused: "root.elt",
+    activedescendant: "div.tagLine",
+    key: "VK_DOWN",
+    options: { },
+    waitFor: "inspector-updated"
+  },
+  {
+    desc: "Focus on header tag",
+    focused: "div.focusableElms.0",
+    activedescendant: "div.tagLine",
+    key: "VK_RETURN",
+    options: { }
+  },
+  {
+    desc: "Activate header tag editor",
+    focused: "div.editor.tag.inplaceEditor.input",
+    activedescendant: "div.tagLine",
+    key: "VK_RETURN",
+    options: { }
+  },
+  {
+    desc: "Activate header id attribute editor",
+    focused: "div.editor.attrList.children.0.children.1.inplaceEditor.input",
+    activedescendant: "div.tagLine",
+    key: "VK_TAB",
+    options: { }
+  },
+  {
+    desc: "Deselect text in header id attribute editor",
+    focused: "div.editor.attrList.children.0.children.1.inplaceEditor.input",
+    activedescendant: "div.tagLine",
+    key: "VK_TAB",
+    options: { }
+  },
+  {
+    desc: "Move the cursor to the left",
+    focused: "div.editor.attrList.children.0.children.1.inplaceEditor.input",
+    activedescendant: "div.tagLine",
+    key: "VK_LEFT",
+    options: { }
+  },
+  {
+    desc: "Modify the attribute",
+    focused: "div.editor.attrList.children.0.children.1.inplaceEditor.input",
+    activedescendant: "div.tagLine",
+    key: "A",
+    options: { }
+  },
+  {
+    desc: "Commit the attribute change",
+    focused: "div.focusableElms.1",
+    activedescendant: "div.tagLine",
+    key: "VK_RETURN",
+    options: { },
+    waitFor: "inspector-updated"
+  },
+  {
+    desc: "Tab and focus on header class attribute",
+    focused: "div.focusableElms.2",
+    activedescendant: "div.tagLine",
+    key: "VK_TAB",
+    options: { }
+  },
+  {
+    desc: "Tab and focus on header new attribute node",
+    focused: "div.focusableElms.3",
+    activedescendant: "div.tagLine",
+    key: "VK_TAB",
+    options: { }
+  },
+];
+
+let elms = {};
+
+add_task(function* () {
+  let url = `data:text/html;charset=utf-8,${TEST_URI}`;
+  let { inspector } = yield openInspectorForURL(url);
+
+  elms.docBody = inspector.markup.doc.body;
+  elms.root = inspector.markup.getContainer(inspector.markup._rootNode);
+  elms.div = yield getContainerForSelector("div", inspector);
+  elms.body = yield getContainerForSelector("body", inspector);
+
+  // Initial focus is on root element and active descendant should be set on
+  // body tag line.
+  testNavigationState(inspector, elms, elms.docBody, elms.body.tagLine);
+
+  // Focus on the tree element.
+  elms.root.elt.focus();
+
+  for (let testData of TESTS) {
+    yield runAccessibilityNavigationTest(inspector, elms, testData);
+  }
+
+  elms = null;
+});
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/markup/test/helper_markup_accessibility_navigation.js
@@ -0,0 +1,70 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+/* eslint no-unused-vars: [2, {"vars": "local"}] */
+/* import-globals-from head.js */
+"use strict";
+
+/**
+ * Execute a keyboard event and check that the state is as expected (focused element, aria
+ * attribute etc...).
+ *
+ * @param {InspectorPanel} inspector
+ *        Current instance of the inspector being tested.
+ * @param {Object} elms
+ *        Map of elements that will be used to retrieve live references to children
+ *        elements
+ * @param {Element} focused
+ *        Element expected to be focused
+ * @param {Element} activedescendant
+ *        Element expected to be the aria activedescendant of the root node
+ */
+function testNavigationState(inspector, elms, focused, activedescendant) {
+  let doc = inspector.markup.doc;
+  let id = activedescendant.getAttribute("id");
+  is(doc.activeElement, focused, `Keyboard focus should be set to ${focused}`);
+  is(elms.root.elt.getAttribute("aria-activedescendant"), id,
+    `Active descendant should be set to ${id}`);
+}
+
+/**
+ * Execute a keyboard event and check that the state is as expected (focused element, aria
+ * attribute etc...).
+ *
+ * @param {InspectorPanel} inspector
+ *        Current instance of the inspector being tested.
+ * @param {Object} elms
+ *        MarkupContainers/Elements that will be used to retrieve references to other
+ *        elements based on objects' paths.
+ * @param {Object} testData
+ *        - {String} desc: description for better logging.
+ *        - {String} key: keyboard event's key.
+ *        - {Object} options, optional: event data such as shiftKey, etc.
+ *        - {String} focused: path to expected focused element in elms map.
+ *        - {String} activedescendant: path to expected aria-activedescendant element in
+ *          elms map.
+ *        - {String} waitFor, optional: markupview event to wait for if keyboard actions
+ *          result in async updates. Also accepts the inspector event "inspector-updated".
+ */
+function* runAccessibilityNavigationTest(inspector, elms,
+  {desc, key, options, focused, activedescendant, waitFor}) {
+  info(desc);
+
+  let markup = inspector.markup;
+  let doc = markup.doc;
+  let win = doc.defaultView;
+
+  let updated;
+  if (waitFor) {
+    updated = waitFor === "inspector-updated" ?
+      inspector.once(waitFor) : markup.once(waitFor);
+  } else {
+    updated = Promise.resolve();
+  }
+  EventUtils.synthesizeKey(key, options, win);
+  yield updated;
+
+  let focusedElement = lookupPath(elms, focused);
+  let activeDescendantElement = lookupPath(elms, activedescendant);
+  testNavigationState(inspector, elms, focusedElement, activeDescendantElement);
+}