Bug 1201595 - improving a11y checks reliability and error messaging. r=ato
authorYura Zenevich <yzenevich@mozilla.com>
Thu, 10 Sep 2015 11:45:33 -0400
changeset 294623 e6155c93aa202d11b696fcee36b9be127f88ca3d
parent 294622 9f8526509f5a19a6b89ee7178bba9c5545e12005
child 294624 f0fbe3de27cb6812269c67eac9a917f2a95d1b25
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersato
bugs1201595
milestone43.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 1201595 - improving a11y checks reliability and error messaging. r=ato
testing/marionette/client/marionette/tests/unit/test_accessibility.py
testing/marionette/driver.js
testing/marionette/elements.js
testing/marionette/listener.js
--- a/testing/marionette/client/marionette/tests/unit/test_accessibility.py
+++ b/testing/marionette/client/marionette/tests/unit/test_accessibility.py
@@ -92,17 +92,17 @@ class TestAccessibility(MarionetteTestCa
         self.run_element_test(self.valid_elementIDs, lambda button: button.tap())
 
     def test_single_tap_raises_element_not_accessible(self):
         self.setup_accessibility()
         self.run_element_test(self.invalid_elementIDs,
                               lambda button: self.assertRaises(ElementNotAccessibleException,
                                                                button.tap))
         self.run_element_test(self.falsy_elements,
-                              lambda button: self.assertRaises(ElementNotAccessibleException,
+                              lambda button: self.assertRaises(ElementNotVisibleException,
                                                                button.tap))
 
     def test_single_tap_raises_no_exceptions(self):
         self.setup_accessibility(False, True)
         # No exception should be raised
         self.run_element_test(self.invalid_elementIDs, lambda button: button.tap())
         # Elements are invisible
         self.run_element_test(self.falsy_elements,
@@ -115,17 +115,17 @@ class TestAccessibility(MarionetteTestCa
         self.run_element_test(self.valid_elementIDs, lambda button: button.click())
 
     def test_click_raises_element_not_accessible(self):
         self.setup_accessibility()
         self.run_element_test(self.invalid_elementIDs,
                               lambda button: self.assertRaises(ElementNotAccessibleException,
                                                                button.click))
         self.run_element_test(self.falsy_elements,
-                              lambda button: self.assertRaises(ElementNotAccessibleException,
+                              lambda button: self.assertRaises(ElementNotVisibleException,
                                                                button.click))
 
     def test_click_raises_no_exceptions(self):
         self.setup_accessibility(False, True)
         # No exception should be raised
         self.run_element_test(self.invalid_elementIDs, lambda button: button.click())
         # Elements are invisible
         self.run_element_test(self.falsy_elements,
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -1798,17 +1798,17 @@ GeckoDriver.prototype.singleTap = functi
   let {id, x, y} = cmd.parameters;
 
   switch (this.context) {
     case Context.CHROME:
       throw new WebDriverError("Command 'singleTap' is not available in chrome context");
 
     case Context.CONTENT:
       this.addFrameCloseListener("tap");
-      yield this.listener.singleTap({id: id, corx: x, cory: y});
+      yield this.listener.singleTap(id, x, y);
       break;
   }
 };
 
 /**
  * An action chain.
  *
  * @param {Object} value
--- a/testing/marionette/elements.js
+++ b/testing/marionette/elements.js
@@ -93,17 +93,18 @@ Accessibility.prototype = {
    * @param nsIDOMElement element
    * @param Boolean mustHaveAccessible a flag indicating that the element must
    * have an accessible object
    * @return nsIAccessible object for the element
    */
   getAccessibleObject(element, mustHaveAccessible = false) {
     let acc = this.accessibleRetrieval.getAccessibleFor(element);
     if (!acc && mustHaveAccessible) {
-      this.handleErrorMessage('Element does not have an accessible object');
+      this.handleErrorMessage('Element does not have an accessible object',
+        element);
     }
     return acc;
   },
 
   /**
    * Check if the accessible has a role that supports some action
    * @param nsIAccessible object
    * @return Boolean an indicator of role being actionable
@@ -173,21 +174,25 @@ Accessibility.prototype = {
       accessible = accessible.parent;
     }
     return false;
   },
 
   /**
    * Send an error message or log the error message in the log
    * @param String message
+   * @param DOMElement element that caused an error
    */
-  handleErrorMessage(message) {
+  handleErrorMessage(message, element) {
     if (!message) {
       return;
     }
+    if (element) {
+      message += ` -> id: ${element.id}, tagName: ${element.tagName}, className: ${element.className}\n`;
+    }
     if (this.strict) {
       throw new ElementNotAccessibleError(message);
     }
     dump(Date.now() + " Marionette: " + message);
   }
 };
 
 this.ElementManager = function ElementManager(notSupported) {
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -210,27 +210,28 @@ let getCurrentUrlFn = dispatch(getCurren
 let findElementContentFn = dispatch(findElementContent);
 let findElementsContentFn = dispatch(findElementsContent);
 let isElementSelectedFn = dispatch(isElementSelected);
 let clearElementFn = dispatch(clearElement);
 let isElementDisplayedFn = dispatch(isElementDisplayed);
 let getElementValueOfCssPropertyFn = dispatch(getElementValueOfCssProperty);
 let switchToShadowRootFn = dispatch(switchToShadowRoot);
 let getCookiesFn = dispatch(getCookies);
+let singleTapFn = dispatch(singleTap);
 
 /**
  * Start all message listeners
  */
 function startListeners() {
   addMessageListenerId("Marionette:receiveFiles", receiveFiles);
   addMessageListenerId("Marionette:newSession", newSession);
   addMessageListenerId("Marionette:executeScript", executeScript);
   addMessageListenerId("Marionette:executeAsyncScript", executeAsyncScript);
   addMessageListenerId("Marionette:executeJSScript", executeJSScript);
-  addMessageListenerId("Marionette:singleTap", singleTap);
+  addMessageListenerId("Marionette:singleTap", singleTapFn);
   addMessageListenerId("Marionette:actionChain", actionChain);
   addMessageListenerId("Marionette:multiAction", multiAction);
   addMessageListenerId("Marionette:get", get);
   addMessageListenerId("Marionette:pollForReadyState", pollForReadyState);
   addMessageListenerId("Marionette:cancelRequest", cancelRequest);
   addMessageListenerId("Marionette:getCurrentUrl", getCurrentUrlFn);
   addMessageListenerId("Marionette:getTitle", getTitleFn);
   addMessageListenerId("Marionette:getPageSource", getPageSourceFn);
@@ -324,17 +325,17 @@ function restart(msg) {
  * Removes all listeners
  */
 function deleteSession(msg) {
   removeMessageListenerId("Marionette:receiveFiles", receiveFiles);
   removeMessageListenerId("Marionette:newSession", newSession);
   removeMessageListenerId("Marionette:executeScript", executeScript);
   removeMessageListenerId("Marionette:executeAsyncScript", executeAsyncScript);
   removeMessageListenerId("Marionette:executeJSScript", executeJSScript);
-  removeMessageListenerId("Marionette:singleTap", singleTap);
+  removeMessageListenerId("Marionette:singleTap", singleTapFn);
   removeMessageListenerId("Marionette:actionChain", actionChain);
   removeMessageListenerId("Marionette:multiAction", multiAction);
   removeMessageListenerId("Marionette:get", get);
   removeMessageListenerId("Marionette:pollForReadyState", pollForReadyState);
   removeMessageListenerId("Marionette:cancelRequest", cancelRequest);
   removeMessageListenerId("Marionette:getTitle", getTitleFn);
   removeMessageListenerId("Marionette:getPageSource", getPageSourceFn);
   removeMessageListenerId("Marionette:getCurrentUrl", getCurrentUrlFn);
@@ -909,44 +910,37 @@ function checkVisible(el, x, y) {
   }
   return true;
 }
 
 
 /**
  * Function that perform a single tap
  */
-function singleTap(msg) {
-  let command_id = msg.json.command_id;
-  try {
-    let el = elementManager.getKnownElement(msg.json.id, curContainer);
-    let acc = accessibility.getAccessibleObject(el, true);
-    // after this block, the element will be scrolled into view
-    let visible = checkVisible(el, msg.json.corx, msg.json.cory);
-    checkVisibleAccessibility(acc, visible);
-    if (!visible) {
-      sendError(new ElementNotVisibleError("Element is not currently visible and may not be manipulated"), command_id);
-      return;
-    }
-    checkActionableAccessibility(acc);
-    if (!curContainer.frame.document.createTouch) {
-      actions.mouseEventsOnly = true;
-    }
-    let c = coordinates(el, msg.json.corx, msg.json.cory);
-    if (!actions.mouseEventsOnly) {
-      let touchId = actions.nextTouchId++;
-      let touch = createATouch(el, c.x, c.y, touchId);
-      emitTouchEvent('touchstart', touch);
-      emitTouchEvent('touchend', touch);
-    }
-    actions.mouseTap(el.ownerDocument, c.x, c.y);
-    sendOk(command_id);
-  } catch (e) {
-    sendError(e, command_id);
+function singleTap(id, corx, cory) {
+  let el = elementManager.getKnownElement(id, curContainer);
+  // after this block, the element will be scrolled into view
+  let visible = checkVisible(el, corx, cory);
+  if (!visible) {
+    throw new ElementNotVisibleError("Element is not currently visible and may not be manipulated");
   }
+  let acc = accessibility.getAccessibleObject(el, true);
+  checkVisibleAccessibility(acc, el, visible);
+  checkActionableAccessibility(acc, el);
+  if (!curContainer.frame.document.createTouch) {
+    actions.mouseEventsOnly = true;
+  }
+  let c = coordinates(el, corx, cory);
+  if (!actions.mouseEventsOnly) {
+    let touchId = actions.nextTouchId++;
+    let touch = createATouch(el, c.x, c.y, touchId);
+    emitTouchEvent('touchstart', touch);
+    emitTouchEvent('touchend', touch);
+  }
+  actions.mouseTap(el.ownerDocument, c.x, c.y);
 }
 
 /**
  * Check if the element's unavailable accessibility state matches the enabled
  * state
  * @param nsIAccessible object
  * @param WebElement corresponding to nsIAccessible object
  * @param Boolean enabled element's enabled state
@@ -964,87 +958,90 @@ function checkEnabledAccessibility(acces
   if (!explorable && !disabledAccessibility) {
     message = 'Element is enabled but is not explorable via the ' +
       'accessibility API';
   } else if (enabled && disabledAccessibility) {
     message = 'Element is enabled but disabled via the accessibility API';
   } else if (!enabled && !disabledAccessibility) {
     message = 'Element is disabled but enabled via the accessibility API';
   }
-  accessibility.handleErrorMessage(message);
+  accessibility.handleErrorMessage(message, element);
 }
 
 /**
  * Check if the element's visible state corresponds to its accessibility API
  * visibility
  * @param nsIAccessible object
+ * @param WebElement corresponding to nsIAccessible object
  * @param Boolean visible element's visibility state
  */
-function checkVisibleAccessibility(accesible, visible) {
+function checkVisibleAccessibility(accesible, element, visible) {
   if (!accesible) {
     return;
   }
   let hiddenAccessibility = accessibility.isHidden(accesible);
   let message;
   if (visible && hiddenAccessibility) {
     message = 'Element is not currently visible via the accessibility API ' +
       'and may not be manipulated by it';
   } else if (!visible && !hiddenAccessibility) {
     message = 'Element is currently only visible via the accessibility API ' +
       'and can be manipulated by it';
   }
-  accessibility.handleErrorMessage(message);
+  accessibility.handleErrorMessage(message, element);
 }
 
 /**
  * Check if it is possible to activate an element with the accessibility API
  * @param nsIAccessible object
+ * @param WebElement corresponding to nsIAccessible object
  */
-function checkActionableAccessibility(accesible) {
+function checkActionableAccessibility(accesible, element) {
   if (!accesible) {
     return;
   }
   let message;
   if (!accessibility.hasActionCount(accesible)) {
     message = 'Element does not support any accessible actions';
   } else if (!accessibility.isActionableRole(accesible)) {
     message = 'Element does not have a correct accessibility role ' +
       'and may not be manipulated via the accessibility API';
   } else if (!accessibility.hasValidName(accesible)) {
     message = 'Element is missing an accesible name';
   } else if (!accessibility.matchState(accesible, 'STATE_FOCUSABLE')) {
     message = 'Element is not focusable via the accessibility API';
   }
-  accessibility.handleErrorMessage(message);
+  accessibility.handleErrorMessage(message, element);
 }
 
 /**
  * Check if element's selected state corresponds to its accessibility API
  * selected state.
  * @param nsIAccessible object
+ * @param WebElement corresponding to nsIAccessible object
  * @param Boolean selected element's selected state
  */
-function checkSelectedAccessibility(accessible, selected) {
+function checkSelectedAccessibility(accessible, element, selected) {
   if (!accessible) {
     return;
   }
   if (!accessibility.matchState(accessible, 'STATE_SELECTABLE')) {
     // Element is not selectable via the accessibility API
     return;
   }
 
   let selectedAccessibility = accessibility.matchState(
     accessible, 'STATE_SELECTED');
   let message;
   if (selected && !selectedAccessibility) {
     message = 'Element is selected but not selected via the accessibility API';
   } else if (!selected && selectedAccessibility) {
     message = 'Element is not selected but selected via the accessibility API';
   }
-  accessibility.handleErrorMessage(message);
+  accessibility.handleErrorMessage(message, element);
 }
 
 
 /**
  * Function to create a touch based on the element
  * corx and cory are relative to the viewport, id is the touchId
  */
 function createATouch(el, corx, cory, touchId) {
@@ -1443,25 +1440,26 @@ function getActiveElement() {
 /**
  * Send click event to element.
  *
  * @param {WebElement} id
  *     Reference to the web element to click.
  */
 function clickElement(id) {
   let el = elementManager.getKnownElement(id, curContainer);
-  let acc = accessibility.getAccessibleObject(el, true);
   let visible = checkVisible(el);
-  checkVisibleAccessibility(acc, visible);
   if (!visible) {
     throw new ElementNotVisibleError("Element is not visible");
   }
-  checkActionableAccessibility(acc);
+  let acc = accessibility.getAccessibleObject(el, true);
+  checkVisibleAccessibility(acc, el, visible);
+
   if (utils.isElementEnabled(el)) {
     checkEnabledAccessibility(acc, el, true);
+    checkActionableAccessibility(acc, el);
     utils.synthesizeMouseAtCenter(el, {}, el.ownerDocument.defaultView);
   } else {
     throw new InvalidElementStateError("Element is not Enabled");
   }
 }
 
 /**
  * Get a given attribute of an element.
@@ -1511,17 +1509,18 @@ function getElementTagName(id) {
  * Determine the element displayedness of the given web element.
  *
  * Also performs additional accessibility checks if enabled by session
  * capability.
  */
 function isElementDisplayed(id) {
   let el = elementManager.getKnownElement(id, curContainer);
   let displayed = utils.isElementDisplayed(el);
-  checkVisibleAccessibility(accessibility.getAccessibleObject(el), displayed);
+  checkVisibleAccessibility(
+    accessibility.getAccessibleObject(el), el, displayed);
   return displayed;
 }
 
 /**
  * Retrieves the computed value of the given CSS property of the given
  * web element.
  *
  * @param {String} id
@@ -1579,32 +1578,34 @@ function isElementEnabled(id) {
  * Determines if the referenced element is selected or not.
  *
  * This operation only makes sense on input elements of the Checkbox-
  * and Radio Button states, or option elements.
  */
 function isElementSelected(id) {
   let el = elementManager.getKnownElement(id, curContainer);
     let selected = utils.isElementSelected(el);
-    checkSelectedAccessibility(accessibility.getAccessibleObject(el), selected);
+    checkSelectedAccessibility(
+      accessibility.getAccessibleObject(el), el, selected);
   return selected;
 }
 
 /**
  * Send keys to element
  */
 function sendKeysToElement(msg) {
   let command_id = msg.json.command_id;
   let val = msg.json.value;
 
   try {
     let el = elementManager.getKnownElement(msg.json.id, curContainer);
     // Element should be actionable from the accessibility standpoint to be able
     // to send keys to it.
-    checkActionableAccessibility(accessibility.getAccessibleObject(el, true));
+    checkActionableAccessibility(
+      accessibility.getAccessibleObject(el, true), el);
     if (el.type == "file") {
       let p = val.join("");
       fileInputElement = el;
       // In e10s, we can only construct File objects in the parent process,
       // so pass the filename to driver.js, which in turn passes them back
       // to this frame script in receiveFiles.
       sendSyncMessage("Marionette:getFiles",
                       {value: p, command_id: command_id});