494815 - Accessible name subtree computation should ignore whitespace when determining whether to strip user-entered value

Status: VERIFIED 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.2a1pre) Gecko/20090524 Minefield/3.6a1pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; 

Bug 452767 changed accessible name subtree computation to exclude a user-entered value if it is at the start or end of the subtree; e.g. a label containing the label text and an input tag. However, if there is whitespace between the input field and the start/end, this breaks, as the user-entered value is not technically at the end of the subtree anymore. Unfortunately, there are many cases where this occurs, especially since a lot of code is automatically generated. Therefore, I think that this computation needs to ignore whitespace when determining whether to strip user-entered values.

Reproducible: Always

Steps to Reproduce:
1. Open the attached test case.
2. Move focus to the first editable text field.
3. Type some text.
4. Observe that the text typed is not included in the accessible name of the field.
5. Move focus to the second editable text field.
6. Type some text.
7. Check the accessible name of the field.
Actual Results:  
The text typed in step 6 was included in the accessible name of the field (as checked in step 7).

Expected Results:  
The text typed in step 6 should not be included in the accessible name of the field. That is, the result for step 7 should be the same as for step 4.

Comment 1

[James Teh [:Jamie]]

Attachment: Test case.

This is the test case from bug 452767 with an additional test to reproduce this bug.

Comment 2

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 3

[alexander :surkov (:asurkov)]

Comment on attachment 390689 [details] [diff] [review]
patch

changing request review to Ginn since Marco is on vacation.

Comment 4

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

Comment on attachment 390689 [details] [diff] [review]
patch

r=me, with request for comments and a nit:

>@@ -331,31 +331,41 @@ nsTextEquivUtils::AppendFromValue(nsIAcc

>+  for (PRInt32 idx = indexOf - 1; idx >= 0; idx--) {

    // check for preceding text...

>+    if (!parent->GetChildAt(idx)->TextIsOnlyWhitespace()) {
>+      PRUint32 childCount = parent->GetChildCount();

      // .. and subsequent text

>+      for (PRUint32 idx = indexOf + 1; idx < childCount; idx++) {

I would prefer the outer and inner for loops have different names for 'idx' (perhaps i and j) to make this code easier to read -- but that is a personal preference

Comment 5

[alexander :surkov (:asurkov)]

Comment on attachment 390689 [details] [diff] [review]
patch

Cancelling review from Ginn since I david granted it and to save Ginn's time for tree reorg.

Comment 6

[alexander :surkov (:asurkov)]

landed on 1.9.2 http://hg.mozilla.org/mozilla-central/rev/51ea902fddc0

Comment 7

[Marco Zehe (:MarcoZ)]

Verified fixed in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090804 Minefield/3.6a1pre (.NET CLR 3.5.30729)