539252 - remove "Description: " prefix from MSAA description

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

That's was requested by AT vendor. I'm going to put the patch and try server build to ensure we won't break anybody.

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

We shouldn't break anybody. I'll put a link on try server build soon.

Comment 2

[alexander :surkov (:asurkov)]

try server build  - https://build.mozilla.org/tryserver-builds/[e-mail]-try-d4dcf9d635e4/

Comment 3

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

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

The pre-existing comment worries me. Are there any vendors left to ping? Maybe we should ping Aaron on this.

Comment 4

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

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

>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
>@@ -434,24 +434,16 @@ __try {
> 

You could do
if (description.IsEmpty()) {
  xpAccessible->GetDescription(description);
}

>   if (!description.IsEmpty()) {
>     *pszDescription = ::SysAllocStringLen(description.get(),
>                                           description.Length());
>     return *pszDescription ? S_OK : E_OUTOFMEMORY;
>   }
> 
>   xpAccessible->GetDescription(description);

and remove this line ^^^

>-  if (!description.IsEmpty()) {

Comment 5

[alexander :surkov (:asurkov)]

Yeah, that's nicer. Thanks.

Comment 6

[Marco Zehe (:MarcoZ)]

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

functinality-wise this doesn't break JAWS, WE or HAL. r=me.

Comment 7

[alexander :surkov (:asurkov)]

Attachment: patch2

Comment 8

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

Comment on attachment 421626 [details] [diff] [review]
patch2

r=me thanks

Comment 9

[alexander :surkov (:asurkov)]

landed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/f7b7c302fc45

Comment 10

[alexander :surkov (:asurkov)]

Comment on attachment 421626 [details] [diff] [review]
patch2

it's trivial, safe and tested, helpful for screen readers.

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 421626 [details] [diff] [review]
patch2

We'll try again on the 1.9.2.3 cycle; I do like the idea of removing code!

Comment 12

[James Teh [:Jamie]]

We missed this. :) Unfortunately, it breaks NVDA.

The reason this "Description: " prefix was added in the first place is that Mozilla hack accDescription to contain group position information, which can't be exposed using MSAA alone. (It may also be used to expose other info, but group position info is the only case I know about.) For example, list items expose something like "5 of 7" as the description and tree items expose something like "L1, 59 of 59". Without the "Description: " prefix, it's impossible to determine with absolute certainty whether this is data or the description the widget intended to expose. ATs can now get group position info using IAccessible2, so NVDA just ignores the description completely if it does not contain the "Description: " prefix. It's unlikely that a widget would expose "5 of 7" or "L1, 59 of 59" as its description, so we could guess based on pattern matching, but technically speaking, this is not 100% safe.

I agree that the "Description: " prefix is ugly, but the entire group position info in description hack is ugly. Unfortunately, it has been kept for backwards compatibility.

I'd be happy to remove NVDA's special description processing, but if we're going to do this, I really think the group position info hack should be removed. Alternatively, we can pattern match as mentioned above, but as noted, this is not 100% safe.

Comment 13

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

Filed bug 561924 to address inconsistency.

Comment 14

[alexander :surkov (:asurkov)]

Comment on attachment 421626 [details] [diff] [review]
patch2

cancelling approval request since it regress nvda (see bug 561924)

Comment 15

[James Teh [:Jamie]]

Note that even once bug 561924 is closed, we will remove the Mozilla specific description logic from NVDA. Unfortunately, this means that newer versions of NVDA will report "description:" before all descriptions in Firefox <= 3.6 and, worse still, it will report, for example, "l1, 1 of 3" for tree view items. This won't be an issue once 3.7 is released, since we can expect users to upgrade, but we can't expect users to run nightlies. In other words, whatever we do, something will break.

Related NVDA ticket: http://www.nvda-project.org/ticket/630