748069 - [AccessFu] Document UtteranceGenerator

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Eitan Isaacson [:eeejay]]

Add some much needed documentation to UtteranceGenerator.

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Document UtteranceGenerator.

Comment 2

[alexander :surkov (:asurkov)]

Comment on attachment 617604 [details] [diff] [review]
Document UtteranceGenerator.

Review of attachment 617604 [details] [diff] [review]:
-----------------------------------------------------------------

thank you for doing this

::: accessible/src/jsat/AccessFu.jsm
@@ +52,5 @@
>  
>    /**
> +   * Start the special AccessFu mode, this primarily means controlling the
> +   * virtual cursor with arrow keys. Currently, on platforms other than
> +   * Android this needs to be called explicitly.

it makes sense to keep that "Currently, ..." under @note section

::: accessible/src/jsat/UtteranceGenerator.jsm
@@ +27,5 @@
> + * An utterance is an array of strings.
> + *
> + * It should not be assumed that flattening an utterance array would create a
> + * gramatically correct sentence. For example:
> + * ['graphic', 'Welcome to my home page'].

makes sense to say what "objects, actions or state change" results in these strings

@@ +29,5 @@
> + * It should not be assumed that flattening an utterance array would create a
> + * gramatically correct sentence. For example:
> + * ['graphic', 'Welcome to my home page'].
> + * Each string element in an utterance should be gramatically correct in itself.
> + * For example: ['list item 2 of 5', 'Alabama'].

from this example, it is not clear how to use it. you might want to show an example in the end

@@ +32,5 @@
> + * Each string element in an utterance should be gramatically correct in itself.
> + * For example: ['list item 2 of 5', 'Alabama'].
> + *
> + * An utterance is ordered from the least to the most important. Speaking the
> + * last string usually makes sense, but speaking the first often won't.

sort of strange that last is more important than the first one, sort of rtl? :)

@@ +34,5 @@
> + *
> + * An utterance is ordered from the least to the most important. Speaking the
> + * last string usually makes sense, but speaking the first often won't.
> + * For example ['button', 'clicked'] for a clicked event. Speaking only
> + * 'clicked' makes sense. Speaking 'button' does not.

if it doesn't make sense to speak something then what sense does it make to expose it. It's not clear how the caller should decide what to speak and what to not.

@@ +59,5 @@
> +   * Generates an utterance for an object.
> +   * @param {nsIAccessible} aAccessible accessible object to generate utterance
> +   *    for.
> +   * @param {boolean} aForceName include the object's name in the utterance
> +   *    even if this object type does not usually have it's name uttered.

a ref to object that doesn't have uttered name would be nice, because I don't have any clue when I should force name and when I shouldn't do that

@@ +83,5 @@
> +  /**
> +   * Generates an utterance for an action performed.
> +   * TODO: May become more verbose in the future.
> +   * @param {nsIAccessible} aAccessible accessible object that was the target
> +   *    of the action.

the accessible object that the action was invoked on

@@ +86,5 @@
> +   * @param {nsIAccessible} aAccessible accessible object that was the target
> +   *    of the action.
> +   * @param {string} aActionName the name of the action, one of the keys in
> +   *    {@link gActionMap}.
> +   * @return {Array} A one string array with the action.

btw, it's still not clear why you need an array for one item

@@ +98,5 @@
> +   * @param {nsIAccessible} aAccessible accessible object of the tab's attached
> +   *    document.
> +   * @param {string} aTabState the tab state name, see
> +   *    {@link Presenter.tabStateChanged}.
> +   * @return {Array} The tab state utterace.

same here

Comment 3

[Eitan Isaacson [:eeejay]]

(In reply to alexander :surkov from comment #2)
> Comment on attachment 617604 [details] [diff] [review]
> Document UtteranceGenerator.
> 
> Review of attachment 617604 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> thank you for doing this
> 
> ::: accessible/src/jsat/AccessFu.jsm
> @@ +52,5 @@
> >  
> >    /**
> > +   * Start the special AccessFu mode, this primarily means controlling the
> > +   * virtual cursor with arrow keys. Currently, on platforms other than
> > +   * Android this needs to be called explicitly.
> 
> it makes sense to keep that "Currently, ..." under @note section
> 

I'm throwing that sentence away since it is no longer true.

> 
> @@ +29,5 @@
> > + * It should not be assumed that flattening an utterance array would create a
> > + * gramatically correct sentence. For example:
> > + * ['graphic', 'Welcome to my home page'].
> > + * Each string element in an utterance should be gramatically correct in itself.
> > + * For example: ['list item 2 of 5', 'Alabama'].
> 
> from this example, it is not clear how to use it. you might want to show an
> example in the end
> 

It is not an example of how to use it, just an example of what to expect (grammar that is internal to the elements, but not in between elements).

> @@ +32,5 @@
> > + * Each string element in an utterance should be gramatically correct in itself.
> > + * For example: ['list item 2 of 5', 'Alabama'].
> > + *
> > + * An utterance is ordered from the least to the most important. Speaking the
> > + * last string usually makes sense, but speaking the first often won't.
> 
> sort of strange that last is more important than the first one, sort of rtl?
> :)
> 

true. that might change in the future if we decide to put the description after the object name, or at least make that a preference. i have seen it done in both ways in different platforms and screen readers.

this also has to do with the fact that the "read everything under this container accessible" feature has not yet been implemented here, although i prototyped it in the past. it uses preorder traversal for the subtree (because that is how nsIAccessiblePivot works). So preorder dictates description (parent) first, name second.

anyway, we will cross that bridge when we reach it.

> @@ +34,5 @@
> > + *
> > + * An utterance is ordered from the least to the most important. Speaking the
> > + * last string usually makes sense, but speaking the first often won't.
> > + * For example ['button', 'clicked'] for a clicked event. Speaking only
> > + * 'clicked' makes sense. Speaking 'button' does not.
> 
> if it doesn't make sense to speak something then what sense does it make to
> expose it. It's not clear how the caller should decide what to speak and
> what to not.
> 

it is on a case by case bases, each generator uses the rule differently.

> @@ +59,5 @@
> > +   * Generates an utterance for an object.
> > +   * @param {nsIAccessible} aAccessible accessible object to generate utterance
> > +   *    for.
> > +   * @param {boolean} aForceName include the object's name in the utterance
> > +   *    even if this object type does not usually have it's name uttered.
> 
> a ref to object that doesn't have uttered name would be nice, because I
> don't have any clue when I should force name and when I shouldn't do that
> 

I changed the return description to:
   * @return {Array} Two string array. The first string describes the object
   *    and its states. The second string is the object's name. Some object
   *    types may have the description or name omitted, instead an empty string
   *    is returned as a placeholder. Whether the object's description or it's role
   *    is included is determined by {@link verbosityRoleMap}.

> @@ +86,5 @@
> > +   * @param {nsIAccessible} aAccessible accessible object that was the target
> > +   *    of the action.
> > +   * @param {string} aActionName the name of the action, one of the keys in
> > +   *    {@link gActionMap}.
> > +   * @return {Array} A one string array with the action.
> 
> btw, it's still not clear why you need an array for one item
> 

besides the TODO above it. i gave a lengthy description defining an utterance as an array.

Comment 4

[Eitan Isaacson [:eeejay]]

Attachment: Bug 748069 - Document UtteranceGenerator. r=surkov

Comment 5

[alexander :surkov (:asurkov)]

(In reply to Eitan Isaacson [:eeejay] from comment #3)

> > 
> > @@ +29,5 @@
> > > + * It should not be assumed that flattening an utterance array would create a
> > > + * gramatically correct sentence. For example:
> > > + * ['graphic', 'Welcome to my home page'].
> > > + * Each string element in an utterance should be gramatically correct in itself.
> > > + * For example: ['list item 2 of 5', 'Alabama'].
> > 
> > from this example, it is not clear how to use it. you might want to show an
> > example in the end
> > 
> 
> It is not an example of how to use it, just an example of what to expect
> (grammar that is internal to the elements, but not in between elements).

I meant the information ordering. I'd say that
#1 is Generates speech utterances from objects, actions and state changes. An utterance is an array of strings.
#2 is An utterance is ordered from the least to the most important...
#3 is It should not be assumed that flattening an utterance array would create a gramatically correct sentence...

> > sort of strange that last is more important than the first one, sort of rtl?
> > :)
> > 
> 
> true. that might change in the future if we decide to put the description
> after the object name, or at least make that a preference. i have seen it
> done in both ways in different platforms and screen readers.

I'd say it should depend on how easy to use it.
#1. get most important announcement: 
var array = generator.genForObject(acc);
return array[array.length - 1];
vs
generator.getForObject(acc)[0];

#2. speak everything:
return generator.getForObject(acc).reverse().join();
vs
return generator.getForObject(acc).join();

> this also has to do with the fact that the "read everything under this
> container accessible" feature has not yet been implemented here, although i
> prototyped it in the past. it uses preorder traversal for the subtree
> (because that is how nsIAccessiblePivot works). So preorder dictates
> description (parent) first, name second.

so you traverse objects from top to left-bottom, what is announcement order?

> > if it doesn't make sense to speak something then what sense does it make to
> > expose it. It's not clear how the caller should decide what to speak and
> > what to not.

> it is on a case by case bases, each generator uses the rule differently.

ok, you might want to say about this

> I changed the return description to:
>    * @return {Array} Two string array. The first string describes the object
>    *    and its states. The second string is the object's name. Some object
>    *    types may have the description or name omitted, instead an empty
> string
>    *    is returned as a placeholder. 

it's not clear what is returned as a placeholder.

Comment 6

[alexander :surkov (:asurkov)]

Comment on attachment 621155 [details] [diff] [review]
Bug 748069 - Document UtteranceGenerator. r=surkov

Review of attachment 621155 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/jsat/AccessFu.jsm
@@ +53,5 @@
>    },
>  
>    /**
> +   * Start the special AccessFu mode, this primarily means controlling the
> +   * virtual cursor with arrow keys.

btw, the comment makes me think there's 'not special AccessFu mode' or at least AccessFu have different modes

Comment 7

[Eitan Isaacson [:eeejay]]

(In reply to alexander :surkov from comment #6)
> Comment on attachment 621155 [details] [diff] [review]
> Bug 748069 - Document UtteranceGenerator. r=surkov
> 
> Review of attachment 621155 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/jsat/AccessFu.jsm
> @@ +53,5 @@
> >    },
> >  
> >    /**
> > +   * Start the special AccessFu mode, this primarily means controlling the
> > +   * virtual cursor with arrow keys.
> 
> btw, the comment makes me think there's 'not special AccessFu mode' or at
> least AccessFu have different modes

Removed "the special".

Comment 8

[Eitan Isaacson [:eeejay]]

Attachment: Document UtteranceGenerator. r=surkov

Comment 9

[Eitan Isaacson [:eeejay]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/bc7f3a9deba8

Comment 10

[Ryan VanderMeulen [:RyanVM]]

http://hg.mozilla.org/mozilla-central/rev/bc7f3a9deba8