561924 - Remove group info hack from accDescription

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

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

Attachment: WIP

Fix inconsistency left from bug 539252. See excellent description at bug 539252 comment 12.

Comment 1

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

This will obviously need vendor outreach before we fix it.

Comment 2

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

Comment on attachment 441667 [details] [diff] [review]
WIP

>diff --git a/accessible/src/msaa/nsAccessibleWrap.cpp b/accessible/src/msaa/nsAccessibleWrap.cpp
>--- a/accessible/src/msaa/nsAccessibleWrap.cpp
>+++ b/accessible/src/msaa/nsAccessibleWrap.cpp
>@@ -369,76 +369,17 @@ __try {
>     return E_FAIL;
> 
>   // For items that are a choice in a list of choices, use MSAA description
>   // field to shoehorn positional info, it's becoming a defacto standard use for
>   // the field.

Will need to delete this comment as well. Of course this comment is making me worried about users with older versions of screen readers.

Comment 3

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

Apparently the plan is to sniff the AT version.

Comment 4

[Marco Zehe (:MarcoZ)]

Correct, in nsAccessNodeWrap, we already have sniffing for JAWS 7.10 and don't initialize IAccessible2 at all in that case. We should outreach to both FS and WGW to find out which versions of Firefox-supporting screen readers still need the hack, and implement it conditionally.

Comment 5

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

(In reply to comment #4)
> Correct, in nsAccessNodeWrap, we already have sniffing for JAWS 7.10 and don't
> initialize IAccessible2 at all in that case.

How does this work for the multi-AT case?

Comment 6

[Marco Zehe (:MarcoZ)]

We do check for several known modules (one being jhook for JAWS, for example), to determine whether we should allow the new Ctrl+Tab-Panels or disable them up-front. We can ask for the modules, and we can ask for the version numbers, as shown in the JAWS 7.10-no-IA2-case. We just need to expand upon that I think. It's all in accessible/src/msaa/nsAccessNodeWrap.cpp.

Comment 7

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

I guess nobody runs Jaws (jhook) and NVDA concurrently right?

Comment 8

[James Teh [:Jamie]]

It's generally not recommended to run two screen readers simultaneously, although it is done sometimes. Among other things, this breaks the system screen reader flag; Mozilla doesn't use this, but other apps do (broken behaviour imo). However, I suspect this code will break if a user runs JAWS, exits JAWS and then starts NVDA without first restarting Firefox. There's really not much that can be done to fix this. It's the price Mozilla pays for supporting legacy ATs. :) It should probably be documented in some sort of accessibility FAQ or something.

Comment 9

[Marco Zehe (:MarcoZ)]

I agree with jamie, this *is* a possibility. But hey, nobody would want to go back to an old version of JAWS after using NVDA with Firefox anyway. ;)

Seriously, I believe these would be edge cases that we can deal with in a section of the screen readers FAQ if need be.

Comment 10

[James Teh [:Jamie]]

See bug 539252 comment #15. Whatever we do, we're going to have breakage for NVDA users unless we literally release a stable version of NVDA ande a stable version of Firefox 4 at the same time. Anyway, something really needs to be done about this, as descriptions are completely broken for Firefox 4pre and NVDA until this is sorted.

Comment 11

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

You mean you don't bundle FF with NVDA?! (wink)

Comment 12

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

Anyways, I think we will go ahead and fix this without sniffing. I just pushed an updated patch to try server.

Comment 13

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

Try build: http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-dff8de8cfb4d/

Comment 14

[James Teh [:Jamie]]

Works as expected.

I still have no idea how we're going to support this change. New builds of NVDA will handle descriptions properly with either Firefox 4 or Firefox 3.6. We cannot support both; there's no way for us to reliably determine which way to interpret description. If we change NVDA to behave with the description hack removed (as per this patch), users will start hearing, for example, "L1, 1 of 2" on tree view items in Firefox <= 3.6 and Thunderbird <= 3.1.

Comment 15

[alexander :surkov (:asurkov)]

We want to have it fixed on 2.0. We've introduced some changes how we expose accessible description for gecko 2.0 already which makes screen readers broken in some way. Accessible description is important information exposed to screen readers and finally we should fix that and do it right. Iirc David has a almost-ready patch for this so it shouldn't take a long time.

Comment 16

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

Attachment: patch

Comment 17

[Marco Zehe (:MarcoZ)]

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

r=me.

Comment 18

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

Landed on central: http://hg.mozilla.org/mozilla-central/rev/2f2dd2813d17

Comment 19

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

Note to approvers: low risk -- removes broken/obsolete functionality. Improves accessibility performance (Windows only).

Comment 20

[James Teh [:Jamie]]

I still disagree with this and bug 539252 being low risk. As I noted, this breaks all current versions of NVDA. In addition, any new versions of NVDA will work only with Firefox before or after the change, not both. In other words, unless we time a release of NVDA with a release of Firefox, users will have to deal with broken functionality for a while. There is no work around.

Comment 21

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

Jamie this change will appear in ff4. Can you time a release of nvda to mitigate user pain? Let's figure this out over irc/email?

Comment 22

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

(In reply to comment #20)
> I still disagree with this and bug 539252 being low risk.

Note the statement to approvers about 'risk' here was about risk of gecko internal instability.

I'm still hopeful we can make this less painful and i think ff4 is a good target for changes like these.

Comment 23

[James Teh [:Jamie]]

For reference, I've finally thought of a solution which will allow ATs that need to support both versions of Gecko without breaking one or the other. The idea is to use IAccessibleApplication::toolkitVersion() to detect Gecko 2 and act accordingly. For out-of-process ATs, doing this for every accessible is expensive, so it's probably best to cache the result for each process.

Comment 24

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

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

We are intended this change for FF 4.0 so that ATVs can coordinate releases and user expectation. It is code removal.

Comment 25

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

I butchered what I was trying to say there.

We are targeting FF 4.0 for this significant change in Windows accessibility API expectation along with other similar changes. We are hoping ATVs can make changes on their end and co-release as/if necessary alongside FF 4.0.

Comment 26

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

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

This already landed on central (back approving patch)

Comment 27

[alexander :surkov (:asurkov)]

(In reply to comment #25)
> I butchered what I was trying to say there.
> 
> We are targeting FF 4.0 for this significant change in Windows accessibility
> API expectation along with other similar changes. We are hoping ATVs can make
> changes on their end and co-release as/if necessary alongside FF 4.0.

It breaks users of existing commercial AT. I think that's no right way to go while we have alternatives and iirc we were agree at this point. Why did you change your mind?