[AccessFu] Read virtual cursor position's subtree

RESOLVED FIXED in mozilla15

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: eeejay, Assigned: eeejay)

Tracking

Trunk
mozilla15
ARM
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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

5 years ago
(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.
(Assignee)

Comment 2

5 years ago
(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?
(Assignee)

Comment 3

5 years ago
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.
(Assignee)

Comment 4

5 years ago
Created attachment 626995 [details] [diff] [review]
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.
Attachment #626995 - Flags: review?(dbolter)

Comment 5

5 years ago
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.
(Assignee)

Comment 6

5 years ago
(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.
Note Eitan filed bug 758675 for link utterance improvement.
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.
Attachment #626995 - Flags: review?(dbolter) → review+
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.
I scrolled down about 3/4's of the patch during review then forgot the final 1/4. Looks good.
(Assignee)

Comment 11

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/1b96b75f831a
Assignee: nobody → eitan
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/1b96b75f831a
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.