753986 - [AccessFu] Read virtual cursor position's subtree

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Eitan Isaacson [:eeejay]]

Right now we mostly read the role/state and accessible name. But we need to also support reading everything in the subtree. For example when the virtual cursor is on a landmark.

Comment 1

[Marco Zehe (:MarcoZ)]

(In reply to Eitan Isaacson [:eeejay] from comment #0)
> Right now we mostly read the role/state and accessible name. But we need to
> also support reading everything in the subtree. For example when the virtual
> cursor is on a landmark.

You mean when on a navigation landmark, read all the items that are contained within automatically? That can become very verbose.

Comment 2

[Eitan Isaacson [:eeejay]]

(In reply to Marco Zehe (:MarcoZ) from comment #1)
> (In reply to Eitan Isaacson [:eeejay] from comment #0)
> > Right now we mostly read the role/state and accessible name. But we need to
> > also support reading everything in the subtree. For example when the virtual
> > cursor is on a landmark.
> 
> You mean when on a navigation landmark, read all the items that are
> contained within automatically? That can become very verbose.

Well, maybe landmarks was a bad example. If the user is navigating by paragraphs, or by table cells. Once they land on the paragraph, shouldn't it just read everything and not stop at each link or formatting change?

Comment 3

[Eitan Isaacson [:eeejay]]

Or another example I found in nvda: If you navigate to a list item using "i", it will read the entire list item. If there are links or inline graphics, they will be read as well and prefixed with a description. So

Click <a href="#">here</a>.

Will be read as "Click link here period".

So this is what I mean by read subtree, in this case read the subtree of the list item.

Comment 4

[Eitan Isaacson [:eeejay]]

Attachment: Change presenter and utterance generator to be subtree-based.

To reiterate the goal of this patch: When the virtual cursor lands on a node, any node, the utterance should be able to present it in a detailed manner with no redundant phrases.

In the current SimpleTraversalRule we don't have this issue much because we mostly just land on leaves. But if, for example we land on a heading, we need to read the content under it. If we land on a list item, the subtree is already in its name, but we should probably still do this so we get descriptions of the subtree children like images and links. Same for links, we may land on a link node that has a text leaf and a graphic as children, etc.

Comment 5

[Marco Zehe (:MarcoZ)]

Comment on attachment 626995 [details] [diff] [review]
Change presenter and utterance generator to be subtree-based.

>+   * It only includes the accessible's visible chidren.

Nit: missing l in children.

Question: What will we speak if you have
<a href="bla" title="Link to something"><img src="foo.jpg" /></a>
Which is missing the name of the image since there's neither an alt text nor a title. But there *is* a title in the link itself, but the way I see it, we would stop speaking that as the acc name if we switch to the subtree based rule here.

This reminds me of an issue I'm regularly having with NVDA where such marked up links, which are not seldom, simply render as links without names in their virtual buffer, and only if I use object navigation, I get the name we calculate for the link accessible spoken. They use a similar technique, getting all the info they can from the sub tree, but in this case ignoring the link's name. I've gone over this with Jamie over and over, and so far there's not been a satisfying solution to this in NVDA.

Comment 6

[Eitan Isaacson [:eeejay]]

(In reply to Marco Zehe (:MarcoZ) from comment #5)
> Comment on attachment 626995 [details] [diff] [review]
> Change presenter and utterance generator to be subtree-based.
> 
> >+   * It only includes the accessible's visible chidren.
> 
> Nit: missing l in children.
> 
> Question: What will we speak if you have
> <a href="bla" title="Link to something"><img src="foo.jpg" /></a>
> Which is missing the name of the image since there's neither an alt text nor
> a title. But there *is* a title in the link itself, but the way I see it, we
> would stop speaking that as the acc name if we switch to the subtree based
> rule here.
> 

The link title ends up in the description, not the link name. So even now we don't speak it. We need to make a new link utterance function that would speak the description.

> This reminds me of an issue I'm regularly having with NVDA where such marked
> up links, which are not seldom, simply render as links without names in
> their virtual buffer, and only if I use object navigation, I get the name we
> calculate for the link accessible spoken. They use a similar technique,
> getting all the info they can from the sub tree, but in this case ignoring
> the link's name. I've gone over this with Jamie over and over, and so far
> there's not been a satisfying solution to this in NVDA.

The link name we provide is always the concatenated strings in the subtree, not the title. So ignoring the link name, and traversing the subtree instead should give a similar effect. As for the link title, that is another issue, and we should open a bug for that.

Comment 7

[David Bolter [:davidb] (NeedInfo me for attention)]

Note Eitan filed bug 758675 for link utterance improvement.

Comment 8

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 626995 [details] [diff] [review]
Change presenter and utterance generator to be subtree-based.

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

r=me (sorry for delay - I was mulling this over then forgot about it)

::: accessible/src/jsat/Presenters.jsm
@@ +239,5 @@
>  
>      let output = [];
> +    aContext.newAncestry.forEach(
> +      function (acc) {
> +        output.push.apply(output, UtteranceGenerator.genForObject(acc));

It looks like you can get rid of the second arg in you definition of:
genForObject: function genForObject(aAccessible, aForceName)

(as well as the line that adds the bit)

@@ +418,5 @@
> +      return list;
> +    }
> +
> +    if (!this._subtreePreOrder)
> +      this._subtreePreOrder = traversePreorder(this._accessible);

OK since (as per IRC) this is scoped to the presenter context and the life cycle is that of the event handler we don't need to worry about _subtreePreOrder staleness on reuse AFAICT.

Comment 9

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 626995 [details] [diff] [review]
Change presenter and utterance generator to be subtree-based.

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

::: accessible/src/jsat/UtteranceGenerator.jsm
@@ +67,2 @@
>     */
> +  genForObject: function genForObject(aAccessible) {

Doh! Ignore that comment :)

@@ +77,1 @@
>        flags |= INCLUDE_NAME;

Nice.

Comment 10

[David Bolter [:davidb] (NeedInfo me for attention)]

I scrolled down about 3/4's of the patch during review then forgot the final 1/4. Looks good.

Comment 11

[Eitan Isaacson [:eeejay]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/1b96b75f831a

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1b96b75f831a