Last Comment Bug 773039 - Talkback announces all items in menu when menu is shown
: Talkback announces all items in menu when menu is shown
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: -- normal (vote)
: Firefox 16
Assigned To: Eitan Isaacson [:eeejay]
:
Mentors:
Depends on: 773952
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-11 14:46 PDT by Eitan Isaacson [:eeejay]
Modified: 2012-07-14 07:16 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
Don't populate menu accessibility event with descendant strings. (1.30 KB, patch)
2012-07-11 14:47 PDT, Eitan Isaacson [:eeejay]
blassey.bugs: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Eitan Isaacson [:eeejay] 2012-07-11 14:46:37 PDT
This is confusing and chatty for blind users. This does not happen in other apps.
Comment 1 Eitan Isaacson [:eeejay] 2012-07-11 14:47:47 PDT
Created attachment 641207 [details] [diff] [review]
Don't populate menu accessibility event with descendant strings.
Comment 2 David Bolter [:davidb] 2012-07-12 17:19:25 PDT
Eitan how does this fix work? (Just curious)
Comment 3 Eitan Isaacson [:eeejay] 2012-07-12 17:21:41 PDT
(In reply to David Bolter [:davidb] from comment #2)
> Eitan how does this fix work? (Just curious)

The default implementation of dispatchPopulateAccessibilityEvent calls dispatchPopulateAccessibilityEvent of all the child views, and each view appends text to the event. The override here does not go in to child views.
Comment 4 David Bolter [:davidb] 2012-07-12 17:22:54 PDT
Bingo. Thanks!
Comment 5 Brad Lassey [:blassey] (use needinfo?) 2012-07-12 22:13:34 PDT
Comment on attachment 641207 [details] [diff] [review]
Don't populate menu accessibility event with descendant strings.

Review of attachment 641207 [details] [diff] [review]:
-----------------------------------------------------------------

Eitan, I'd be a little worried about the side effects of this change, but I suspect they'd all be related to accessibility and trust that you'll work through what ever crops up.
Comment 6 Eitan Isaacson [:eeejay] 2012-07-12 23:06:10 PDT
(In reply to Brad Lassey [:blassey] from comment #5)
> Comment on attachment 641207 [details] [diff] [review]
> Don't populate menu accessibility event with descendant strings.
> 
> Review of attachment 641207 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Eitan, I'd be a little worried about the side effects of this change, but I
> suspect they'd all be related to accessibility and trust that you'll work
> through what ever crops up.

First, yes. This is a code path that is only reached when accessibility is on. Second, it is relatively well documented here:
http://developer.android.com/guide/topics/ui/accessibility/apps.html#populate-events
Comment 7 Eitan Isaacson [:eeejay] 2012-07-12 23:10:50 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/6a5d9afb58e5
Comment 8 Ed Morley [:emorley] 2012-07-13 05:29:23 PDT
https://hg.mozilla.org/mozilla-central/rev/6a5d9afb58e5
Comment 9 Marco Zehe (:MarcoZ) 2012-07-13 05:34:37 PDT
Comment on attachment 641207 [details] [diff] [review]
Don't populate menu accessibility event with descendant strings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): default widget implementation
User impact if declined: Whenever a TalkBack user opens the menu, the whole menu items get read to them. Makes working very hard because one does not know which item actually has the focus.
Testing completed (on m-c, etc.): Yes, on a local build.
Risk to taking this patch (and alternatives if risky): None that we can see.
String or UUID changes made by this patch: None.
Comment 10 Alex Keybl [:akeybl] 2012-07-13 10:04:39 PDT
Comment on attachment 641207 [details] [diff] [review]
Don't populate menu accessibility event with descendant strings.

[Triage Comment]
Approved for FF15 given that the a11y reward outweighs the near-zero risk to a non-a11y user.

Note You need to log in before you can comment on or make changes to this bug.