IAccessible::accNavigate with NAVRELATION_* fails

VERIFIED FIXED in Firefox 22

Status

()

VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: Jamie, Assigned: surkov)

Tracking

({regression})

Trunk
mozilla24
x86_64
Windows 7
regression
Points:
---

Firefox Tracking Flags

(firefox22 verified, firefox23 verified, firefox24 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Str:
1. Open the attached test case.
2. Retrieve the IAccessible for the first table.
3. Call IAccessible::accNavigate(NAVRELATION_LABELLED_BY, CHILDID_SELF).
Expected: The caption accessible should be returned.
Actual: E_FAIL.

This works in Firefox 21 but is broken in Firefox 22 and beyond.
(Reporter)

Comment 1

5 years ago
Created attachment 755759 [details]
Test case.
(Assignee)

Comment 2

5 years ago
How can I trigger it?
(Reporter)

Comment 3

5 years ago
I don't understand. You trigger it as per the str I gave above. I don't know how to trigger it otherwise.
(Assignee)

Comment 4

5 years ago
(In reply to James Teh [:Jamie] from comment #3)
> I don't understand. You trigger it as per the str I gave above. I don't know
> how to trigger it otherwise.

I hoped there's a tool or perhaps I could use NVDA for that. I'm pretty sure the bug is on MSAA layer because core logic is covered by automated testing.
(Reporter)

Comment 5

5 years ago
I haven't actually implemented any functionality which uses this yet. However, here's how you can trigger it using the NVDA Python console.

You're using a Windows vm on a Mac, so you can't use the NVDA key, which is a bit painful, so you first need to temporarily bind a key you can use to contextually activate the NVDA Python console:
1. Right click the NVDA icon on the system tray, then Tools -> Python console.
2. Type these lines (you'll need to paste each line separately):
import globalCommands
globalCommands.commands.bindGesture("kb:control+alt+z", "activatePythonConsole")

Now the actual steps to trigger:
1. Open the test case.
2. Make sure focus is on the document.
3. Press control+alt+z.
4. Type:
focus.getChild(2).IAccessibleObject.accNavigate(0x1003, 0)
Expected: Output should say something like: <comtypes.client.lazybind.Dispatch object at 0x05C29290>
Actual: Output says something like:
Traceback (most recent call last):
  File "<console>", line 1, in <module>
COMError: (-2147467259, 'Unspecified error', (None, None, None, 0, None))
(Assignee)

Comment 6

5 years ago
Created attachment 756425 [details] [diff] [review]
patch
Assignee: nobody → surkov.alexander
Attachment #756425 - Flags: review?(trev.saunders)
Comment on attachment 756425 [details] [diff] [review]
patch

>-  if (xpRelation) {
>+  if (xpRelation != -1) {

why not < 0?

I'd prefer we checked it was less than the max relation, but we don't have a constant for that anymore :(
Attachment #756425 - Flags: review?(trev.saunders) → review+
(Assignee)

Comment 8

5 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #7)
> Comment on attachment 756425 [details] [diff] [review]
> patch
> 
> >-  if (xpRelation) {
> >+  if (xpRelation != -1) {
> 
> why not < 0?

it's the same, why does it matter?

> I'd prefer we checked it was less than the max relation, but we don't have a
> constant for that anymore :(

we live under assumption constants are not negative, so it should be ok
(In reply to alexander :surkov from comment #8)
> (In reply to Trevor Saunders (:tbsaunde) from comment #7)
> > Comment on attachment 756425 [details] [diff] [review]
> > patch
> > 
> > >-  if (xpRelation) {
> > >+  if (xpRelation != -1) {
> > 
> > why not < 0?
> 
> it's the same, why does it matter?

just treating -1 special looks funny

> > I'd prefer we checked it was less than the max relation, but we don't have a
> > constant for that anymore :(
> 
> we live under assumption constants are not negative, so it should be ok

sure but what's the point in calling RelationByType() if you know  the relation type you want doesn't actually exist?
(Assignee)

Comment 10

5 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #9)
> (In reply to alexander :surkov from comment #8)
> > (In reply to Trevor Saunders (:tbsaunde) from comment #7)
> > > Comment on attachment 756425 [details] [diff] [review]
> > > patch
> > > 
> > > >-  if (xpRelation) {
> > > >+  if (xpRelation != -1) {
> > > 
> > > why not < 0?
> > 
> > it's the same, why does it matter?
> 
> just treating -1 special looks funny

so you prefer to have

if (xpRelation >= 0) {
  Relation rel = RelationByType();
}

> > > I'd prefer we checked it was less than the max relation, but we don't have a
> > > constant for that anymore :(
> > 
> > we live under assumption constants are not negative, so it should be ok
> 
> sure but what's the point in calling RelationByType() if you know  the
> relation type you want doesn't actually exist?

no point but we don't do that since we return early
https://hg.mozilla.org/mozilla-central/rev/fbccd02d5cff
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
(Reporter)

Comment 13

5 years ago
Any chance of getting this backported? Otherwise, proper support for table captions in NVDA will work for Firefox 21 but not for 22 or 23.
Alex, what bug is this a regression from? The keyword indicates that it is, but I don't see any dependency. It would help the beta and aurora approval cases if we knew where this came from.
(Assignee)

Comment 15

5 years ago
Comment on attachment 756425 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 873453
User impact if declined: NVDA and some AT are partially broken
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): no
String or IDL/UUID changes made by this patch: no
Attachment #756425 - Flags: approval-mozilla-aurora?
(Reporter)

Comment 16

5 years ago
Verified fixed in Firefox 24.0a1 (2013-06-07).
Status: RESOLVED → VERIFIED
status-firefox24: --- → fixed
Comment on attachment 756425 [details] [diff] [review]
patch

The bug that the regression is from landed only on mozilla 24, so this shouldn't need to be uplifted to 23.
Attachment #756425 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
(Assignee)

Comment 18

5 years ago
(In reply to lsblakk@mozilla.com [:lsblakk] from comment #17)
> Comment on attachment 756425 [details] [diff] [review]
> patch
> 
> The bug that the regression is from landed only on mozilla 24, so this
> shouldn't need to be uplifted to 23.

sorry a wrong bug number was specified, it was supposed to be bug 527461.
(Assignee)

Comment 19

5 years ago
Comment on attachment 756425 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #):  bug 527461
User impact if declined: NVDA and some AT are partially broken
Testing completed (on m-c, etc.): no
Risk to taking this patch (and alternatives if risky): no
String or IDL/UUID changes made by this patch:no
Attachment #756425 - Flags: approval-mozilla-beta?
Attachment #756425 - Flags: approval-mozilla-aurora?
Attachment #756425 - Flags: approval-mozilla-aurora-
Comment on attachment 756425 [details] [diff] [review]
patch

Approving to fix comment 13, given low risk and high reward for a11y users.
Attachment #756425 - Flags: approval-mozilla-beta?
Attachment #756425 - Flags: approval-mozilla-beta+
Attachment #756425 - Flags: approval-mozilla-aurora?
Attachment #756425 - Flags: approval-mozilla-aurora+

Comment 22

5 years ago
Verified as fixed with Firefox 22.0 (RC - 20130618035212) on a Windows 7 x64 VM running on Mac OSX 10.7.5. I used the steps in comment 5 and got the expected output (with a different address than the example, but that should be expected).

Updated

5 years ago
status-firefox22: fixed → verified
Keywords: verifyme

Comment 23

5 years ago
Verified as fixed under the conditions in comment 22 on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0 (20130725195523)
status-firefox23: fixed → verified

Comment 24

5 years ago
Verified as fixed on Firefox 24 beta 1:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Firefox/24.0
status-firefox24: fixed → verified
Keywords: verifyme
QA Contact: ioana.budnar
You need to log in before you can comment on or make changes to this bug.