104477 - If screen reader is present, actively append word "checked" or "unchecked" to checkboxes in components list

Status: VERIFIED FIXED

(SeaMonkey :: Installer, defect)

Description

[Aaron Leventhal]

We use a listbox with checkboxes in them.
The checkboxes are drawn on the screen, and for some reason the text isn't
available to the screen readers.

We can find out if a screen reader is present by calling
SystemParametersInfo() with the SPI_GETSCREENREADER option.

* If a screen reader is present , we can append a i18n string "- checked" or "-
unchecked" to the text for each component name. That solves the problem of the
screen reader not knowing that it's on a checkbox.

* We need to figure out what's wrong with the way we're displaying the text on
screen - why can't the screen reader see it?

Comment 1

[Aaron Leventhal]

Don't worry about the second bullet point above, I found a screen reader that
reads the text fine.

Changing the bug summary to accurately reflect the task in the first bullet point.

If SystemParametersInfo() with the SPI_GETSCREENREADER option tells us we're
running with a screen reader, use text instead of just the image to indicate
whether the checkbox is checked or unchecked.

Syd, can we prioritize this one? It will make installer accessible, which is
crucial.

Comment 2

[Sean Su]

reassigning to Curt.

Comment 3

[Curt Patrick (gone)]

Aaron, I'm not familiar with screen readers and how they work.  Think you could
demo this to me so I can see exactly what the problem is?  I'll be around
tomorrow (Tuesday) after 3:00 and all day Wednesday if you can find a little
time for me.

Comment 4

[Syd Logan]

plussing

Comment 5

[Henrik Gemal]

is this really nsbeta1+ ?

Comment 6

[Aaron Leventhal]

This is really nsbeta1+, because otherwise screen reader users can't use the
installer. They won't get very far if they can't install, will they?

We could also use regular checkboxes, instead of checkboxes drawn on the screen
with graphics, if we wanted to solve this a different way. 

Comment 7

[Daniel Veditz [:dveditz]]

Henrik: nsbeta1+ means important to Netscape, which sometimes (often?) has
different priorities than the Mozilla community. In the case of "sec508" bugs
those are things that AOL feels it needs to do to comply with the "Americans
with Disabilities Act".

Comment 8

[Curt Patrick (gone)]

Aaron, I'm working on this bug now, and have a couple questions for you:

- Does the "- checked" need to actually be visible?  I'm thinking about forcing
it offscreen by putting a bunch of spaces between it and the visible text.  Then
I don't have to do any special check for the text reader.  Will that work?

- Does this have any international implications?  Do all readers look for the
same text regardless of the language the software is in.  I.e., will these two
strings need translated?

Comment 9

[Aaron Leventhal]

Yes, it does need to be internatitionalized :(

Also, you might as well keep it onscreen, since I don't know the answer, and
this will only take affect in the rare case that a screen reader is present.

Comment 10

[Curt Patrick (gone)]

Attachment: Patch 1

I haven't figure out exactly how to use SystemParametersInfo, but all the rest
of the functionality is in place and, since I'm going to be away for a week, I
want to get that reviewed.  I plugged in a variable called bTestParameter which
fakes the reader, but at compile time, for testing.

Comment 11

[dprice (gone)]

Attachment: change to install.it

looks like curt missed this little change in install.it  We need to get this
part landed today for the string freeze.

Comment 12

[dprice (gone)]

Attachment: patch for mozilla side

Comment 13

[scottputterman]

Please check in the strings for this patch today if possible.  We'll adt1.0.1+
the bug when the rest of the bug is finished.

Comment 14

[Syd Logan]

what is the difference between = and =- ?

Comment 15

[Daniel Veditz [:dveditz]]

The hyphen is part of the text of the string. It probably should be "= - ", but
there's a chance the code (which I haven't looked at) is putting the extra space
after the existing text. I'd have preferred something like [Checked] which would
be less likely to look like part of the description but I guess it doesn't
really matter since most folks won't see it.

Comment 16

[dprice (gone)]

I checked the code, it is adding the hyphen and the space.  So the correct
strings don't contain the hyphen.

Comment 17

[dprice (gone)]

Attachment: correct strings

the netscape patch is identical

Comment 18

[dprice (gone)]

Attachment: netscape version

Comment 19

[Daniel Veditz [:dveditz]]

Comment on attachment 86167 [details] [diff] [review]
correct strings

sr=dveditz

Comment 20

[Daniel Veditz [:dveditz]]

Comment on attachment 86180 [details] [diff] [review]
netscape version

sr=dveditz

Comment 21

[Aaron Leventhal]

Still need this part:

We can find out if a screen reader is present by calling the win32 function
SystemParametersInfo() with the SPI_GETSCREENREADER option.

See
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/wcesdkr/htm/_wcesdk_win32_systemparametersinfo.asp

Comment 22

[dprice (gone)]

right now we're just trying to land the strings that need to be localized.  The
bug is still a ways from being fixed.

Comment 23

[Michele Carlson]

Thanks for landing the strings now.

Comment 24

[Curt Patrick (gone)]

