Closed Bug 1411198 Opened 3 years ago Closed 2 years ago

Unlabeled voice input button

Categories

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

All
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 62
Tracking Status
fennec + ---
firefox60 --- wontfix
firefox61 --- wontfix
firefox62 --- verified

People

(Reporter: aarnaud, Assigned: shah, Mentored)

Details

(Keywords: good-first-bug)

Attachments

(3 files)

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.
Eitan, can you investigate?
Flags: needinfo?(eitan)
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.
Status: UNCONFIRMED → NEW
tracking-fennec: --- → ?
Ever confirmed: true
OS: Unspecified → Android
Hardware: Unspecified → All
Version: unspecified → Trunk
tracking-fennec: ? → +
Keywords: good-first-bug
Priority: -- → P2
I'm not current on the latest Android UI. Michael will probably have a good idea where to fix this.
Flags: needinfo?(eitan) → needinfo?(michael.l.comella)
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)?
Flags: needinfo?(michael.l.comella) → needinfo?(eitan)
Sure, you can ask UX. But I think any label is better than none.
Flags: needinfo?(eitan)
Hi, I would like to work on this bug, if it is possible. Thank you.
Hey Jaewoongyu – thanks for your interest! Feel free to start working on it and please ask :nechen if you have any questions!
Mentor: cnevinchen
Could you please guide me a little bit? It could be really appreciated.
and could you please assign it to me? Thank you.
What do you exactly mean by Open Firefox for Android? and how to do that?
I am not sure how to approach it to fix
(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.
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.
[triage] Bulk edit from title: this is a non-critical issue. Please remove priority if you wish this to be re-triaged.
Priority: P2 → P3
I've resolved this issue. I was wondering if someone could help walk me through getting access as a committer. 

Thanks,

Shah
Flags: needinfo?(cnevinchen)
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!
Flags: needinfo?(cnevinchen)
Attachment #8975707 - Flags: review?(michael.l.comella)
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.
Attachment #8975707 - Flags: review?(michael.l.comella) → review-
Assignee: nobody → shah
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! :)
Attachment #8976413 - Flags: review?(michael.l.comella) → review+
Comment on attachment 8976409 [details]
Bug 1411198 - Unlabeled voice input button.

https://reviewboard.mozilla.org/r/244558/#review251646
Attachment #8976409 - Flags: review?(michael.l.comella) → review+
Comment on attachment 8975707 [details]
Bug 1411198 - Unlabeled voice input button.

https://reviewboard.mozilla.org/r/243940/#review251648
Attachment #8975707 - Flags: review?(michael.l.comella) → review+
Pushed by michael.l.comella@gmail.com:
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
No worries about the delay! And I appreciate the feedback, this is how I become a better developer :)
Attachment #8976413 - Flags: checkin+
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.
Attachment #8976413 - Flags: checkin+
Device:
 - Nexus 6P (Android 8.1.0)

Verified in the latest Nightly, both of the buttons are properly announced.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.