753859 - Button to open tabs has no proper accessibility label, TalkBack uust says "Image button"

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Marco Zehe (:MarcoZ)]

1. Download Fennec nightly.
2. Turn on accessibility and TalkBack.
3. Run Fennec.
4. navigate to the top with your directional controller. TalkBack will say "Enter search or address Button".
5. Navigate to the right, onto the button that opens the tab list.

Expected: TalkBack should say something useful.
Actual: TalkBack will just say "Image button". The image needs proper alternative text for TalkBack to speak its meaning correctly.

This is in the native UI part of Fennec and thus not part of the Accessibility APIs Core component.

Comment 1

[Eitan Isaacson [:eeejay]]

It is spoken as "image button", but visually it shows a +, or a number greater than one when there are tabs. That should somehow be relayed textually. Maybe "New Tab", and "n Tabs".

Comment 3

[Eitan Isaacson [:eeejay]]

Attachment: Added description to toolbar tabs button.

This adds some strings to localize as well. Biggest disruption is making the resource a class member.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 622986 [details] [diff] [review]
Added description to toolbar tabs button.

Looks OK to me, but I want Sriram to OK holding on the mResources instead of just doing a getResources() as needed.

Comment 5

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 622986 [details] [diff] [review]
Added description to toolbar tabs button.

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

I wouldn't add a mResources variable. It can be got from mContext when needed.
For the case where the string is needed, the Context supports it (internally it does thisContext.getResources().getString()).
http://developer.android.com/reference/android/content/Context.html#getString%28int%29

Comment 6

[Eitan Isaacson [:eeejay]]

Attachment: Added description to toolbar tabs button.

Removed mResource, added localization note.

Comment 7

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 623226 [details] [diff] [review]
Added description to toolbar tabs button.

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

This looks good to me.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 623226 [details] [diff] [review]
Added description to toolbar tabs button.


>+            mTabs.setContentDescription(mContext.getString(R.string.num_tabs, count));

>+<!-- Localization note (num_tabs) : Number of tabs is always plural. -->
>+<!ENTITY num_tabs "&#037;d tabs">

Some languages do not have just singular and plural states. Sometimes there are more states. I don't know if Android can handle those. This might be as good as we can do for now.

Looping in L10N just in case

Comment 9

[Eitan Isaacson [:eeejay]]

Comment on attachment 623226 [details] [diff] [review]
Added description to toolbar tabs button.

Getting Pike in the convo. I'll wait till next week to land this.

Comment 10

[Axel Hecht [:Pike]]

Comment on attachment 623226 [details] [diff] [review]
Added description to toolbar tabs button.

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

Android does support plurals, http://developer.android.com/guide/topics/resources/string-resource.html#Plurals. Sadly, that requires a different set of XML elements per language, and we can't do that with our hack to get DTDs in.

Short nit on the comment below. f=me with that.

::: mobile/android/base/locales/en-US/android_strings.dtd
@@ +45,5 @@
>  <!ENTITY forward "Forward">
>  
>  <!ENTITY close_tab "Close Tab">
>  <!ENTITY new_tab "New Tab">
> +<!-- Localization note (num_tabs) : Number of tabs is always plural. -->

I'd say "more than one" instead of "plural". Also, add a comment that we can't use real plural forms, referencing this bug?

Something like "We can't use android plural forms, sadly".

Comment 11

[Eitan Isaacson [:eeejay]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/0febfcd8f16c

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0febfcd8f16c

Comment 13

[Marco Zehe (:MarcoZ)]

Verified fixed in Fennec/15.0a1 2012-05-21