1411198 - Unlabeled voice input button

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Alex ARNAUD]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20171003235057

Steps to reproduce:

Dear all,

I'm a blind person that uses TalkBack (screen reader) to use Firefox on Android. An unlabeled button on Firefox makes the Firefox experience more difficult.


1) Open Firefox for Android (here beta)
2) Open a new tab
3) Select the URL bar to enter a URL
4) The button to launch voice input in not labeled


Actual results:

Talkback speaks "unbaled button"


Expected results:

Talkback should speaks "voice input button" (or something like that, I'm French, I don't know the exact word that is used on others apps like Chrome)

Best regards.

Comment 1

[Marco Zehe (:MarcoZ)]

Eitan, can you investigate?

Comment 2

[Bogdan Surd, QA [:bsurd, NI]]

Checked this on a Nexus 6P (8.0) and I can confirm that both the Voice input and QR code reader buttons are announced as unlabeled buttons.

Comment 3

[Eitan Isaacson [:eeejay]]

I'm not current on the latest Android UI. Michael will probably have a good idea where to fix this.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I don't have cycles to fix this myself but:
- Here is the button: https://searchfox.org/mozilla-central/rev/c99d035f00dd894feff38e4ad28a73fb679c63a6/mobile/android/app/src/photon/res/layout/toolbar_edit_layout.xml#56
- It will need a contentDescription added
- QRCode also needs a contentDescription (as Bogdan mentions)

Eitan, what strings would you recommend (or should we ask UX)?

Comment 5

[Eitan Isaacson [:eeejay]]

Sure, you can ask UX. But I think any label is better than none.

Comment 6

[Jaewoongyu]

Hi, I would like to work on this bug, if it is possible. Thank you.

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Hey Jaewoongyu – thanks for your interest! Feel free to start working on it and please ask :nechen if you have any questions!

Comment 8

[Jaewoongyu]

Could you please guide me a little bit? It could be really appreciated.

Comment 9

[Jaewoongyu]

and could you please assign it to me? Thank you.

Comment 10

[Jaewoongyu]

What do you exactly mean by Open Firefox for Android? and how to do that?

Comment 11

[Jaewoongyu]

I am not sure how to approach it to fix

Comment 12

[Alex ARNAUD]

(In reply to Jaewoongyu from comment #10)
> What do you exactly mean by Open Firefox for Android? and how to do that?

I open the Android built-in application launcher and I touch the icon Firefox or Firefox Beta.

Best regards.

Comment 13

[Frank Gauthier]

I believe I have made the necessary changes to the source code. I am just reviewing how to submit this as a patch so it can be reviewed and tested.

Comment 14

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

[triage] Bulk edit from title: this is a non-critical issue. Please remove priority if you wish this to be re-triaged.

Comment 15

[Shah Syed]

I've resolved this issue. I was wondering if someone could help walk me through getting access as a committer. 

Thanks,

Shah

Comment 16

[Nevin Chen(Not active on Bugzilla)]

Hi there! Welcome!
1. Use mozreview to submit your patch (http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html)
2. Find a reviewer (in your commit message, just add r?xxxx at the end)
3. After it lands, find someone to vouch for you to get level one access. 
Hope that helps!

Comment 17

[Shah Syed]

Attachment: Bug 1411198 - Unlabeled voice input button.

Added contentDescription strings for QR Code and Voice Input

Review commit: https://reviewboard.mozilla.org/r/243940/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/243940/

Comment 18

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8975707 [details]
Bug 1411198 - Unlabeled voice input button.

https://reviewboard.mozilla.org/r/243940/#review250582

Thanks so much for tackling this, Shah – this is on the right track!

Please append `r=mcomella` to your commit message in your next revision to add me as your reviewer!

::: mobile/android/base/locales/en-US/android_strings.dtd:72
(Diff revision 1)
> +<!ENTITY url_bar_qrcode_text2 "Use QR Code to view website">
> +<!ENTITY url_bar_mic_text2 "Use microphone to enter address">

nit: Can you add a localization note to each of these two strings? See the pattern in the file below. We want to make it clear that this string will only be used for accessibility, e.g.

"This text will be heard by non-visual users when the focus the QR code reader button in the url bar."

::: mobile/android/base/locales/en-US/android_strings.dtd:72
(Diff revision 1)
> +<!ENTITY url_bar_qrcode_text2 "Use QR Code to view website">
> +<!ENTITY url_bar_mic_text2 "Use microphone to enter address">

nit: I think we could make the text a little more explicit. How about:

QR: "Open a website with your QR code reader"
Microphone: "Use your microphone to search or enter address"

Sorry to be so nitpicky. My thinking:

QR: This makes it clearer a QR code *reader* is being used to select a website to view, rather than other interpretations.

Microphone: Our url bar text is "Search or enter address" so we should be consistent.

Comment 19

[Shah Syed]

Attachment: Bug 1411198 - Unlabeled voice input button.

Added localization notes and more verbose strings for non-visual users.

Review commit: https://reviewboard.mozilla.org/r/244558/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/244558/

Comment 20

[Shah Syed]

Comment on attachment 8975707 [details]
Bug 1411198 - Unlabeled voice input button.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/243940/diff/1-2/

Comment 21

[Shah Syed]

Attachment: Bug 1411198 - Unlabeled voice input button.

Made QR string consistent with respect to other url related strings

Review commit: https://reviewboard.mozilla.org/r/244566/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/244566/

Comment 22

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8976413 [details]
Bug 1411198 - Unlabeled voice input button.

https://reviewboard.mozilla.org/r/244566/#review251644

This looks good to me, thanks Shah! Sorry it took me so long to get back to you – it was busy last week.

One nit I have is the commit style:
- For a single commit, changes for review comments should generally not be addressed in separate commits, i.e. they should be squashed (we have review tools, called "interdiff" that let us see what changed between pushes so the commits are not necessary)
- For multiple commits, it's generally not worth the time to squash the review comment changes into their original commits: then they generally shouldn't be squashed.
- Commits should never have the same description, even for review comments. In this case, your extended commit summaries would actually be appropriate commit titles, e.g. "Bug xyz - review: Made QR string consistent with respect to other url related strings"

That being said, I don't think it's worth the turn-around time to fix this so I'm just going to land it. Keep it in mind for the future though! :)

Comment 23

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8976409 [details]
Bug 1411198 - Unlabeled voice input button.

https://reviewboard.mozilla.org/r/244558/#review251646

Comment 24

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8975707 [details]
Bug 1411198 - Unlabeled voice input button.

https://reviewboard.mozilla.org/r/243940/#review251648

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/291f380e8167
Unlabeled voice input button. r=mcomella
https://hg.mozilla.org/integration/autoland/rev/6afab2248a4d
Unlabeled voice input button. r=mcomella
https://hg.mozilla.org/integration/autoland/rev/3f3064a1dfed
Unlabeled voice input button. r=mcomella

Comment 26

[Shah Syed]

No worries about the delay! And I appreciate the feedback, this is how I become a better developer :)

Comment 27

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/291f380e8167
https://hg.mozilla.org/mozilla-central/rev/6afab2248a4d
https://hg.mozilla.org/mozilla-central/rev/3f3064a1dfed

Comment 28

[Shah Syed]

Comment on attachment 8976413 [details]
Bug 1411198 - Unlabeled voice input button.

Disregard last comment, I thought code needed to be checked in but it was already done.

Comment 29

[Bogdan Surd, QA [:bsurd, NI]]

Device:
 - Nexus 6P (Android 8.1.0)

Verified in the latest Nightly, both of the buttons are properly announced.