778588 - Support direct voice input from the location bar

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, enhancement)

Description

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

In Fennec you first have to open the keyboard and then tap the microphone icon to get to the speech recognition.

In Chrome you can tap the little microphone icon in the location bar to do direct voice entry. It does not show the keyboard at all, it immediately opens a small dialog that runs the voice recognition.

Maybe we can do the same in Fennec?

Comment 2

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I am de-duping this because I do not think this actually the same as 774309.

That bug talks about Fennec not being one of the choices after doing a voice search that also yields web results.

This bug is about having something similar to the little microphone icon in the location bar like Chrome does.

Comment 3

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

Attachment: Screenshot of feature in Chrome

Stefan is referring to the circled microphone icon in the location bar.

Comment 4

[Aaron Train [:aaronmt]]

We've got voice input button available on the 'Find in Page' (at least on stock keyboard) bar; so this is certainly possible.

Comment 5

[Aaron Train [:aaronmt]]

This can be fixed by changing 

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/layout/awesomebar_search.xml#10

android:inputType to 'text' - but this introduces keyboard suggestions; and we lose the ability to enter URI like about:pages.

I'm not sure how to get voice without losing other functionality; adding 'text' alongside the others didn't seem to add work.

Comment 6

[Aaron Train [:aaronmt]]

Actually never-mind the above; was thinking of the keyboard instead of the awesome bar, different context. Yes, the solution is to add a voice button on the awesome bar to maintain the current inputType on the URL bar.

Comment 7

[zac]

Regarding problem description, actually the microphone button is not on the keyboard on nexus4. The mic button has been replaced by the forward slash (useful) so one cannot get to voice input directly from the keyboard. Workaround is to assign mic button to 123 keyboard instead of ABC keyboard, but now voice input involves an extra step (from ABC keyboard, press 123 button, press mic icon) for all apps.  So the workaround is not a good solution.  Can we please add a mic icon to the awesome bar as shown in the attachment by st3fan?  Thx.

Comment 9

[James Hugman [:jhugman] [@jhugman]]

