629986 - unlabeled edit fields in the crash reporter

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Trevor Saunders (:tbsaunde)]

User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0b11pre) Gecko/20110129 Firefox/4.0b11pre
Build Identifier: 

The crash reporter has two text entry fields, one for your email and one for what was happening at the time of the crash.  Niether of these fields appear to expose there label to a screen reader.

Reproducible: Always

Comment 1

[(not currently active) Ted Mielczarek]

I would swear I ran the UI by our a11y folks when I first created it, but maybe we missed something. We're just creating it using standard GTK APIs, so if there's something else that we're supposed to be doing it's probably not a hard fix:
http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/client/crashreporter_linux.cpp#409

Comment 2

[timeless]

gtk_label_set_mnemonic_widget   would be what one word normally use, but it only works if you have a label.

we could create a label containing the default text *behind* the textarea and then set mnemonic.

I claim this bug should be wontfix. The default values for the textareas tell the user what to do.

reporter: are you familiar w/ ATK? is there a specific thing you want us to do? is there a precise reason why the default values for the text fields is insufficient?

Comment 3

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

Trevor is a member of our Mozilla accessibility community and knows ATK. I'd assume this bug is valid but can't test ATM.

Comment 4

[alexander :surkov (:asurkov)]

(In reply to comment #2)

> I claim this bug should be wontfix. The default values for the textareas tell
> the user what to do.

I think he expect to hear an name of controls (which is exposed by associated label in general) while the value is different thing.

Though that's ok to expose default text as a name (it's similar to HTML 5 placeholder). But if HTML5 placeholder isn't used here then perhaps it's hard to expose default text and easiest way would be put aria-label attribute with this text on element.

Comment 5

[alexander :surkov (:asurkov)]

(In reply to comment #4)

> Though that's ok to expose default text as a name (it's similar to HTML 5
> placeholder). But if HTML5 placeholder isn't used here then perhaps it's hard
> to expose default text and easiest way would be put aria-label attribute with
> this text on element.

Sorry, got used to think in Gecko bounds. GTK APIs likely don't have ARIA.

Comment 6

[Trevor Saunders (:tbsaunde)]

(In reply to comment #5)
> (In reply to comment #4)
> 
> > Though that's ok to expose default text as a name (it's similar to HTML 5
> > placeholder). But if HTML5 placeholder isn't used here then perhaps it's hard
> > to expose default text and easiest way would be put aria-label attribute with
> > this text on element.
> 
> Sorry, got used to think in Gecko bounds. GTK APIs likely don't have ARIA.

(In reply to comment #4)
> (In reply to comment #2)
> 
> > I claim this bug should be wontfix. The default values for the textareas tell
> > the user what to do.
> 
> I think he expect to hear an name of controls (which is exposed by associated
> label in general) while the value is different thing.

I'm not sure how well the default value explains the purpose of the field, I didn't observe any value in the text field whe I moved to it.  I haven't had the time yet to examine what is exposed to an AT, but the experience I had when using the crash reporter was that when I moved to one of the text fields I was told it was a text field, but not that there was a default value or that a label was in some way related, so I had no idea what was supposed to go in the field.  I could gues email or description, and a default might well be enough to confirm that.  When I explored the text field after moving to it I was unable to see any preexisting text in it that might have been a default value.  So this bug was to track looking into why ascreen reader provided no information as to the purpose of the text fields.
   
> 
> Though that's ok to expose default text as a name (it's similar to HTML 5
> placeholder). But if HTML5 placeholder isn't used here then perhaps it's hard
> to expose default text and easiest way would be put aria-label attribute with
> this text on element.

This seems like it might work, although I'm more familiar with other bits of linux accessibility than the exact details of gtk apis.  I should be able to look into the details of this in the next couple days.

Comment 7

[Marco Zehe (:MarcoZ)]

Is there a label for these text fields visually? If not, then we shouldn't put one in for screen readers, either, I think, since that would be giving information that is not available to others.

If, on the other hand, there are labels associated with those text fields, it's just a matter of correcting the XUL markup for Crash Reporter.

Comment 8

[(not currently active) Ted Mielczarek]

There aren't explicit labels for the text boxes. They have gray text that explains their purpose. The comment box has gray text that says "Add a comment (comments are publicly visible)" and the email box has gray text that says "Enter your email address here". However, if you focus either text box (or enter some text in them), we don't display that text anymore. It seems like making that text available to a screen reader when the text box is focused is the right thing to do, however.

Note that we're sort of abusing GTK to get the gray hint text displayed, we actually set the widget as insensitive and set its text to the hint text, but on focus we set it as sensitive and remove the text. (Unless you've entered text in the field, in which case we just leave that text there.)

Comment 9

[Trevor Saunders (:tbsaunde)]

(In reply to comment #8)
> There aren't explicit labels for the text boxes. They have gray text that
> explains their purpose. The comment box has gray text that says "Add a comment
> (comments are publicly visible)" and the email box has gray text that says
> "Enter your email address here". However, if you focus either text box (or
> enter some text in them), we don't display that text anymore. It seems like
> making that text available to a screen reader when the text box is focused is
> the right thing to do, however.

yup, See below for why I think this causes a problem.
 
> Note that we're sort of abusing GTK to get the gray hint text displayed, we
> actually set the widget as insensitive and set its text to the hint text, but
> on focus we set it as sensitive and remove the text. (Unless you've entered
> text in the field, in which case we just leave that text there.)

So, I suspect that what is happening is that the screen reader  doesn't query the text field for information until you've actually moved to it, at which point its too late to get any useful information.  Since I don't think we should require that screen readers examine every text field and other widget before you move to it on the odd chance  that something describing its purpose may go away I think we should make that text avalable  to screen readers after the widget has gained focus.  The idea in comment 1 to use a hidden gtk label seems reasonable, I'm not sure if there is something easier or better though, although I'm inclined to think not.  Marco, the crash reporter seems to be gtk not xul fwiw.

Comment 10

[(not currently active) Ted Mielczarek]

Okay, that seems fine to me. I'm not likely to write this patch as I'm neither a GTK expert nor an a11y expert, but I'd be happy to review one. The easiest way to test changes to the crash reporter client is to install the "Crash Me Now!" extension, and use Tools -> Crash Me! -> Null pointer deref! to cause a crash and launch the client.

crashme extension: http://crashme.googlecode.com/files/crashme-new.xpi

Comment 11

[timeless]

Attachment: missing GTK_LABEL() casts for first argument of calls to gtk_label_set_mnemonic_widget

someone should send this to try and find out if it works. i don't keep a build env and am doing this in the middle of the night on a whim.

Comment 12

[Trevor Saunders (:tbsaunde)]

Attachment: build log for the test patch

It looks like you need to cast the gtkwidget to a gtklabel somehow, I'm not sure what the right way to do this with gtk objects is.

Comment 13

[timeless]

Attachment: with casts

could you try this?

Comment 14

[Trevor Saunders (:tbsaunde)]

Comment on attachment 509536 [details] [diff] [review]
with casts

Ok, looks good, I also pushed to try for linux see http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-4c1700ec69d0 
Thanks for working on this!

Comment 15

[timeless]

trev: could you run the build and confirm it does the right thing for your reader? if so, please load <https://bugzilla.mozilla.org/attachment.cgi?id=509536&action=edit> and change feedback? to feedback+ and set review? :ted

thanks for helping

Comment 16

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/f049f500468d