715877 - JPAKE pairing code is not accessible to accessibility APIs

Status: VERIFIED FIXED

(Firefox :: Sync, defect)

Description

[Marco Zehe (:MarcoZ)]

The code that one needs to enter for a new device is not accessible to assistive technologies and thus cannot be read by a blind person without sighted assistance.

STR:
1. Run a Firefox that does not have sync set up yet.
2. On the start page, click "Setup sync".
3. Now, run a machine's Firefox that does have Sync set up in a profile.
4. From the start page, click "Pair a device".
5. You're instructed to enter the code shown on the new device that is to be added.

Expected: The code should be retrievable via accessibility APIs.
Actual: The whole UI piece contains the instruction to type in this code on the other device, the links, and the buttons, but not the code itself.
The code itself is probably some kind of graphic that is not accessible to screen readers or other assistive technologies, and no alternative text is provided for it.

Comment 1

[Philipp von Weitershausen [:philikon]]

Marco, thanks for the report. Let me first point out that the code is not graphical at all, so my initial suspicion is that there's some confusion over the setup process here. But I'm not ruling out an accessibility bug.

Just to be clear, this is how the pairing process works:

* Let's say you have
** Computer A with Sync set up and
** Computer B where Sync is not set up yet.

* You want to pair the two:
** On Computer B, you choose Set up Sync -> I have an account. Now you should see the code displayed in three form field boxes that are non-editable.
** On Computer A, you choose "Pair a Device" from the home tab or the Sync prefpane. The dialog that opens will let you enter the code shown on Computer B.

Is this what you did?

Comment 2

[Marco Zehe (:MarcoZ)]

Philipp, yes, this is what I did, and I did see the instructions that "this code" should be entered on the other device. However I could not read the code itself. So either these form fields are marked up in a way accessibility APIs don't see them, or for some reason they escaped my screen reader. In any way, perhaps they should be focusable, and just marked read-only so one can tab to each one of them. That way, one can review the code using the cursor keys and make sure it is accurately transferred to machine A.

Comment 3

[Philipp von Weitershausen [:philikon]]

Yeah we use XUL <textbox disabled="true"> form fields to display the code on Computer B [see 1,2]. I wonder if this is a XUL/a11n bug here or whether we should be doing something more a11n-friendly. I literally have no idea, so my hope is that Neil or David can answer this one for us... Neil? David? :)

[1] https://mxr.mozilla.org/mozilla-central/source/browser/base/content/syncSetup.xul#266
[2] https://mxr.mozilla.org/mozilla-central/source/browser/base/content/syncSetup.js#695

Comment 4

[Neil Deakin]

Do you actually want the textboxes to be readonly? Or not to be textboxes at all?

Comment 5

[Philipp von Weitershausen [:philikon]]

(In reply to Neil Deakin from comment #4)
> Do you actually want the textboxes to be readonly? Or not to be textboxes at
> all?

Yes, we want them to be read-only and, while not a technical requirement, IMHO we want them to at least look like textboxes too. That way the dialogue displaying the code on computer B looks very similar to the dialogue on computer A where you enter the code (the goal for the user being to make the two "look the same").

Comment 6

[Marco Zehe (:MarcoZ)]

Additional benefit is that on Computer B the user can review the code character by character using the arrow keys, making sure they can enter the code on computer A correctly. Simple labels would again have the problem that a) they're not focusable and b) they're not reviewable by a person using a screen reader, as easily as a read-only textbox is.

Comment 7

[Neil Deakin]

It sounds like you really just want readonly textboxes, not disabled textboxes.

Comment 8

[Marco Zehe (:MarcoZ)]

Yes, Sir! ;-)

Comment 9

[Philipp von Weitershausen [:philikon]]

(In reply to Neil Deakin from comment #7)
> It sounds like you really just want readonly textboxes, not disabled
> textboxes.

Ah. Yeah, that does sound like it would make more sense. Thanks, Neil!

Comment 10

[Philipp von Weitershausen [:philikon]]

Oops, wrong Murali.

Comment 11

[Murali]

Attachment: Textboxes made readonly instead of disabled

Could you please suggest the changes I need to make to the patch? Thanks!

Comment 12

[Marco Zehe (:MarcoZ)]

Comment on attachment 591024 [details] [diff] [review]
Textboxes made readonly instead of disabled

Tip: For even better accessibility, convert the xul:description above the textbox elements into a xul:label and add a control attribute to it that points at "easySetupPIN1". That way, when tabbing, for the first textbox, screen readers will speak the text of &addDevice.setup.enterCode.label;. That way, the user will know what these fields are for without having to review the whole window manually.

Comment 13

[Murali]

Attachment: Textboxes made readonly instead of disabled, description changed to label

Can you please tell me whether this is the expected patch? Thanks! :)

Comment 14

[Murali]

Attachment: Textboxes made readonly instead of disabled, description changed to label

Forgot to change the value attribute of label earlier.

Comment 15

[Philipp von Weitershausen [:philikon]]

Comment on attachment 591032 [details] [diff] [review]
Textboxes made readonly instead of disabled, description changed to label

This looks good to me and it seems like MarcoZ is on board with these changes, too. r=me.

Please update your patch to include your committer name and an appropriately formatted commit message. See http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed.

Thanks!

Comment 16

[Murali]

Attachment: Author information added, attributes alignment fixed (they line up now)

Comment 17

[Marco Zehe (:MarcoZ)]

Comment on attachment 591041 [details] [diff] [review]
Author information added, attributes alignment fixed (they line up now)

You could'v eleft the <label></label> in place, would have worked the same way, but this is just as good, too.

Comment 18

[Murali]

Thanks a lot! :)

Comment 19

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/services/services-central/rev/2879d93049e8

Comment 20

[Tracy Walker [:tracy]]

based on the fact I can copy the codes to clipboard, I say this is fixed. but will let the a11y folks verify with their technology.

Comment 21

[Gregory Szorc [:gps]]

Merged to m-c: https://hg.mozilla.org/mozilla-central/rev/2879d93049e8

Comment 22

[Tracy Walker [:tracy]]

m-c nightly build of 20120127 has copyable jpake code fields

Comment 23

[Marco Zehe (:MarcoZ)]

I can also confirm that this now works very nicely with screen readers in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120127 Firefox/12.0a1. Thanks Murali!

Comment 24

[Firefox Bug Husbandry Bot]

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