Bug 1003491 - Make traversal rule land on bigger items that have simple subtrees. r=yzen
authorEitan Isaacson <eitan@monotonous.org>
Wed, 30 Apr 2014 21:33:37 -0700
changeset 181537 8c3758dd1ba8cabb8df03180f646187b07c60d11
parent 181536 ed3ea6cb5afeb1ec05fa2dd9ba41aa1acd4e9fdc
child 181538 a973436eb53b2d977a5d9257c005fba6fb71befa
push id272
push userpvanderbeken@mozilla.com
push dateMon, 05 May 2014 16:31:18 +0000
reviewersyzen
bugs1003491
milestone32.0a1
Bug 1003491 - Make traversal rule land on bigger items that have simple subtrees. r=yzen
accessible/src/jsat/OutputGenerator.jsm
accessible/src/jsat/TraversalRules.jsm
accessible/src/jsat/Utils.jsm
accessible/tests/mochitest/jsat/test_output.html
accessible/tests/mochitest/jsat/test_traversal.html
--- a/accessible/src/jsat/OutputGenerator.jsm
+++ b/accessible/src/jsat/OutputGenerator.jsm
@@ -592,16 +592,24 @@ this.UtteranceGenerator = {
     },
 
     columnheader: function columnheader() {
       return this.objectOutputFunctions.cell.apply(this, arguments);
     },
 
     rowheader: function rowheader() {
       return this.objectOutputFunctions.cell.apply(this, arguments);
+    },
+
+    statictext: function statictext(aAccessible) {
+      if (Utils.isListItemDecorator(aAccessible, true)) {
+        return [];
+      }
+
+      return this.objectOutputFunctions.defaultFunc.apply(this, arguments);
     }
   },
 
   _getContextStart: function _getContextStart(aContext) {
     return aContext.newAncestry;
   },
 
   _getLocalizedRole: function _getLocalizedRole(aRoleStr) {
@@ -773,17 +781,17 @@ this.BrailleGenerator = {
 
     rowheader: function rowheader() {
       return this.objectOutputFunctions.cell.apply(this, arguments);
     },
 
     statictext: function statictext(aAccessible, aRoleStr, aState, aFlags) {
       // Since we customize the list bullet's output, we add the static
       // text from the first node in each listitem, so skip it here.
-      if (aAccessible.parent.role == Roles.LISTITEM) {
+      if (Utils.isListItemDecorator(aAccessible)) {
         return [];
       }
 
       return this.objectOutputFunctions._useStateNotRole.apply(this, arguments);
     },
 
     _useStateNotRole: function _useStateNotRole(aAccessible, aRoleStr, aState, aFlags) {
       let braille = [];
--- a/accessible/src/jsat/TraversalRules.jsm
+++ b/accessible/src/jsat/TraversalRules.jsm
@@ -1,41 +1,45 @@
 /* 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/. */
 
+/* global PrefCache, Roles, Prefilters, States, Filters, Utils,
+   TraversalRules */
+/* exported TraversalRules */
+
 'use strict';
 
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 const Cu = Components.utils;
 const Cr = Components.results;
 
-this.EXPORTED_SYMBOLS = ['TraversalRules'];
+this.EXPORTED_SYMBOLS = ['TraversalRules']; // jshint ignore:line
 
 Cu.import('resource://gre/modules/accessibility/Utils.jsm');
 Cu.import('resource://gre/modules/XPCOMUtils.jsm');
-XPCOMUtils.defineLazyModuleGetter(this, 'Roles',
+XPCOMUtils.defineLazyModuleGetter(this, 'Roles',  // jshint ignore:line
   'resource://gre/modules/accessibility/Constants.jsm');
-XPCOMUtils.defineLazyModuleGetter(this, 'Filters',
+XPCOMUtils.defineLazyModuleGetter(this, 'Filters',  // jshint ignore:line
   'resource://gre/modules/accessibility/Constants.jsm');
-XPCOMUtils.defineLazyModuleGetter(this, 'States',
+XPCOMUtils.defineLazyModuleGetter(this, 'States',  // jshint ignore:line
   'resource://gre/modules/accessibility/Constants.jsm');
-XPCOMUtils.defineLazyModuleGetter(this, 'Prefilters',
+XPCOMUtils.defineLazyModuleGetter(this, 'Prefilters',  // jshint ignore:line
   'resource://gre/modules/accessibility/Constants.jsm');
 
 let gSkipEmptyImages = new PrefCache('accessibility.accessfu.skip_empty_images');
 
 function BaseTraversalRule(aRoles, aMatchFunc, aPreFilter) {
   this._explicitMatchRoles = new Set(aRoles);
   this._matchRoles = aRoles;
   if (aRoles.indexOf(Roles.LABEL) < 0) {
     this._matchRoles.push(Roles.LABEL);
   }
-  this._matchFunc = aMatchFunc || function (acc) { return Filters.MATCH; };
+  this._matchFunc = aMatchFunc || function() { return Filters.MATCH; };
   this.preFilter = aPreFilter || gSimplePreFilter;
 }
 
 BaseTraversalRule.prototype = {
     getMatchRoles: function BaseTraversalRule_getmatchRoles(aRules) {
       aRules.value = this._matchRoles;
       return aRules.value.length;
     },
@@ -86,78 +90,87 @@ var gSimpleTraversalRoles =
    Roles.TOGGLE_BUTTON,
    Roles.ENTRY,
    Roles.KEY,
    Roles.HEADER,
    Roles.HEADING,
    Roles.SLIDER,
    Roles.SPINBUTTON,
    Roles.OPTION,
+   Roles.LISTITEM,
    // Used for traversing in to child OOP frames.
    Roles.INTERNAL_FRAME];
 
 var gSimpleMatchFunc = function gSimpleMatchFunc(aAccessible) {
-  function hasZeroOrSingleChildDescendants () {
-    for (let acc = aAccessible; acc.childCount > 0; acc = acc.firstChild) {
-      if (acc.childCount > 1) {
+  // An object is simple, if it either has a single child lineage,
+  // or has a flat subtree.
+  function isSingleLineage(acc) {
+    for (let child = acc; child; child = child.firstChild) {
+      if (child.childCount > 1) {
         return false;
       }
     }
+    return true;
+  }
 
+  function isFlatSubtree(acc) {
+    for (let child = acc.firstChild; child; child = child.nextSibling) {
+      if (child.childCount > 0) {
+        return false;
+      }
+    }
     return true;
   }
 
   switch (aAccessible.role) {
   case Roles.COMBOBOX:
     // We don't want to ignore the subtree because this is often
     // where the list box hangs out.
     return Filters.MATCH;
   case Roles.TEXT_LEAF:
     {
       // Nameless text leaves are boring, skip them.
       let name = aAccessible.name;
-      if (name && name.trim())
-        return Filters.MATCH;
-      else
-        return Filters.IGNORE;
+      return (name && name.trim()) ? Filters.MATCH : Filters.IGNORE;
     }
   case Roles.STATICTEXT:
-    {
-      let parent = aAccessible.parent;
-      // Ignore prefix static text in list items. They are typically bullets or numbers.
-      if (parent.childCount > 1 && aAccessible.indexInParent == 0 &&
-          parent.role == Roles.LISTITEM)
-        return Filters.IGNORE;
-
-      return Filters.MATCH;
-    }
+    // Ignore prefix static text in list items. They are typically bullets or numbers.
+    return Utils.isListItemDecorator(aAccessible) ?
+      Filters.IGNORE : Filters.MATCH;
   case Roles.GRAPHIC:
     return TraversalRules._shouldSkipImage(aAccessible);
   case Roles.HEADER:
   case Roles.HEADING:
     if ((aAccessible.childCount > 0 || aAccessible.name) &&
-        hasZeroOrSingleChildDescendants()) {
+        (isSingleLineage(aAccessible) || isFlatSubtree(aAccessible))) {
       return Filters.MATCH | Filters.IGNORE_SUBTREE;
-    } else {
-      return Filters.IGNORE;
+    }
+    return Filters.IGNORE;
+  case Roles.LISTITEM:
+    {
+      let item = aAccessible.childCount === 2 &&
+        aAccessible.firstChild.role === Roles.STATICTEXT ?
+        aAccessible.lastChild : aAccessible;
+        return isSingleLineage(item) || isFlatSubtree(item) ?
+          Filters.MATCH | Filters.IGNORE_SUBTREE : Filters.IGNORE;
     }
   default:
     // Ignore the subtree, if there is one. So that we don't land on
     // the same content that was already presented by its parent.
     return Filters.MATCH |
       Filters.IGNORE_SUBTREE;
   }
 };
 
 var gSimplePreFilter = Prefilters.DEFUNCT |
   Prefilters.INVISIBLE |
   Prefilters.ARIA_HIDDEN |
   Prefilters.TRANSPARENT;
 
-this.TraversalRules = {
+this.TraversalRules = { // jshint ignore:line
   Simple: new BaseTraversalRule(gSimpleTraversalRoles, gSimpleMatchFunc),
 
   SimpleOnScreen: new BaseTraversalRule(
     gSimpleTraversalRoles, gSimpleMatchFunc,
     Prefilters.DEFUNCT | Prefilters.INVISIBLE | Prefilters.ARIA_HIDDEN |
     Prefilters.TRANSPARENT | Prefilters.OFFSCREEN),
 
   Anchor: new BaseTraversalRule(
--- a/accessible/src/jsat/Utils.jsm
+++ b/accessible/src/jsat/Utils.jsm
@@ -338,16 +338,27 @@ this.Utils = {
         let target = relation.getTarget(i);
         if (target.parent === aLabel) {
           return target;
         }
       }
     }
 
     return null;
+  },
+
+  isListItemDecorator: function isListItemDecorator(aStaticText,
+                                                    aExcludeOrdered) {
+    let parent = aStaticText.parent;
+    if (aExcludeOrdered && parent.parent.DOMNode.nodeName === 'OL') {
+      return false;
+    }
+
+    return parent.role === Roles.LISTITEM && parent.childCount > 1 &&
+      aStaticText.indexInParent === 0;
   }
 };
 
 /**
  * State object used internally to process accessible's states.
  * @param {Number} aBase     Base state.
  * @param {Number} aExtended Extended state.
  */
--- a/accessible/tests/mochitest/jsat/test_output.html
+++ b/accessible/tests/mochitest/jsat/test_output.html
@@ -87,16 +87,27 @@ https://bugzilla.mozilla.org/show_bug.cg
           expectedUtterance: [
             ["list 1 item", "First item", "1.", "list one"],
             ["1.", "list one", "First item", "list 1 item"]
           ],
           expectedBraille: [
             ["1.", "list one"],
             ["1.", "list one"]
           ]
+        },
+        {
+          accOrElmOrID: "li_two",
+          expectedUtterance: [
+            ["list 1 item", "First item", "list two"],
+            ["list two", "First item", "list 1 item"]
+          ],
+          expectedBraille: [
+            ["*", "list two"],
+            ["*", "list two"]
+          ]
         }, {
           accOrElmOrID: "cell",
           expectedUtterance: [[
             "table with 1 column and 1 row", "Fruits and vegetables",
             "Column 1 Row 1", "list 4 items", "First item", "link", "Apples",
             "link", "Bananas", "link", "Peaches", "Last item", "link", "Plums"
           ], [
             "Apples", "link", "First item", "Bananas", "link", "Peaches",
@@ -402,16 +413,19 @@ https://bugzilla.mozilla.org/show_bug.cg
       <a id="anchor_arialabelandtext" href="#test" aria-label="Tests" title="goes to the tests">Tests</a>
       <textarea id="textarea" cols="80" rows="5">
         This is the text area text.
       </textarea>
       <h1 id="heading" title="Test heading"></h1>
       <ol id="list">
         <li id="li_one">list one</li>
       </ol>
+      <ul id="unorderd_list">
+        <li id="li_two">list two</li>
+      </ul>
       <dl id="dlist">
         <dd id="dd_one">
           dd one
         </dd>
       </dl>
       <table>
         <caption>Fruits and vegetables</caption>
         <tr>
--- a/accessible/tests/mochitest/jsat/test_traversal.html
+++ b/accessible/tests/mochitest/jsat/test_traversal.html
@@ -106,18 +106,18 @@
                               'Or me! ', 'Value 1', 'Value 2', 'Value 3',
                               'Electronic mailing address:', 'input-1-5',
                               'button-1-3', 'heading-2', 'heading-3',
                               'button-2-1', 'button-2-2', 'button-2-3',
                               'button-2-4', 'Programming Language',
                               'A esoteric weapon wielded by only the most ' +
                               'formidable warriors, for its unrelenting strict' +
                               ' power is unfathomable.',
-                              'Lists of Programming Languages', 'Lisp ',
-                              'Scheme', 'Racket', 'Clojure', 'JavaScript', 'heading-5',
+                              '• Lists of Programming Languages', 'Lisp ',
+                              '1. Scheme', '2. Racket', '3. Clojure', '• JavaScript', 'heading-5',
                               'image-2', 'image-3', 'Not actually an image',
                               'link-1', 'anchor-1', 'link-2', 'anchor-2', 'link-3',
                               '3', '1', '4', '1', 'Just an innocuous separator',
                               'Dirty Words', 'Meaning', 'Mud', 'Wet Dirt',
                               'Dirt', 'Messy Stuff']);
 
       gQueue.invoke();
     }