Bug 915458 - Land on nesting labels instead of their children, and present them correctly. r=yzen
authorEitan Isaacson <eitan@monotonous.org>
Mon, 14 Oct 2013 12:56:19 -0700
changeset 164528 9e39971f3bb3e948338209ae3a7149eb2e4cc934
parent 164527 8d7c1598c14db31f88b84f1e66a0d6433cb1ef4e
child 164529 0e82e8f1c85fd4c086774eb77e1616124b372e4e
push id3066
push userakeybl@mozilla.com
push dateMon, 09 Dec 2013 19:58:46 +0000
treeherdermozilla-beta@a31a0dce83aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyzen
bugs915458
milestone27.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 915458 - Land on nesting labels instead of their children, and present them correctly. r=yzen
accessible/src/jsat/OutputGenerator.jsm
accessible/src/jsat/Presentation.jsm
accessible/src/jsat/TraversalRules.jsm
accessible/src/jsat/Utils.jsm
accessible/src/jsat/content-script.js
accessible/tests/mochitest/jsat/test_utterance_order.html
--- a/accessible/src/jsat/OutputGenerator.jsm
+++ b/accessible/src/jsat/OutputGenerator.jsm
@@ -310,16 +310,27 @@ this.OutputGenerator = {
       }
 
       this._addName(output, aAccessible, aFlags);
       this._addLandmark(output, aAccessible);
 
       return output;
     },
 
