431802 - node_child_of accessible relation returns wrong object on autocomplete list items

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Michael Curran]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050105 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050105 Minefield/3.0pre

When asking for the node_child_of accessible relation on list items of particular autocomplete widgets (such as the google search field on the Firefox toolbar, any edit field on a webpage in Firefox, or email fields in Thunderbird), the first list item is returned, rather than either the list itself, or nothing at all. This bug can cause ATs that use node_child_of to go in to an endless loop, or have other unexpected results due to it returning completely the wrong thing.

Reproducible: Always

Comment 1

[Marco Zehe (:MarcoZ)]

STR:
1. Focus the Google Search textbox.
2. Start typing in something you know you have typed in before.
3. Press DownArrow to go into the list of suggestions.

The selected list item has the wrong node_child_of relation.

Comment 2

[alexander :surkov (:asurkov)]

Marco, how do you get that relation?

Comment 3

[alexander :surkov (:asurkov)]

Sure, we shouldn't show different result when you get node_child_of relation. But why you can't use GetParent() instead?

Btw, I tried to reproduce this on html:select elements - their relation is always null. Could you figure out when you get null and when the first item?

Comment 4

[Marco Zehe (:MarcoZ)]

The issue is with XUL textboxes that have a dropdown, like the Search box at the top if you start typing in something you have typed in before, then press DownArrow to get the first suggestion.

Comment 5

[Michael Curran]

(In reply to comment #3)
> But why you can't use GetParent() instead?
As there is no particular rule as to when to use node_child_of verses accParent, NVDA uses node_child_of if it exists, else it uses accParent.
Note that if it returns NULL, then of course we do then try accParent. We also now in NVDA check to see if it returns the same object we just asked, then we also class that as being invalid and use accParent. But, I still think this should be classed as a bug in Gecko as if node_child_of relation exists, then ATs should be able to trust it.

> Btw, I tried to reproduce this on html:select elements - their relation is
> always null. Could you figure out when you get null and when the first item?
The list of a html select element who's size is 1 (a combobox) has a node_child_of pointing to the actual combo box. I don't think html select elements with a size of 2 or higher need it or have it, as the list is actually hyararchically at the right position in the accessibility representation -- combo box lists are a separate popup window and therefore need the relation.

Note that the only problem I find with node_child_of relation is on the list items in particular auto complete widgets.

Comment 6

[alexander :surkov (:asurkov)]

Attachment: mochitest based test

you can put it into mochitest shell, I type into textbox 'h' it shows one result (outlineitem which is node child of itself), btw, nextSibling method on latest outlineitem fails (it shoudn't)

Comment 7

[alexander :surkov (:asurkov)]

in this case (mochitest) the problem is inside TreeView->GetParentIndex which returns 0 for the 0th item. It sounds the problem is not on our side.

Comment 8

[alexander :surkov (:asurkov)]

that's cool - they just doesn't implement it (http://lxr.mozilla.org/mozilla/source/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp#820)

Comment 9

[alexander :surkov (:asurkov)]

Attachment: patch

autocomplete results aren't hierarchical, so we should return -1

Comment 10

[[e-mail]]

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

Looks like browser's copy of pageInfo.js has the same bug (the suite port was most likely fixed because it had a better review!)

Comment 11

[alexander :surkov (:asurkov)]

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

low risk, benefits - allows AT to work with autocomplete correctly.

Comment 12

[alexander :surkov (:asurkov)]

(In reply to comment #10)
> (From update of attachment 319370 [details] [diff] [review])
> Looks like browser's copy of pageInfo.js has the same bug (the suite port was
> most likely fixed because it had a better review!)
> 

Neil, should I file the bug for this?

Comment 13

[[e-mail]]

(In reply to comment #12)
> Neil, should I file the bug for this?
I only noticed by code inspection. I'm just assuming it might be a bug :-)

Comment 14

[Mike Schroepfer]

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

a+ schrep

Comment 15

[Jason Orendorff [:jorendorff]]

(1.9 triage) Just hasn't landed yet.

Comment 16

[Marco Zehe (:MarcoZ)]

I'm back from an accessibility trade show now and will check this in Saturday morning (German time) if tree permits.

Comment 17

[Mike Schroepfer]

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

Removing approval since this patch was not landed before we closed for 1.9.

Comment 18

[Marco Zehe (:MarcoZ)]

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

Renominating for inclusion. This is a major show stopper for the NVDA screen reader.

Comment 19

[Marco Zehe (:MarcoZ)]

Also see Neil's explanation in comment #10.

Comment 20

[Mike Schroepfer]

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

a+ given Gavin's explination on dev.planning.  Looks to affect only accessibility and the impact there is huge.

Comment 21

[:Gavin Sharp [email: [e-mail]]]

Landed:
mozilla/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp 	1.82 


Surkov, can you ensure the test gets landed?

Comment 22

[alexander :surkov (:asurkov)]

(In reply to comment #21)
> 
> Surkov, can you ensure the test gets landed?
> 

sure, we track in-testsuite? flags in a11y.