Closed Bug 431802 Opened 12 years ago Closed 12 years ago

node_child_of accessible relation returns wrong object on autocomplete list items

Categories

(Core :: Disability Access APIs, defect, major)

x86
Windows Vista
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla1.9

People

(Reporter: mick, Assigned: surkov)

Details

(Keywords: access)

Attachments

(2 files)

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
Keywords: access
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.
Version: unspecified → Trunk
Marco, how do you get that relation?
Status: UNCONFIRMED → NEW
Ever confirmed: true
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?
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.
(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.


Attached file 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)
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.
Attached patch patchSplinter Review
autocomplete results aren't hierarchical, so we should return -1
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #319370 - Flags: superreview?(neil)
Attachment #319370 - Flags: review?(neil)
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!)
Attachment #319370 - Flags: superreview?(neil)
Attachment #319370 - Flags: superreview+
Attachment #319370 - Flags: review?(neil)
Attachment #319370 - Flags: review+
Comment on attachment 319370 [details] [diff] [review]
patch

low risk, benefits - allows AT to work with autocomplete correctly.
Attachment #319370 - Flags: approval1.9?
(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?
(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 on attachment 319370 [details] [diff] [review]
patch

a+ schrep
Attachment #319370 - Flags: approval1.9? → approval1.9+
(1.9 triage) Just hasn't landed yet.
Whiteboard: [missed 1.9 checkin]
I'm back from an accessibility trade show now and will check this in Saturday morning (German time) if tree permits.
Comment on attachment 319370 [details] [diff] [review]
patch

Removing approval since this patch was not landed before we closed for 1.9.
Attachment #319370 - Flags: approval1.9+
Comment on attachment 319370 [details] [diff] [review]
patch

Renominating for inclusion. This is a major show stopper for the NVDA screen reader.
Attachment #319370 - Flags: approval1.9?
Also see Neil's explanation in comment #10.
Flags: blocking1.9?
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.
Attachment #319370 - Flags: approval1.9? → approval1.9+
Flags: blocking1.9? → blocking1.9+
Landed:
mozilla/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp 	1.82 


Surkov, can you ensure the test gets landed?
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [missed 1.9 checkin]
Target Milestone: --- → mozilla1.9
(In reply to comment #21)
> 
> Surkov, can you ensure the test gets landed?
> 

sure, we track in-testsuite? flags in a11y.
You need to log in before you can comment on or make changes to this bug.