The search button should not dismiss the awesomescreen

VERIFIED FIXED in Firefox 16

Status

()

--
enhancement
VERIFIED FIXED
7 years ago
2 years ago

People

(Reporter: paul.feher, Assigned: vangelis_51)

Tracking

Trunk
Firefox 16
ARM
Android
Points:
---

Firefox Tracking Flags

(blocking-fennec1.0 -)

Details

(Whiteboard: [mentor=mbrubeck@mozilla.com][good first bug][lang=java])

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
Build: Nightly:Fennec/13.0a1 (2012-02-21)
Device: HTC Desire Z 
OS: Android 2.3

Steps to reproduce:
1. Open Fennec.
2. Tap on the awesome bar to open the awesome panel.
3. Tap on the search button of your android phone.

Expected Result:
This should open up the Fennec search menu that should list the following search options: Google, Amazon.com, Twitter, Wikipedia.

Actual Result: 
The fennec home page is displayed.
Ux feedback wanted here.
Severity: normal → enhancement
blocking-fennec1.0: --- → ?
This doesn't block the first release, though I think it's a reasonable thing to implement.  If anyone wants to work on this I can help out.
blocking-fennec1.0: ? → -
Whiteboard: [mentor=mbrubeck@mozilla.com][good first bug][lang=java]

Comment 3

