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

RESOLVED FIXED

Status

()

defect
P3
major
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: MarcoZ, Assigned: surkov)

Tracking

({access, verified1.9.0.7, verified1.9.1})

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
blocking1.9.0.4 -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(2 attachments, 2 obsolete attachments)

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.
Flags: blocking1.9.1?
Flags: blocking1.9.0.4?
Assignee

Updated

11 years ago
Assignee: nobody → surkov.alexander
Assignee

Comment 1

11 years ago
Posted patch patch (obsolete) — Splinter Review
Attachment #340545 - Flags: review?(aaronleventhal)
Assignee

Updated

11 years ago
Attachment #340545 - Flags: review?(marco.zehe)
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!
Assignee

Comment 3

11 years ago
(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!
Assignee

Comment 4

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

Comment 5

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

Updated

11 years ago
Attachment #340545 - Flags: review?(aaronleventhal) → review+

Comment 6

11 years ago
Comment on attachment 340545 [details] [diff] [review]
patch

Other than that r+
Assignee

Comment 7

11 years ago
(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.
Assignee

Updated

11 years ago
Status: NEW → ASSIGNED

Comment 8

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

Comment 9

11 years ago
Posted patch patch2Splinter Review
up to dated to trunk
Attachment #340545 - Attachment is obsolete: true
Assignee

Updated

11 years ago
Flags: in-testsuite+
Assignee

Comment 10

11 years ago
checked in, http://hg.mozilla.org/mozilla-central/rev/32bfbcb0e5dd
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Flags: blocking1.9.0.4? → blocking1.9.0.4-
Posted patch Gecko 1.9.0 port of patch (obsolete) — Splinter Review
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.
Attachment #358357 - Flags: review?(surkov.alexander)
Assignee

Comment 12

11 years ago
Posted patch 1.9.0 port2Splinter Review
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?
Attachment #358357 - Attachment is obsolete: true
Attachment #358357 - Flags: review?(surkov.alexander)
Assignee

Comment 13

11 years ago
Marco, ping
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 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.
Attachment #358430 - Flags: approval1.9.0.7?

Updated

11 years ago
Flags: blocking1.9.1? → blocking1.9.1+
Keywords: fixed1.9.1
Priority: -- → P3
Comment on attachment 358430 [details] [diff] [review]
1.9.0 port2

Approved for 1.9.0.7, a=dveditz for release-drivers.
Attachment #358430 - Flags: approval1.9.0.7? → approval1.9.0.7+
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
Keywords: fixed1.9.0.7
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.
You need to log in before you can comment on or make changes to this bug.