457166 - Conversation links in GMail Inbox do not expose an action, cannot be activated by screen readers.

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect, P3)

Description

[Marco Zehe (:MarcoZ)]

This was reported by Freedom Scientific, but probably affects all screen readers across all platforms.

STR:
1. Login to your GMail account.
2. Go to Inbox.
3. Look at the links for any of the inbox conversations. Using a screen reader, try to activate any of the links.

Result: It does not work.

Observations:
1. The table cell containing the subject and possible number of messages in the conversation is given an ARIA role of "link".
2. The text is nested within 2 divs and a span within that table cell.
3. The link accessible created for this does not expose any actions, making it impossible for screen readers to activate it.

This appears to be new in Google's markup, I remember having seen this working previously.

This prevents *any* screen reader from working with this major web app, requesting that this block 1.9.1 and also one of the next 1.9.0.x releases.

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 2

[Marco Zehe (:MarcoZ)]

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

I'm getting one test failure when running the whole suite:
821 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/a11y/accessible/test_nsIAccessible_actions.html | Wrong action name of the accessible for combobox_expanded - got "open", expected "close"

Did you see this while developing the feature? r=me with that answered/if the tests run OK on tinderboxes.

Other than that, the patch fixes the problem at gmail. Great work!

Comment 3

[alexander :surkov (:asurkov)]

(In reply to comment #2)
> (From update of attachment 340545 [details] [diff] [review])
> I'm getting one test failure when running the whole suite:
> 821 ERROR TEST-UNEXPECTED-FAIL |
> chrome://mochikit/content/a11y/accessible/test_nsIAccessible_actions.html |
> Wrong action name of the accessible for combobox_expanded - got "open",
> expected "close"
> 
> Did you see this while developing the feature? r=me with that answered/if the
> tests run OK on tinderboxes.

Yep. I didn't think you test the patches ;) Actually I think it's a bug that aria-expanded didn't drop collapsed state. I'll file the bug and fix it so this patch won't be modified. Of course, I'm ensure to land that bug before this one. Thank you you keeping your eyes open.

> Other than that, the patch fixes the problem at gmail. Great work!

Great. Well we ware right then!

Comment 4

[alexander :surkov (:asurkov)]

Marco, I filed bug 457219 for the issue with collapsed/expanded states.

Comment 5

[Aaron Leventhal]

Why do we pass in the state as a parameter instead of just using State(this) inside the method?

Comment 6

[Aaron Leventhal]

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

Other than that r+

Comment 7

[alexander :surkov (:asurkov)]

(In reply to comment #5)
> Why do we pass in the state as a parameter instead of just using State(this)
> inside the method?

I just remember state calculating may be complex task (for example visibility states). So once I got state I try to reuse it.

Comment 8

[Aaron Leventhal]

Thanks, that's what I thought. I didn't see the state reuse before but I see it now.

Comment 9

[alexander :surkov (:asurkov)]

Attachment: patch2

up to dated to trunk

Comment 10

[alexander :surkov (:asurkov)]

checked in, http://hg.mozilla.org/mozilla-central/rev/32bfbcb0e5dd

Comment 11

[Marco Zehe (:MarcoZ)]

Attachment: Gecko 1.9.0 port of patch

Alex, can you take a look at this? It works in Gmail, but something is wrong with the tests. It doesn't run through the tests, doesn't output a log, and the test harness window is open with some content I don't have access to. I suppose something went wrong with porting the test or with what gets included from common.js.

Comment 12

[alexander :surkov (:asurkov)]

Attachment: 1.9.0 port2

I see one test failure in test_nsIAccessible_actions.html

not ok - Wrong action name of the accessible for combobox_expanded got "open", expected "close"

it's because bug 457219 wasn't fixed on 1.9.0. Until we don't need that bug there I would suggest to remove tests for "combobox_expanded".

test_nsIAccessible_actions.xul works fine

running all tests together (manually "run tests" button) works fine with fixed test_nsIAccessible_actions.html.

Does it work for you?

Comment 13

[alexander :surkov (:asurkov)]

Marco, ping

Comment 14

[Marco Zehe (:MarcoZ)]

Yes, this patch works in principle, although I find it strange that I can't seem to be able to get all tests running with our usual command line. It opens the test harness window instead of running the tests and putting stuff into the log file. But anyway...The tests pass for me with your patch applied.

Comment 15

[Marco Zehe (:MarcoZ)]

Comment on attachment 358430 [details] [diff] [review]
1.9.0 port2

This patch addresses a problem screen readers have in GMail where they can't open a message anymore due to changes in Google's markup.

Comment 16

[Daniel Veditz [:dveditz]]

Comment on attachment 358430 [details] [diff] [review]
1.9.0 port2

Approved for 1.9.0.7, a=dveditz for release-drivers.

Comment 17

[Marco Zehe (:MarcoZ)]

Checking in accessible/src/base/nsARIAMap.cpp;
/cvsroot/mozilla/accessible/src/base/nsARIAMap.cpp,v  <--  nsARIAMap.cpp
new revision: 1.29; previous revision: 1.28
done
Checking in accessible/src/base/nsARIAMap.h;
/cvsroot/mozilla/accessible/src/base/nsARIAMap.h,v  <--  nsARIAMap.h
new revision: 1.11; previous revision: 1.10
done
Checking in accessible/src/base/nsAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessible.cpp,v  <--  nsAccessible.cpp
new revision: 1.380; previous revision: 1.379
done
Checking in accessible/src/base/nsAccessible.h;
/cvsroot/mozilla/accessible/src/base/nsAccessible.h,v  <--  nsAccessible.h
new revision: 1.125; previous revision: 1.124
done
Checking in accessible/tests/mochitest/nsIAccessible_actions.js;
/cvsroot/mozilla/accessible/tests/mochitest/nsIAccessible_actions.js,v  <--  nsIAccessible_actions.js
new revision: 1.2; previous revision: 1.1
done
Checking in accessible/tests/mochitest/test_nsIAccessible_actions.html;
/cvsroot/mozilla/accessible/tests/mochitest/test_nsIAccessible_actions.html,v  <--  test_nsIAccessible_actions.html
new revision: 1.2; previous revision: 1.1
done
Checking in accessible/tests/mochitest/test_nsIAccessible_actions.xul;
/cvsroot/mozilla/accessible/tests/mochitest/test_nsIAccessible_actions.xul,v  <--  test_nsIAccessible_actions.xul
new revision: 1.3; previous revision: 1.2
done

Comment 18

[Marco Zehe (:MarcoZ)]

Verified fixed in Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.0.7pre) Gecko/2009021006 GranParadiso/3.0.7pre

Also, this landed on m-c when m-c was still 1.9.1, setting verified accordingly.