I can see two ways of doing this: 

 * add a button in http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/layout/toolbar_edit_layout.xml#19 It would be easiest to style this button distinct from the EditText box itself.
 * further customize the CustomEditText (or one of its parents) use a compound drawable (see https://github.com/yanchenko/droidparts/blob/develop/droidparts/src/org/droidparts/widget/ClearableEditText.java#L83 ).

Either way, separating the detection and firing of the RecognizerIntent (http://stackoverflow.com/questions/12003507/can-a-voice-action-button-be-added-next-to-edittext-in-android ) would make it easier to use in text prompts. http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/prompts/PromptInput.java#67

Preferred route: use compound drawables in http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/toolbar/ToolbarEditText.java calling out to http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/ActivityHandlerHelper.java#25 .

Once that's landed, we can think about the PromptInput bits.

Comment 10

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

(In reply to James Hugman [:jhugman] [@jhugman] from comment #9)
> I can see two ways of doing this: 
> 
>  * add a button in
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/
> layout/toolbar_edit_layout.xml#19 It would be easiest to style this button
> distinct from the EditText box itself.

Sounds like a good first step. I don't think we need to worry too much right now at making the Voice Recognizer available to all CustomEditText instances. Let's focus on the Toolbar first and get the UX working well enough. We also have an easier time scoping the "opt-in" Setting for just the Toolbar for now.

> Either way, separating the detection and firing of the RecognizerIntent
> (http://stackoverflow.com/questions/12003507/can-a-voice-action-button-be-
> added-next-to-edittext-in-android ) would make it easier to use in text
> prompts.
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/prompts/
> PromptInput.java#67

Separating is fine, but let's not go to Prompts just yet.

> Preferred route: use compound drawables in
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/toolbar/
> ToolbarEditText.java calling out to
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/
> ActivityHandlerHelper.java#25 .

Sounds like a plan. Mike Comella and Martyn should be able to provide some review support.

Comment 11

[James Hugman [:jhugman] [@jhugman]]

Attachment: Proof of concept patch.

TODO: add preferences, detection of recognizer (and removal of the option if not available), add to Prompt interface. 

NEED: Assets (currently taken from ICS standard icon set, which is the right color to be displayed on a white background). UX: How are the voice recognition strings get displayed (or do we just display the first guess).

A lot of the todo will change when we integrate QR Codes https://bugzilla.mozilla.org/show_bug.cgi?id=602818 .

Comment 12

[James Hugman [:jhugman] [@jhugman]]

Attachment: Added detection of recognizer intent

Plus tidy, and move code together.

Comment 13

[Richard Newman [:rnewman]]

Comment on attachment 8538517 [details] [diff] [review]
Added detection of recognizer intent

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

A quick driveby.

::: mobile/android/base/toolbar/ToolbarEditText.java
@@ +484,5 @@
> +    private void launchVoiceRecognizer() {
> +        Intent intent = createVoiceRecognizerIntent();
> +
> +        Activity activity = GeckoAppShell.getGeckoInterface().getActivity();
> +        ActivityHandlerHelper.startIntentForActivity(activity, intent, new ActivityResultHandler() {

BrowserToolbar.create is given a context as an argument (BrowserApp.onCreateView). That context is the activity it's being attached to.

GAS.gGI.gA is a very roundabout way of getting hold of a reference to an activity, and it's not quite correct -- it'll return the last launched activity, not necessarily the one that owns this toolbar.

You should also consider which activity you want to use here. sIFA uses that activity as the parent for the newly launched one. That might be an argument for this method.

Comment 14

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

Comment on attachment 8538517 [details] [diff] [review]
Added detection of recognizer intent

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

Looking pretty good so far!

Note that you seemed to have made some aesthetic changes to lines which you didn't functionally change - if these are intentional, do them in a separate patch (or bug). If not, please revert them! `hg qcref` in the crecord extension can help here.

(In reply to James Hugman [:jhugman] [@jhugman] from comment #11)
> NEED: Assets (currently taken from ICS standard icon set, which is the right
> color to be displayed on a white background). UX: How are the voice
> recognition strings get displayed (or do we just display the first guess).

For UX assets and interactions, I recommend NIing :antlam.

::: mobile/android/base/resources/layout/toolbar_edit_layout.xml
@@ +15,4 @@
>            android:inputType="textUri|textNoSuggestions"
>            android:imeOptions="actionGo|flagNoExtractUi|flagNoFullscreen"
>            android:selectAllOnFocus="true"
> +          android:drawableRight="@drawable/ab_mic"

Is this a file you added? Did you need to `hg add` it?

@@ +16,5 @@
>            android:imeOptions="actionGo|flagNoExtractUi|flagNoFullscreen"
>            android:selectAllOnFocus="true"
> +          android:drawableRight="@drawable/ab_mic"
> +          gecko:autoUpdateTheme="false"
> +        />

nit: Close tag on the line for the last attribute.

::: mobile/android/base/toolbar/ToolbarEditText.java
@@ +462,5 @@
>  
> +    /**
> +     * Voice recognizer detection, and display of the compound drawable.
> +     * The drawable is specified in layout.xml.
> +     * If the recognizer intent is not going to resolve to anything, we can blank it out.

nit: This comment should be more direct in saying what it does, e.g.

"Configures the voice recognizer by enabling/disabling the button if dictation is available on this device, ..."

imo, the comment specifying that the drawable is defined in the layout XML can better be placed near near setCompoundDrawables...

@@ +475,5 @@
> +    }
> +
> +    private Intent createVoiceRecognizerIntent() {
> +        Intent intent = new Intent(RecognizerIntent.ACTION_RECOGNIZE_SPEECH);
> +        intent.putExtra(RecognizerIntent.EXTRA_LANGUAGE_MODEL, RecognizerIntent.LANGUAGE_MODEL_FREE_FORM);

This is for the toolbar, right? I think LANGUAGE_MODEL_WEB_SEARCH would be more appropriate, but I don't know the details of how they work.

@@ +492,5 @@
> +                    case RecognizerIntent.RESULT_AUDIO_ERROR:
> +                    case RecognizerIntent.RESULT_CLIENT_ERROR:
> +                    case RecognizerIntent.RESULT_NETWORK_ERROR:
> +                    case RecognizerIntent.RESULT_SERVER_ERROR:
> +                        // TODO what do we when the voice recognizer has an error.

These all sound pretty unrecoverable (with the exception of perhaps audio error?). I'd say display a prompt notifying the user the action couldn't be completed and disabling voice input.

@@ +501,5 @@
> +                        break;
> +                }
> +
> +                if (resultCode != Activity.RESULT_OK) {
> +                    return;

Are there any cases here where we should notify the user?

@@ +505,5 @@
> +                    return;
> +                }
> +                List<String> voiceStrings = data.getStringArrayListExtra(RecognizerIntent.EXTRA_RESULTS);
> +
> +                // TODO what do we when the voice recognizer returns zero strings

Does this happen often?

@@ +666,5 @@
> +        private Drawable mIcon;
> +
> +        public OnTouchListener() {
> +            Drawable[] drawables = getCompoundDrawables();
> +            for (int i=0; i<drawables.length; i++) {

nit: Spaces between operators

@@ +672,5 @@
> +                    mIcon = drawables[i];
> +                    mIndex = i;
> +                }
> +            }
> +            mIcon.setBounds(0, 0, mIcon.getIntrinsicWidth(), mIcon.getIntrinsicHeight());

Is this necessary? What happens without it?

@@ +678,5 @@
> +
> +        @Override
> +        public boolean onTouch(View v, MotionEvent event) {
> +            boolean tapped;
> +            switch (mIndex) {

We only expect this to be on the right or left (i.e. rtl) side of the toolbar, right? No need to generalize this to the vertical direction then, since the code is untested and thus unmaintained.

@@ +696,5 @@
> +                    tapped = false;
> +            }
> +            if (tapped) {
> +                launchVoiceRecognizer();
> +        }

nit: indentation.

Comment 15

[Martyn Haigh (:mhaigh)]

Comment on attachment 8538517 [details] [diff] [review]
Added detection of recognizer intent

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

Looks like you'll need to change the manifest file too as it looks like we currently only include the RECORD_AUDIO permission when the MOZ_WEBRTC flag is in use.

::: mobile/android/base/locales/en-US/android_strings.dtd
@@ +566,5 @@
>  <!ENTITY actionbar_done "Done">
>  
> +<!-- Voice search in the awesome bar -->
> +<!ENTITY voicesearch_prompt "Say your search">
> +

This feels a bit...clunky.  How about "Speak now"?

::: mobile/android/base/toolbar/ToolbarEditText.java
@@ +50,5 @@
> +import org.mozilla.gecko.toolbar.BrowserToolbar.OnDismissListener;
> +import org.mozilla.gecko.toolbar.BrowserToolbar.OnFilterListener;
> +import org.mozilla.gecko.util.ActivityResultHandler;
> +import org.mozilla.gecko.util.GamepadUtils;
> +import org.mozilla.gecko.util.StringUtils;

Looks like these last two classes (GamepadUtils & StringUtils) are being imported twice (see lines 14 & 15) - also generally speaking we order our imported classes before the android imports.

@@ +470,5 @@
> +        if (intent.resolveActivity(getContext().getPackageManager()) == null) {
> +            setCompoundDrawablesWithIntrinsicBounds(null, null, null, null);
> +            return;
> +        }
> +        setOnTouchListener(new OnTouchListener());

Calling this private class "OnTouchListener" initially confused me as it's similar in name to the View.OnTouchListener class - perhaps we can rename to avoid ambiguity?

@@ +660,5 @@
>          }
>      }
> +
> +    private class OnTouchListener implements View.OnTouchListener {
> +

nit: remove blank line

Comment 16

[James Hugman [:jhugman] [@jhugman]]

Please could you review of voice integration copy and assets. 

Please see the APK:

APK with forces a retry error (never succeeds), to show full flow and copy. https://dl.dropboxusercontent.com/u/575756/bug-778588-voice-integration-assets-needed.apk

Assets taken straight from Material design.

antlamb not found?

Comment 17

[James Hugman [:jhugman] [@jhugman]]

Attachment: Address feedback, added error recovery.

Comment 18

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

Comment on attachment 8549658 [details] [diff] [review]
Address feedback, added error recovery.

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

Sorry for the delay! Seems like some solid code!

I think the final String choices should go through :antlam.

Also, some UX feedback from :antlam. My keyboard has a voice recognizer input button whose interaction allows you to speak, it fills in what you said, but doesn't drop the recognizer, which allows you to speak more to fill in more text - I do like this interaction though the quickness of Chrome's voice input (which seems to be the thread we're following here) seems useful.

::: mobile/android/base/AndroidManifest.xml.in
@@ +70,5 @@
>      <uses-feature android:name="android.hardware.audio.low_latency" android:required="false"/>
>      <uses-feature android:name="android.hardware.camera.any" android:required="false"/>
>      <uses-feature android:name="android.hardware.microphone" android:required="false"/>
>  #endif
> +    <uses-permission android:name="android.permission.RECORD_AUDIO"/>

I'm not too familiar with the permissions system, but do we actually need this? I'd think because we're passing the recording off to another Activity, it's not necessary to require this for ourselves.

This post seems to agree with me: http://stackoverflow.com/a/6872920

::: mobile/android/base/locales/en-US/android_strings.dtd
@@ +558,5 @@
> +<!-- Voice search in the awesome bar -->
> +<!ENTITY voicesearch_prompt "Speak now…">
> +<!ENTITY voicesearch_failed_title "&brandShortName; Voice Search">
> +<!ENTITY voicesearch_failed_message "There is a problem with voice search right now. Please try later.">
> +<!ENTITY voicesearch_failed_message_recoverable "Your words could not recognized. Try speaking louder or slower or more clearly.">

nit: -> could not be recognized. Please try again.

Notably, I don't think we should assume the best way to improve voice recognition - better to let the user assess the situation for themselves and make assumptions.

::: mobile/android/base/toolbar/ToolbarEditText.java
@@ +452,5 @@
>          };
>      }
>  
> +    /**
> +     * Add the detect if we have the capability for, and differentially show compound drawables.

nit: This comment doesn't make sense to me.

@@ +458,5 @@
> +     */
> +    private void configureCompoundDrawables() {
> +        Intent intent = createVoiceRecognizerIntent();
> +        if (intent.resolveActivity(getContext().getPackageManager()) == null) {
> +            setCompoundDrawablesWithIntrinsicBounds(null, null, null, null);

nit: Add a comment saying that voice search is already declared in the XML and we're undoing that here.

@@ +485,5 @@
> +                    case RecognizerIntent.RESULT_SERVER_ERROR:
> +                        // We have an temporarily unrecoverable error.
> +                        handleVoiceSearchError(false);
> +                        break;
> +                    case RecognizerIntent.RESULT_AUDIO_ERROR:

nit: Generally, newline after breaks. Here and below.

@@ +502,5 @@
> +                // We have a valid result.
> +                List<String> voiceStrings = data.getStringArrayListExtra(RecognizerIntent.EXTRA_RESULTS);
> +
> +                // We have RESULT_OK, not RESULT_NO_MATCH so it should be safe to assume that
> +                // we have at least one match.

nit: combine with the comment above.

@@ +507,5 @@
> +
> +                // Use just the first result. Presenting more will mean building out UI.
> +                String text = voiceStrings.get(0);
> +                setText(text);
> +                setSelection(0, text.length());

You should talk to :antlam about whether the desired behavior is to replace the given text, or append to it (i.e. `setText(text)` or `setText(getText() + " " + text)` or similar).

@@ +515,5 @@
> +
> +    private void handleVoiceSearchError(boolean offerRetry) {
> +        AlertDialog.Builder builder = new AlertDialog.Builder(getContext())
> +            .setTitle(R.string.voicesearch_failed_title)
> +            .setIcon(R.drawable.icon).setNeutralButton(android.R.string.cancel, new DialogInterface.OnClickListener() {

nit: Each chained call should be on its own line so setNeutralButton should also be on a new line.

@@ +524,5 @@
> +            });
> +
> +        if (offerRetry) {
> +            builder.setMessage(R.string.voicesearch_failed_message_recoverable)
> +                .setNegativeButton(R.string.voicesearch_failed_retry, new DialogInterface.OnClickListener() {

How did you choose what to use for each of the negative/neutral buttons?

@@ +685,5 @@
>      }
> +
> +    private class VoiceSearchOnTouchListener implements View.OnTouchListener {
> +        private int mIndex = -1;
> +        private Drawable mIcon;

nit: mVoiceSearchIcon & mVoiceSearchIndex

It took me a second to recognize what the assignment was doing otherwise.

@@ +702,5 @@
> +        public boolean onTouch(View v, MotionEvent event) {
> +            boolean tapped;
> +            switch (mIndex) {
> +                case 0:
> +                    tapped = event.getX() < (getPaddingLeft() + mIcon.getIntrinsicWidth());

Are the MotionEvent coordinates given in the context of the containing View or the screen? I assume the former?

Comment 19

[Martyn Haigh (:mhaigh)]

Comment on attachment 8549658 [details] [diff] [review]
Address feedback, added error recovery.

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

Couple of nits but looks good with mcomellas comments taken onboard too.

::: mobile/android/base/resources/layout/toolbar_edit_layout.xml
@@ +16,5 @@
>            android:imeOptions="actionGo|flagNoExtractUi|flagNoFullscreen"
>            android:selectAllOnFocus="true"
> +          android:drawableRight="@drawable/ab_mic"
> +          gecko:autoUpdateTheme="false"
> +        />

nit: put the tag close on the same line as the last attribute

::: mobile/android/base/toolbar/ToolbarEditText.java
@@ +472,5 @@
> +        return intent;
> +    }
> +
> +    private void launchVoiceRecognizer() {
> +        Intent intent = createVoiceRecognizerIntent();

nit: final

Comment 20

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Sorry for the delay! 

This is looking great. But it seems to not work... I attempted a couple of tries and in the same environment with this versus Google Now. Can we see about improving these performance issues? Under the same environment I got much better results with Google Now.

We can work on the dialog box visuals afterwards :)

Comment 21

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Sidenote: an "Accessibility" item or something under Settings would be a great place to stick this pref.

Comment 22

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

(In reply to Anthony Lam (:antlam) from comment #20)
> But it seems to not work...

iirc, somewhere in the code it states that the input is on a loop for testing purposes.

Comment 23

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

(In reply to Anthony Lam (:antlam) from comment #20)
> Sorry for the delay! 
> 
> This is looking great. But it seems to not work... I attempted a couple of
> tries and in the same environment with this versus Google Now. Can we see
> about improving these performance issues? Under the same environment I got
> much better results with Google Now.

I missed this note from James in comment 16:

 APK with forces a retry error (never succeeds), to show full flow and copy. 

> We can work on the dialog box visuals afterwards :)

Anthony - Can you look over the text in the error dialog in this build? You might not easily see it in "real" builds.

Comment 24

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

Just a ping: I'd like to get this code moving again. Let's not block on the strings. Let's get a revised patch ready and started on the review. We can tweak strings later.

Comment 25

[James Hugman [:jhugman] [@jhugman]]

Attachment: Change patch to address nits.

Comment 26

[Martyn Haigh (:mhaigh)]

Comment on attachment 8560789 [details] [diff] [review]
Change patch to address nits.

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

Overall code looks good - a few comments:

- It seems to accurately detect my voice
- Multiple input appends to the end of the current input, it doesn't replace it.  I don't know if this was intentional but I'd prefer it to replace the current text.

::: mobile/android/base/toolbar/ToolbarEditText.java
@@ +474,5 @@
> +     *
> +     * Currently, only voice input.
> +     */
> +    private void configureCompoundDrawables() {
> +        Intent intent = createVoiceRecognizerIntent();

+final - here and whereever else you can

@@ +543,5 @@
> +                });
> +
> +        if (offerRetry) {
> +            builder
> +                    .setMessage(R.string.voicesearch_failed_message_recoverable)

Join these two lines and realign the folowwing lines.

Comment 27

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Mark Finkle (:mfinkle) from comment #24)
> Just a ping: I'd like to get this code moving again. Let's not block on the
> strings. Let's get a revised patch ready and started on the review. We can
> tweak strings later.

Agree, from a UX point of view, 3 things. 
 - 1) I want to make sure we're keeping to system default UI when it comes to dialog box, so we're not creating new UI

 - 2) I think surfacing the "mic" icon earlier (on the right end of the URL bar before the user taps to activate it) would make this feature more prominent and easier to get to

 - 3) Copy update? "Firefox Voice Input" Sorry! We could not recognize your words. Please try again.

Comment 28

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

(In reply to Anthony Lam (:antlam) from comment #27)
> (In reply to Mark Finkle (:mfinkle) from comment #24)
> > Just a ping: I'd like to get this code moving again. Let's not block on the
> > strings. Let's get a revised patch ready and started on the review. We can
> > tweak strings later.
> 
> Agree, from a UX point of view, 3 things. 
>  - 1) I want to make sure we're keeping to system default UI when it comes
> to dialog box, so we're not creating new UI

We are.

>  - 2) I think surfacing the "mic" icon earlier (on the right end of the URL
> bar before the user taps to activate it) would make this feature more
> prominent and easier to get to

I'm going to push back on this for now. I don't think there is a strong reason to show the mic in the URLBar before going into edit mode, but it will reduce the amount of free space even more. Let's see if we need it.

>  - 3) Copy update? "Firefox Voice Input" Sorry! We could not recognize your
> words. Please try again.

I can make those changes.

I am also chaining the patch to "replace" exiting text instead of appending to it.

The only other thing I would like to see is the keyboard come back after the text is replaced. It's kinda awkward to get the keyboard to re-appear after voice input.

With James on PTO for a bit, I'll work to get this patch landed.

Comment 29

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Mark Finkle (:mfinkle) from comment #28)
> (In reply to Anthony Lam (:antlam) from comment #27)
> > (In reply to Mark Finkle (:mfinkle) from comment #24)
> > > Just a ping: I'd like to get this code moving again. Let's not block on the
> > > strings. Let's get a revised patch ready and started on the review. We can
> > > tweak strings later.
> > 
> > Agree, from a UX point of view, 3 things. 
> >  - 1) I want to make sure we're keeping to system default UI when it comes
> > to dialog box, so we're not creating new UI
> 
> We are.
> 
> >  - 2) I think surfacing the "mic" icon earlier (on the right end of the URL
> > bar before the user taps to activate it) would make this feature more
> > prominent and easier to get to
> 
> I'm going to push back on this for now. I don't think there is a strong
> reason to show the mic in the URLBar before going into edit mode, but it
> will reduce the amount of free space even more. Let's see if we need it.

Sorry, I think I wasn't very clear. I was referring just to the New Tab (i.e. when the URL bar is empty). I agree, once the page has content, we might want to tuck that in the active state of the URL bar. 

Thoughts?

Comment 30

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

Comment on attachment 8560789 [details] [diff] [review]
Change patch to address nits.

Updated for Mike and Martyn's comments. I added a Nightly flag too.

Comment 31

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

https://hg.mozilla.org/integration/fx-team/rev/38a668c3efaa

Comment 32

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

(In reply to Anthony Lam (:antlam) from comment #29)

> Sorry, I think I wasn't very clear. I was referring just to the New Tab
> (i.e. when the URL bar is empty). I agree, once the page has content, we
> might want to tuck that in the active state of the URL bar. 
> 
> Thoughts?

I think that is a different enough usecase to treat it as a new bug.

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/38a668c3efaa

Comment 34

[Aaron Train [:aaronmt]]

Flaviu can you verify this on a range of Android versions?

Comment 35

[Flaviu Cos]

Tested on multiple devices and the results are:

Samsung Galaxy R (Android 2.3.4)
- The voice input launches directly with the error 'Audio problem'; Tapping on 'Speak again' will work though.
- After the voice input 'No speech heard' error is displayed, however tapping 'Cancel' on the error will insert the voice input into the URL bar.

Asus Transformer Tab (Android 4.0.3) OK

Motorola Razr (Android 4.1.2) OK

Samsung Galaxy Nexus (Android 4.2.1) OK

Asus Transformer Tab (Android 4.2.1)
- Double notifications are displayed if the words where unrecognized (one from default speech and one from firefox.

Samsung Galaxy S4 (Android 4.3)  
- The speak now notification sound is played multiple times.
- Double notifications are displayed if the words where unrecognized (one from default speech and one from firefox.

Google Nexus 4 (Android 4.4.4) OK

Google Nexus 7 (Android 5.0.2) OK

Can this bug be considered fixed?
Should I file a bug for each issue mentioned above?

Comment 36

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

(In reply to Flaviu Cos, QA [:flaviu] from comment #35)

Thanks for the testing!

> Can this bug be considered fixed?
> Should I file a bug for each issue mentioned above?

Yes, please.

Comment 37

[Kevin Brosnan [Ex-Mozilla]]

Tracking Nightly+ to keep track of features that are nightly only.

Comment 38

[Sylvestre Ledru [:Sylvestre]]

Both Mark & Margaret told me that we will ship this feature with 42.
Added to the release notes using "Support direct voice input from the URL bar"
as wording.
I will update the item once we have some doc.

Comment 39

[Richard Z.]

This is nice but for non-english users the language needs to be easily switchable and configurable

https://bugzilla.mozilla.org/show_bug.cgi?id=1284871