Closed
Bug 1003491
Opened 10 years ago
Closed 10 years ago
[AccessFu] Change traversal rule to land on the parent of simple subtrees
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: eeejay, Assigned: eeejay)
Details
Attachments
(1 file)
12.04 KB,
patch
|
yzen
:
review+
|
Details | Diff | Splinter Review |
A simple subtree either has a single child lineage or one generation of children.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8414806 -
Flags: review?(yzenevich)
Comment 2•10 years ago
|
||
Comment on attachment 8414806 [details] [diff] [review] Make traversal rule land on bigger items that have simple subtrees. Review of attachment 8414806 [details] [diff] [review]: ----------------------------------------------------------------- Nice. A couple of nits and it works with my patch for status bar quite well. ::: accessible/src/jsat/OutputGenerator.jsm @@ +598,5 @@ > rowheader: function rowheader() { > return this.objectOutputFunctions.cell.apply(this, arguments); > + }, > + > + statictext: function statictext(aAccessible, aRoleStr, aState, aFlags) { Nit: aRoleStr, aState, aFlags are unused arguments. @@ +608,2 @@ > } > + Nit: whitespace. ::: accessible/src/jsat/TraversalRules.jsm @@ +33,5 @@ > this._matchRoles = aRoles; > if (aRoles.indexOf(Roles.LABEL) < 0) { > this._matchRoles.push(Roles.LABEL); > } > + this._matchFunc = aMatchFunc || function () { return Filters.MATCH; }; Nit: extra space - function() @@ +102,5 @@ > var gSimpleMatchFunc = function gSimpleMatchFunc(aAccessible) { > + // An object is simple, if it either has a single child lineage, > + // or has a flat subtree. > + function isSimpleObject (acc) { > + function isSingleLineage() { I would not define isSingleLineage and isFlatSubtree every time we call isSimpleObject. Lets have them outside of its scope. @@ +140,2 @@ > // Ignore prefix static text in list items. They are typically bullets or numbers. > + if (Utils.isListItemDecorator(aAccessible)) { Since you are using "? :" pattern on line 136 lets use it here as well.
Attachment #8414806 -
Flags: review?(yzenevich) → review+
Comment 3•10 years ago
|
||
Comment on attachment 8414806 [details] [diff] [review] Make traversal rule land on bigger items that have simple subtrees. Nice! :-)
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to Yura Zenevich [:yzen] from comment #2) > ::: accessible/src/jsat/OutputGenerator.jsm > @@ +598,5 @@ > > rowheader: function rowheader() { > > return this.objectOutputFunctions.cell.apply(this, arguments); > > + }, > > + > > + statictext: function statictext(aAccessible, aRoleStr, aState, aFlags) { > > Nit: aRoleStr, aState, aFlags are unused arguments. Generally, I would like us to be more strict about preserving the function prototype no matter what argument is used or not. It is one of the pitfalls of dynamic languages that you start obfuscating things because you could. Removing the arguments in the patch I am about to land because most of the file conforms to what you suggest, and it just stands out...
Assignee | ||
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c3758dd1ba8
Assignee: nobody → eitan
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8c3758dd1ba8
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•