+    label: function label(aAccessible, aRoleStr, aStates, aFlags, aContext) {
+      if (aContext.isNestedControl ||
+          aContext.accessible == Utils.getEmbeddedControl(aAccessible)) {
+        // If we are on a nested control, or a nesting label,
+        // we don't need the context.
+        return [];
+      }
+
+      return this.objectOutputFunctions.defaultFunc.apply(this, arguments);
+    },
+
     entry: function entry(aAccessible, aRoleStr, aStates, aFlags) {
       let output = [];
       let desc = this._getLocalizedStates(aStates);
       desc.push(this._getLocalizedRole(
                   (aStates.ext & Ci.nsIAccessibleStates.EXT_STATE_MULTI_LINE) ?
                     'textarea' : 'entry'));
 
       output.push(desc.join(' '));
--- a/accessible/src/jsat/Presentation.jsm
+++ b/accessible/src/jsat/Presentation.jsm
@@ -158,30 +158,31 @@ VisualPresenter.prototype = {
       };
     }
 
     return null;
   },
 
   pivotChanged: function VisualPresenter_pivotChanged(aContext, aReason) {
     this._displayedAccessibles.set(aContext.accessible.document.window,
-                                   { accessible: aContext.accessible,
+                                   { accessible: aContext.accessibleForBounds,
                                      startOffset: aContext.startOffset,
                                      endOffset: aContext.endOffset });
 
-    if (!aContext.accessible)
+    if (!aContext.accessibleForBounds)
       return {type: this.type, details: {method: 'hideBounds'}};
 
     try {
-      aContext.accessible.scrollTo(
+      aContext.accessibleForBounds.scrollTo(
         Ci.nsIAccessibleScrollType.SCROLL_TYPE_ANYWHERE);
 
       let bounds = (aContext.startOffset === -1 && aContext.endOffset === -1) ?
-                   aContext.bounds : Utils.getTextBounds(aContext.accessible,
-                                     aContext.startOffset, aContext.endOffset);
+            aContext.bounds : Utils.getTextBounds(aContext.accessibleForBounds,
+                                                  aContext.startOffset,
+                                                  aContext.endOffset);
 
       return {
         type: this.type,
         details: {
           method: 'showBounds',
           bounds: bounds,
           padding: this.BORDER_PADDING
         }
--- a/accessible/src/jsat/TraversalRules.jsm
+++ b/accessible/src/jsat/TraversalRules.jsm
@@ -43,50 +43,65 @@ const ROLE_SLIDER = Ci.nsIAccessibleRole
 const ROLE_HEADING = Ci.nsIAccessibleRole.ROLE_HEADING;
 const ROLE_HEADER = Ci.nsIAccessibleRole.ROLE_HEADER;
 const ROLE_TERM = Ci.nsIAccessibleRole.ROLE_TERM;
 const ROLE_SEPARATOR = Ci.nsIAccessibleRole.ROLE_SEPARATOR;
 const ROLE_TABLE = Ci.nsIAccessibleRole.ROLE_TABLE;
 const ROLE_INTERNAL_FRAME = Ci.nsIAccessibleRole.ROLE_INTERNAL_FRAME;
 const ROLE_PARAGRAPH = Ci.nsIAccessibleRole.ROLE_PARAGRAPH;
 const ROLE_SECTION = Ci.nsIAccessibleRole.ROLE_SECTION;
+const ROLE_LABEL = Ci.nsIAccessibleRole.ROLE_LABEL;
 
 this.EXPORTED_SYMBOLS = ['TraversalRules'];
 
 Cu.import('resource://gre/modules/accessibility/Utils.jsm');
 Cu.import('resource://gre/modules/XPCOMUtils.jsm');
 
 let gSkipEmptyImages = new PrefCache('accessibility.accessfu.skip_empty_images');
 
 function BaseTraversalRule(aRoles, aMatchFunc) {
+  this._explicitMatchRoles = new Set(aRoles);
   this._matchRoles = aRoles;
-  this._matchFunc = aMatchFunc;
+  if (aRoles.indexOf(ROLE_LABEL) < 0) {
+    this._matchRoles.push(ROLE_LABEL);
+  }
+  this._matchFunc = aMatchFunc || function (acc) { return FILTER_MATCH; };
 }
 
 BaseTraversalRule.prototype = {
     getMatchRoles: function BaseTraversalRule_getmatchRoles(aRules) {
       aRules.value = this._matchRoles;
       return aRules.value.length;
     },
 
     preFilter: Ci.nsIAccessibleTraversalRule.PREFILTER_DEFUNCT |
     Ci.nsIAccessibleTraversalRule.PREFILTER_INVISIBLE |
     Ci.nsIAccessibleTraversalRule.PREFILTER_ARIA_HIDDEN,
 
     match: function BaseTraversalRule_match(aAccessible)
     {
-      if (aAccessible.role == ROLE_INTERNAL_FRAME) {
+      let role = aAccessible.role;
+      if (role == ROLE_INTERNAL_FRAME) {
         return (Utils.getMessageManager(aAccessible.DOMNode)) ?
           FILTER_MATCH  | FILTER_IGNORE_SUBTREE : FILTER_IGNORE;
       }
 
-      if (this._matchFunc)
-        return this._matchFunc(aAccessible);
+      let matchResult = this._explicitMatchRoles.has(role) ?
+          this._matchFunc(aAccessible) : FILTER_IGNORE;
 
-      return FILTER_MATCH;
+      // If we are on a label that nests a checkbox/radio we should land on it.
+      // It is a bigger touch target, and it reduces clutter.
+      if (role == ROLE_LABEL && !(matchResult & FILTER_IGNORE_SUBTREE)) {
+        let control = Utils.getEmbeddedControl(aAccessible);
+        if (control && this._explicitMatchRoles.has(control.role)) {
+          matchResult = this._matchFunc(control) | FILTER_IGNORE_SUBTREE;
+        }
+      }
+
+      return matchResult;
     },
 
     QueryInterface: XPCOMUtils.generateQI([Ci.nsIAccessibleTraversalRule])
 };
 
 var gSimpleTraversalRoles =
   [ROLE_MENUITEM,
    ROLE_LINK,
--- a/accessible/src/jsat/Utils.jsm
+++ b/accessible/src/jsat/Utils.jsm
@@ -9,16 +9,18 @@ const Cc = Components.classes;
 const Ci = Components.interfaces;
 
 const EVENT_STATE_CHANGE = Ci.nsIAccessibleEvent.EVENT_STATE_CHANGE;
 
 const ROLE_CELL = Ci.nsIAccessibleRole.ROLE_CELL;
 const ROLE_COLUMNHEADER = Ci.nsIAccessibleRole.ROLE_COLUMNHEADER;
 const ROLE_ROWHEADER = Ci.nsIAccessibleRole.ROLE_ROWHEADER;
 
+const RELATION_LABEL_FOR = Ci.nsIAccessibleRelation.RELATION_LABEL_FOR;
+
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, 'Services',
   'resource://gre/modules/Services.jsm');
 XPCOMUtils.defineLazyModuleGetter(this, 'Rect',
   'resource://gre/modules/Geometry.jsm');
 
 this.EXPORTED_SYMBOLS = ['Utils', 'Logger', 'PivotContext', 'PrefCache'];
 
@@ -286,16 +288,30 @@ this.Utils = {
     ];
     let roles = this.getAttributes(aAccessible)['xml-roles'];
     if (!roles) {
       return;
     }
 
     // Looking up a role that would match a landmark.
     return this.matchAttributeValue(roles, landmarks);
+  },
+
+  getEmbeddedControl: function getEmbeddedControl(aLabel) {
+    if (aLabel) {
+      let relation = aLabel.getRelationByType(RELATION_LABEL_FOR);
+      for (let i = 0; i < relation.targetsCount; i++) {
+        let target = relation.getTarget(i);
+        if (target.parent === aLabel) {
+          return target;
+        }
+      }
+    }
+
+    return null;
   }
 };
 
 this.Logger = {
   DEBUG: 0,
   INFO: 1,
   WARNING: 2,
   ERROR: 3,
@@ -420,38 +436,55 @@ this.Logger = {
     for (var i=0; i < aAccessible.childCount; i++)
       this._dumpTreeInternal(aLogLevel, aAccessible.getChildAt(i), aIndent + 1);
     }
 };
 
 /**
  * PivotContext: An object that generates and caches context information
  * for a given accessible and its relationship with another accessible.
+ *
+ * If the given accessible is a label for a nested control, then this
+ * context will represent the nested control instead of the label.
+ * With the exception of bounds calculation, which will use the containing
+ * label. In this case the |accessible| field would be the embedded control,
+ * and the |accessibleForBounds| field would be the label.
  */
 this.PivotContext = function PivotContext(aAccessible, aOldAccessible,
   aStartOffset, aEndOffset, aIgnoreAncestry = false,
   aIncludeInvisible = false) {
   this._accessible = aAccessible;
+  this._nestedControl = Utils.getEmbeddedControl(aAccessible);
   this._oldAccessible =
     this._isDefunct(aOldAccessible) ? null : aOldAccessible;
   this.startOffset = aStartOffset;
   this.endOffset = aEndOffset;
   this._ignoreAncestry = aIgnoreAncestry;
   this._includeInvisible = aIncludeInvisible;
 }
 
 PivotContext.prototype = {
   get accessible() {
-    return this._accessible;
+    // If the current pivot accessible has a nested control,
+    // make this context use it publicly.
+    return this._nestedControl || this._accessible;
   },
 
   get oldAccessible() {
     return this._oldAccessible;
   },
 
+  get isNestedControl() {
+    return !!this._nestedControl;
+  },
+
+  get accessibleForBounds() {
+    return this._accessible;
+  },
+
   get textAndAdjustedOffsets() {
     if (this.startOffset === -1 && this.endOffset === -1) {
       return null;
     }
 
     if (!this._textAndAdjustedOffsets) {
       let result = {startOffset: this.startOffset,
                     endOffset: this.endOffset,
@@ -516,17 +549,17 @@ PivotContext.prototype = {
   },
 
   /**
    * A list of the current accessible's ancestry.
    */
   get currentAncestry() {
     if (!this._currentAncestry) {
       this._currentAncestry = this._ignoreAncestry ? [] :
-        this._getAncestry(this._accessible);
+        this._getAncestry(this.accessible);
     }
     return this._currentAncestry;
   },
 
   /*
    * This is a list of the accessible's ancestry up to the common ancestor
    * of the accessible and the old accessible. It is useful for giving the
    * user context as to where they are in the heirarchy.
@@ -577,17 +610,17 @@ PivotContext.prototype = {
    * list of the accessible's subtree in pre or post order.
    * It only includes the accessible's visible chidren.
    * @param {boolean} aPreorder A flag for traversal order. If true, traverse
    * in preorder; if false, traverse in postorder.
    * @param {function} aStop An optional function, indicating whether subtree
    * traversal should stop.
    */
   subtreeGenerator: function subtreeGenerator(aPreorder, aStop) {
-    return this._traverse(this._accessible, aPreorder, aStop);
+    return this._traverse(this.accessible, aPreorder, aStop);
   },
 
   getCellInfo: function getCellInfo(aAccessible) {
     if (!this._cells) {
       this._cells = new WeakMap();
     }
 
     let domNode = aAccessible.DOMNode;
@@ -672,17 +705,17 @@ PivotContext.prototype = {
     }
 
     this._cells.set(domNode, cellInfo);
     return cellInfo;
   },
 
   get bounds() {
     if (!this._bounds) {
-      this._bounds = Utils.getBounds(this._accessible);
+      this._bounds = Utils.getBounds(this.accessibleForBounds);
     }
 
     return this._bounds.clone();
   },
 
   _isDefunct: function _isDefunct(aAccessible) {
     try {
       let extstate = {};
--- a/accessible/src/jsat/content-script.js
+++ b/accessible/src/jsat/content-script.js
@@ -168,16 +168,21 @@ function activateCurrent(aMessage) {
         aAccessible.role != Ci.nsIAccessibleRole.ROLE_KEY) {
       // Only activate keys, don't do anything on other objects.
       return;
     }
 
     if (aAccessible.actionCount > 0) {
       aAccessible.doAction(0);
     } else {
+      let control = Utils.getEmbeddedControl(aAccessible);
+      if (control && control.actionCount > 0) {
+        control.doAction(0);
+      }
+
       // XXX Some mobile widget sets do not expose actions properly
       // (via ARIA roles, etc.), so we need to generate a click.
       // Could possibly be made simpler in the future. Maybe core
       // engine could expose nsCoreUtiles::DispatchMouseEvent()?
       let docAcc = Utils.AccRetrieval.getAccessibleFor(content.document);
       let docX = {}, docY = {}, docW = {}, docH = {};
       docAcc.getBounds(docX, docY, docW, docH);
 
--- a/accessible/tests/mochitest/jsat/test_utterance_order.html
+++ b/accessible/tests/mochitest/jsat/test_utterance_order.html
@@ -134,16 +134,48 @@ https://bugzilla.mozilla.org/show_bug.cg
           accOrElmOrID: 'tab1',
           expected: [['tab list', 'selected tab 1 of 2', 'Account'],
             ['Account', 'selected tab 1 of 2', 'tab list']]
         }, {
           // Test unselected tab
           accOrElmOrID: 'tab2',
           expected: [['tab list', 'tab 2 of 2', 'Advanced'],
             ['Advanced', 'tab 2 of 2', 'tab list']]
+        },
+
+        {
+          // Landing on this label should mimic landing on the checkbox.
+          accOrElmOrID: "label1",
+          expected: [['not checked check button', 'Orange'],
+                     ['Orange', 'not checked check button']]
+        },
+        {
+          // Here we get a top-level view of the form.
+          accOrElmOrID: "form1",
+          expected: [['label', 'not checked check button', 'Orange', 'Orange',
+                      'not checked check button', 'Blue', 'label', 'Blue'],
+                     ['Orange', 'not checked check button', 'Orange', 'label',
+                      'Blue', 'not checked check button', 'Blue', 'label']]
+        },
+        {
+          // This is a non-nesting label.
+          accOrElmOrID: "label2",
+          expected: [['label', 'Blue'], ['Blue', 'label']]
+        },
+        {
+          // This is a distinct control.
+          accOrElmOrID: "input2",
+          expected: [['not checked check button', 'Blue'],
+                     ['Blue', 'not checked check button']]
+        },
+        {
+          // This is a nested control.
+          accOrElmOrID: "input1",
+          expected: [['not checked check button', 'Orange'],
+                     ['Orange', 'not checked check button']]
         }];
 
         // Test all possible utterance order preference values.
         tests.forEach(function run(test) {
           var utteranceOrderValues = [0, 1];
           utteranceOrderValues.forEach(
             function testUtteranceOrder(utteranceOrder) {
               SpecialPowers.setIntPref(PREF_UTTERANCE_ORDER, utteranceOrder);
@@ -213,11 +245,15 @@ https://bugzilla.mozilla.org/show_bug.cg
       <button id="expandedButton" aria-expanded="true">I am expanded</button>
       <button id="collapsedButton" aria-expanded="false">I am collapsed</button>
       <input id="requiredInput" required placeholder="I am required" />
       <button id="hasPopupButton" aria-haspopup="true">I have a popup</button>
       <div role="tablist">
         <a id="tab1" href="#" role="tab" aria-selected="true">Account</a>
         <a id="tab2" href="#" role="tab" aria-selected="false">Advanced</a>
       </div>
+      <form id="form1">
+        <label id="label1"><input id="input1" type="checkbox">Orange</label>
+        <input id="input2" type="checkbox"><label id="label2" for="input2">Blue</label>
+      </form>
     </div>
   </body>
 </html>