Attachment: Patch 2

This should be complete.  I don't have a screen reader to test with, but I
faked on a couple different ways and am convinced this is the real deal.

Comment 25

[scottputterman]

Please get the r=/sr=.  Aaron, do you have the ability to try the patch out?

Comment 26

[Aaron Leventhal]

I'll try this out today or tomorrow.

Comment 27

[dprice (gone)]

Comment on attachment 87319 [details] [diff] [review]
Patch 2

r=dprice

Comment 28

[Daniel Veditz [:dveditz]]

Comment on attachment 87319 [details] [diff] [review]
Patch 2

>+    SendMessage(lpdis->hwndItem, LB_GETTEXT, lpdis->itemID, (LPARAM)tchBuffer);
>+
>+	if(gSystemInfo.bScreenReader)
>+	{
>+      lstrcat(tchBuffer, " - ");

Busted by the tab police -- please clean these up here and
later on in the file.

sr=dveditz if aaronl says it works for him.

Comment 29

[Aaron Leventhal]

I have tried out the patch.

It turns out that the Windows 2000 utility called "Narrator" under Start -
Accessories - Accessibility, is good enough to test with.

We must be exposing these strings in some additional way, other than just
drawing them on the screen. I say this because, although "checked" and
"unchecked" appear on the screen - Narrator is reading the list items without
that part. Give it a try and you'll see what I mean.

Comment 30

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 86167 [details] [diff] [review]
correct strings

(In case you didn't land this already and you still can, although it looks like
chofmann added a + to the bug for this patch.)

Please land this on the 1.0.1 branch.  Once there, remove the
"mozilla1.0.1+" keyword, and add the "fixed1.0.1"

Comment 31

[Aaron Leventhal]

Wait, please try my Microsoft Narrator test first described above before
checking in. There seems to be some work left to be done.

Comment 32

[Curt Patrick (gone)]

I tried out Narrator (which would have been a lot easier if I had even one
machine with speakers!) and it appears that I'm going to have to back way up on
this bug.  The text cannot be changed just when the window is being redrawn, it
must actually be change in the listbox object for the screen reader to pick it
up.  That is significantly more work than I thought I was going to get away
with.  But with Narrator, I do have a real test environment now.

Comment 33

[Curt Patrick (gone)]

Attachment: Patch 3

The most significant differences between this and Patch 3 are in
SiCNodeSetAttributes() and, to a lesser degree, in lbAddItem.

Comment 34

[Sean Su]

Comment on attachment 88968 [details] [diff] [review]
Patch 3

looks great Curt.  Good job!

r=ssu

Comment 35

[Daniel Veditz [:dveditz]]

Comment on attachment 88968 [details] [diff] [review]
Patch 3

sr=dveditz

Comment 36

[Jaime Rodriguez, Jr.]

adt1.0.1+ (on ADT's behalf) approval for checking into the 1.0 branch. pls check
this in to the trunk and branch asap, the replace mozilla1.0.1 with fixed1.0.1
keyword. thanks!

Comment 37

[Curt Patrick (gone)]

Checked in on trunk and branch.

Comment 38

[Curt Patrick (gone)]

I hesitate to mention this but, in talking with Grace about testing this bug it
suddenly occurred to me that Checked and Unchecked are not the only states the
listbox items can be in.  There is also Enabled and Disabled.  A person who can
now hear whether an item is checked still has no way to know whether it is
enabled.  Is that acceptable?  (It is hard to quit thinking like a sighted
person when that is what one is!)

Comment 39

[Aaron Leventhal]

The screen reader can notice the "grayed out font", which is typically used to
indicated the disabled state. The problem with the checkbox was that it was made
with a graphic.

I think we're okay, but I'll double check with the screen reader vendors.

Comment 40

[Curt Patrick (gone)]

I like that answer.  (Picture me wiping sweat from brow.)  I hope your vendors
agree!

Comment 41

[timeless]

Ok, I just tried using microsoft narrator, it won't read disabled, basically 
disabled appears to be tantamount to invisible. Which might be reasonable. What 
does a user gain by being told about disabled options? If you can't trigger the 
item.

The most enjoyable example of this is print settings
file>print>properties>advanced, you get this nice tree, at each level narrator 
tells you what level you're on and how to collapse or expand.

I wasn't able to get narrator to tell me about any of the disabled options in 
the print settings dialog boxes.

Note however that nc4 is really broken, when i select Leave as, it says:
Radio button, checked, to select a different item in the cluster, use the arrow 
keys.
However, the arrow keys don't do anything :-)

It should have said: Leave As, Radio Button, checked, ... and of course the 
arrow keys should have worked :-).

Comment 42

[Aaron Leventhal]

Timeless, Narrator isn't a true screen reader. Blind users don't use it unless
absolutely forced too (even then probably won't). WindowEyes, JAWS, Hal, etc.
are what's actually used in the real world. Narrator was useful to test in this
situation.

Comment 43

[Grace Bush]

verified on branch 2002070308 and trunk 2002070308

Comment 44

[timeless]

Bug 172583 has been added to the database

Comment 45

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1527381