7 years ago
Hi, me and two other students at Cal Poly Pomona would like to work on this bug for an assignment.
(In reply to daletron from comment #3)
> Hi, me and two other students at Cal Poly Pomona would like to work on this
> bug for an assignment.

Great -- you can find links to the code and build instructions here:
https://wiki.mozilla.org/Mobile/Fennec/Android

There are a lot of steps and lots of information involved, so feel free to ask questions here, or email me, or come to the #mobile channel on irc.mozilla.org if you need help.

As a starting point, you should be able to add a key event handler to this activity to detect when the search key is pressed:
https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/AwesomeBar.java
Assignee: nobody → questionth1s

Comment 5

7 years ago
thanks matt for the info!  that helps alot, will be in contact

Comment 6

6 years ago
Please un-assign me from this bug.  Had many problems trying to even build Fennec in Linux.  Thank you Matt for the help.
Okay, no problem.  Feel free to find me on IRC if you'd like any help getting started with Fennec or other development.
Assignee: questionth1s → nobody
(Assignee)

Comment 8

6 years ago
Hello, I would like to get started with this bug.

Updated

6 years ago
Assignee: nobody → vangelis_51
(In reply to Paul Feher from comment #0)
> Expected Result:
> This should open up the Fennec search menu that should list the following
> search options: Google, Amazon.com, Twitter, Wikipedia.

Unlike older versions of Fennec where you had to type a query and then press a button to view the list of search engines, the current Fennec UI automatically displays the search engines as you type.  So I don't think we need to display a search menu anymore...

> Actual Result: 
> The fennec home page is displayed.

...but this is still the wrong thing to do, and should be fixed.  Perhaps instead we should clear the address bar input field and bring up the keyboard when you press the search button.
Summary: The search button should open the open search menu when the awesome panel is displayed → The search button should not dismiss the awesomescreen
(Assignee)

Comment 10

6 years ago
Created attachment 638877 [details] [diff] [review]
Proposed fix to updated bug
Comment on attachment 638877 [details] [diff] [review]
Proposed fix to updated bug

Thanks for the patch!  Note: When you are attaching a patch that's ready for review, you can set the "review" flag to "?" and enter the email of a reviewer.  (For mentored bugs, the mentor is usually a good reviewer.)  If the patch is not fully ready but you'd like some feedback, set the "feedback" flag instead.

This looks good.  Some minor notes below:

>+        } else if (keyCode == KeyEvent.KEYCODE_SEARCH ){
>+             mText.setText("");
>+             InputMethodManager imm = (InputMethodManager) mText.getContext().getSystemService(Context.INPUT_METHOD_SERVICE);
>+             imm.showSoftInput(mText, InputMethodManager.SHOW_IMPLICIT);

We should probably focus the text field too, in case it is un-focused.

The showSoftInput call is not working on my device (the keyboard does not appear), possibly for the reasons discussed in bug 762309.  Does it work on your device?

>+        }else {

Minor style note: There should be a space after the "}".
Attachment #638877 - Flags: feedback+
(Assignee)

Comment 12

6 years ago
(In reply to Matt Brubeck (:mbrubeck) from comment #11)
> Comment on attachment 638877 [details] [diff] [review]
> Proposed fix to updated bug
> 
> Thanks for the patch!  Note: When you are attaching a patch that's ready for
> review, you can set the "review" flag to "?" and enter the email of a
> reviewer.  (For mentored bugs, the mentor is usually a good reviewer.)  If
> the patch is not fully ready but you'd like some feedback, set the
> "feedback" flag instead.

> >+        }else {
> Minor style note: There should be a space after the "}".

Ok noted!

> This looks good.  Some minor notes below:
> 
> >+        } else if (keyCode == KeyEvent.KEYCODE_SEARCH ){
> >+             mText.setText("");
> >+             InputMethodManager imm = (InputMethodManager) mText.getContext().getSystemService(Context.INPUT_METHOD_SERVICE);
> >+             imm.showSoftInput(mText, InputMethodManager.SHOW_IMPLICIT);
> 
> We should probably focus the text field too, in case it is un-focused.
> 
> The showSoftInput call is not working on my device (the keyboard does not
> appear), possibly for the reasons discussed in bug 762309.  Does it work on
> your device?
> 

Yes actually it does work on my device and I can also reproduce the situation of bug 762309. 
So how do we approach this now? Should iI upload possible fixes and someone else can test them?
(In reply to van51 from comment #12)
> > The showSoftInput call is not working on my device (the keyboard does not
> > appear), possibly for the reasons discussed in bug 762309.  Does it work on
> > your device?
> 
> Yes actually it does work on my device and I can also reproduce the
> situation of bug 762309. 
> So how do we approach this now? Should iI upload possible fixes and someone
> else can test them?

If this works on your device, then I think we could just check it in like this.  The problem on my device could be fixed in a follow-up bug if necessary.  (If it only affects a small number of devices, I might not even bother.)

You can upload a patch that addresses the other comments, and request review for it.
(Assignee)

Comment 14

6 years ago
Created attachment 638923 [details] [diff] [review]
Patch addressing last comments
Attachment #638923 - Flags: review?(mbrubeck)
Comment on attachment 638923 [details] [diff] [review]
Patch addressing last comments

Great!  I'll check this in shortly.  Thank you very much.
Attachment #638923 - Flags: review?(mbrubeck) → review+
Pushed to mozilla-inbound.  This will be merged to mozilla-central today or tomorrow, and will appear in nightly builds the next day.  Thanks again!
https://hg.mozilla.org/integration/mozilla-inbound/rev/66f1b29d806b
Status: NEW → ASSIGNED
Comment on attachment 638923 [details] [diff] [review]
Patch addressing last comments

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

::: mobile/android/base/AwesomeBar.java
@@ +376,4 @@
>              keyCode == KeyEvent.KEYCODE_VOLUME_UP ||
>              keyCode == KeyEvent.KEYCODE_VOLUME_DOWN) {
>              return super.onKeyDown(keyCode, event);
> +        } else if (keyCode == KeyEvent.KEYCODE_SEARCH ){

Whitespace before the ).
(In reply to Tom Schuster [:evilpie] from comment #17)
> Whitespace before the ).

Oops!  Sorry I missed that in review.  Fixed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c820391d9a2e
https://hg.mozilla.org/mozilla-central/rev/66f1b29d806b
https://hg.mozilla.org/mozilla-central/rev/c820391d9a2e
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

6 years ago
Target Milestone: --- → Firefox 16
(Reporter)

Comment 20

6 years ago
Verified on:
Firefox Nightly 18.0a1 (2012-09-17)
Firefox Aurora 17.0a2 (2012-09-17)
Mobile Firefox 16.0b3
Device: HTC Desire Z
OS: Android 2.3.3

The search button now has the behavior described in comment9 marking this as verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.