452767 - Accessible name subtree computation must not include a user-entered value if at start or end

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[James Teh [:Jamie]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/20080827031643 Minefield/3.1a2pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/20080827031643 Minefield/3.1a2pre ID:20080827031643

When handling nodes (such as labels) that determine their accessible name from their children, Gecko currently concatenates the text from all children of the node to form the accessible name of that node. This includes editable children such as text input fields. Unfortunately, in the case of text input fields, the text of the node is the value, which means that the value of the text input field is included in the label. Worse, because unnamed fields such as these receive their accessible name from the label, the name of the text input field also includes the value of the field.

To solve this issue, when gathering text from children to determine the name of the parent (such as a label), text from editable child nodes should not be included.

Reproducible: Always

Steps to Reproduce:
1. Open the attached test case.
2. Move focus to the input text field. Observe that the accessible name of this field and its parent label is "Name:".
3. Type "a". Examine the accessible name of the field and its parent label.
4. Type "b". Examine the accessible name of the field and its parent label.
Actual Results:  
After step 3, the accessible name of both the field and its parent label is "a".
After step 4, the accessible name of both the field and its parent label is "ab".

Expected Results:  
The accessible name of both label and field should remain as "Name:".

Perhaps the read only state could be used to determine whether the text of a given child should be included in the parent's accessible name.

Comment 1

[James Teh [:Jamie]]

Attachment: Test case.

This test case provides an input text field inside a label. Observe that the label's name always includes the value of the input text field, if any.

Comment 2

[Eitan Isaacson [:eeejay]]

I don't know how to change the "assigned" field. Do I need special permissions for that?

Anyway, I would like to take this bug.

Comment 3

[Marco Zehe (:MarcoZ)]

Eitan, create a patch, and when you attach it to the bug, you'll probably see a checkbox "Take this bug". Check it, and you should be done.

Comment 4

[Eitan Isaacson [:eeejay]]

Attachment: Proposed patch

This patch does exactly as Jamie suggests: if the accessible is not "read only", don't include the value in the name. I guess I need to think if this is correct in all cases.

Comment 5

[alexander :surkov (:asurkov)]

(In reply to comment #2)
> I don't know how to change the "assigned" field. Do I need special permissions
> for that?

Yes you need special rights for this, at least if you're not reporter of this bug.

> Anyway, I would like to take this bug.

reassigned

Comment 6

[alexander :surkov (:asurkov)]

Would be nice to get opinion from Aaron and Marco if we really want to do this. I just worry because this code is pretty old iirc.

Comment 7

[alexander :surkov (:asurkov)]

Also we should ensure our mochitests (accessibility/tests/mochitest/test_nsIAccessible_name.html/xul) don't fail and you should extend them to test this bug.

Comment 8

[Aaron Leventhal]

This is actually by design, but perhaps we need to discuss some tweaks to the design.

Here's the case we're trying to solve by doing this. You often have a checkbox or radio button with an embedded control in the label. For example, a checkbox:

[ ] Leave mail on the server for ______ days

The field could be a textfield or combo box, etc. It's a very common pattern both in HTML and UIs.

Comment 9

[Aaron Leventhal]

Comment on attachment 336292 [details] [diff] [review]
Proposed patch

FWIW r- on the code:

PRBool useValue = (state & nsIAccessibleStates::STATE_READONLY);

This is not a legal use of boolean because the logical and will often result in a high value (not 0 or 1).

You need to put != 0 at the end of it.

Comment 10

[alexander :surkov (:asurkov)]

(In reply to comment #8)
> This is actually by design, but perhaps we need to discuss some tweaks to the
> design.
> 
> Here's the case we're trying to solve by doing this. You often have a checkbox
> or radio button with an embedded control in the label. For example, a checkbox:
> 
> [ ] Leave mail on the server for ______ days
> 
> The field could be a textfield or combo box, etc. It's a very common pattern
> both in HTML and UIs.

So we should handle this example and provided example <label>Name: <input/></label> because in this case label and input elements are linked (If I click label then input get a focus). So we should have have special proceedings in the code to handle these cases.

Comment 11

[Aaron Leventhal]

Alex, can you clarify what the rule would be?

Comment 12

[alexander :surkov (:asurkov)]

If we can think of two examples, then I guess we should include value of editable elements depending on role of accessible the name we calculate for. So that if it's label role then we shouldn't include the value. by this way I think we can be sure we don't break anything. Does it sound ok?

Comment 13

[Aaron Leventhal]

I'm still trying to understand what a good rule would be.

Is it: don't use value from controls for name of an ancestor label accessible?

But, if the ancestor is another control, we could use the value?

Comment 14

[alexander :surkov (:asurkov)]

Oops, it seems I missed you example actually. Aaron, could you provide HTML code for you example?

Comment 15

[Marco Zehe (:MarcoZ)]

I believe the example Aaron mentioned is from XUL. In that case, the two controls (the checkbox and the textbox) are siblings. In your example, the label is the textbox's parent.

Comment 16

[alexander :surkov (:asurkov)]

(In reply to comment #15)
> I believe the example Aaron mentioned is from XUL. In that case, the two
> controls (the checkbox and the textbox) are siblings. In your example, the
> label is the textbox's parent.

Ok, Marco, please give me XUL example, preferably without ARIA.

Comment 17

[Aaron Leventhal]

Marco, it happens in HTML and ARIA all the time.

Comment 18

[Michael Curran]

(In reply to comment #11)
> Alex, can you clarify what the rule would be?
I would have thought that the rule would be:
When generating the name of a label, ignore descendants who have a labeledBy or
describedBy relationship pointing to the label in question.
As far as I can see, it makes no sence in any case to create a circuler
relationship via the name generation and accessible relations.

Comment 19

[Michael Curran]

(In reply to comment #18)
> I would have thought that the rule would be:
> When generating the name of a label, ignore descendants who have a labeledBy or
> describedBy relationship pointing to the label in question.
This is assuming that Gecko accessibility does actually force an accessibile relationship between a control and its ancester label. The control's name gets the name of the label, so I would guess that it does.

Comment 20

[Aaron Leventhal]

Here's a XUL example:
http://mxr.mozilla.org/mozilla-central/source/browser/components/preferences/privacy.xul#108

Here's an HTML+ARIA example:
http://www.marcozehe.de/2008/03/23/easy-aria-tip-2-aria-labelledby-and-aria-describedby/

Here's a plain HTML example (scroll down past the ARIA example to the "clever" use of HTML):
http://projectcerbera.com/blog/2008/03/aria-clever-html

In the end the accessible name for the radio or checkbox ends up being the same as the embedded control. This allows the user to hear the full context when either the radio/checkbox or embedded control is focused.

Comment 21

[alexander :surkov (:asurkov)]

Ok, it sounds the example

<label>Shut down computer after <input name="shutdownTime" type="text" value="10"> minutes</label>

makes this bug invalid.

Comment 22

[Aaron Leventhal]

What's the overall problem we're solving? Are we trying to avoid speaking things twice in the virtual buffer?

Comment 23

[Aaron Leventhal]

For this bug now: we could tweak the rule to only include values in accNames for form congtrols (but not on the accName of a <label>'s accessible object).

Would that fix the problem?

Comment 24

[James Teh [:Jamie]]

This actually doesn't relate to the virtual buffer. It relates purely to focus announcement. The problem is not the accName of the label, but rather, the accName of the field.

The use case is something like this:
<label>Comment: <input type="text" name="comment/"></label>
If you have a relatively long comment (e.g. "This is a relatively long comment."), this is extremely annoying, as you hear both the name and the value. In this example:
"Comment: This is a relatively long comment. edit This is a relatively long comment."

A possible solution might be to avoid including form fields in the accName if they are not followed by any further read-only text. For example:
<label>Delete mail after <input type="text" name="deletemailafter" value="5"/> days</label>
would have an accName of "Delete mail after 5 days". However, this:
<label>Username: <input type="text" name="username"/></label>
would only have an accName of "Username:", because the input text field was not followed by any other text and should thus not be included.

Comment 25

[Eitan Isaacson [:eeejay]]

(In reply to comment #24)
> A possible solution might be to avoid including form fields in the accName if
> they are not followed by any further read-only text.

I am thinking this is a dilemma the screen reader should solve. I am not sure I understand why the value needs to be in the name. If a screen reader encounters an editable text field, shouldn't it read the value (accValue or text interfaces)?

It seems to me that by putting the value in the name we are determining policy for the screen readers. For example, a screen reader should choose the level of verbosity, if the value is in the name it is hard to do. Also a screen reader could not easily work around this behavior in per-application scripts.

Also, assistive technologies usually have a good idea of the context of a given accessible, and are better suited at making this kind of choice.

Comment 26

[James Teh [:Jamie]]

(In reply to comment #24)
> A possible solution might be to avoid including form fields in the accName if
> they are not followed by any further read-only text.
This is similar to Aaron's idea in comment 6 of bug 455482, except that Aaron suggests avoiding including the value if it either begins or ends the label instead of just ends. Both is probably better. I can't see why this solution wouldn't work. It still allows the "Delete mail after ___ days" scenario while eliminating pointless duplication of the value at the end.

(In reply to comment #25)
> I am thinking this is a dilemma the screen reader should solve. I am not sure I
> understand why the value needs to be in the name. If a screen reader encounters
> an editable text field, shouldn't it read the value (accValue or text
> interfaces)?
The scenario that this value duplication is supposed to solve is illustrated by the "Delete mail after ___ days" example. Without the value duplication, the screen reader would read the label as "Delete mail after days". So, you might hear:
"Delete mail after days edit 5"
Without analysing the document (which is expensive and error prone), there's no way for an AT to know that the value is required to make the label meaningful.

Dream:
A better solution to all of this is to introduce a new accessibility concept, which I'm going to name postName. postName would be a name which should be presented *after* the value, while name would be presented before the value as it always has been. This way, you could have:
name="Delete mail after"
value="5"
postName=" days"
Unfortunately, this would require a change to all of the accessibility APIs and ATs, so it's probably not going to happen. :)

Comment 27

[Aaron Leventhal]

> Aaron suggests avoiding including the value if it either begins or ends the 
> label instead of just ends.

That's what we've decided to go with. It's described
https://developer.mozilla.org/en/ARIA_User_Agent_Implementors_Guide#Name_Computation under (f), specifically subitem (iii) there.

Comment 28

[alexander :surkov (:asurkov)]

fixed in bug 455886 (section f. of ARIA name computation guide http://www.w3.org/WAI/PF/aria-implementation/).