Last Comment Bug 740306 - Back button does not dismiss the keyboard in awesomebar
: Back button does not dismiss the keyboard in awesomebar
Status: VERIFIED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: Firefox 14
Assigned To: :Margaret Leibovic
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-29 04:48 PDT by Paul Feher
Modified: 2012-05-16 12:20 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
verified
+


Attachments
patch (2.62 KB, patch)
2012-03-30 16:32 PDT, :Margaret Leibovic
mark.finkle: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Paul Feher 2012-03-29 04:48:44 PDT
Nightly Fennec 14.0a1 (2012-03-28)
Build:20120328115435
http://hg.mozilla.org/mozilla-central/rev/c3fd0768d46a
Device: Samsung Nexus S
OS: Android 2.3.6

Steps to reproduce:
1. Perform a sync operation 
2. Tap on the bookmark section on awesome
3. Select any folder
4. Tap on the URL bar for the keyboard to appear
5. Tap the back button

Expected:
The keyboard is dismissed.

Actual
You move up a folder level in the bookmarks UI.
Comment 1 :Margaret Leibovic 2012-03-29 10:32:36 PDT
This is tricky, since code was added in bug 700951 to explicitly not dismiss the keyboard, but I guess this wasn't considered at that point since this bookmarks folder UI didn't exist then. It seems to me we want to make a special case for when the user explicitly brings up the keyboard, but I'm not sure how we can track that.
Comment 2 :Margaret Leibovic 2012-03-30 16:32:01 PDT
Created attachment 611078 [details] [diff] [review]
patch

I tried using imm.isActive() to check if the keyboard was showing, but I found that it always returned true, which wasn't helpful. However, I found that hideSoftInputFromWindow returns a boolean to indicate whether or not it successfully hid the keyboard, so I decided to take advantage of that.

I also expanded this bug to cover cases where the user brings up the soft keyboard in the history tab. Since we're explicitly hiding the keyboard when the user selects the bookmarks/history tab, if it's showing in one of those tabs, the user must have brought it up, so I think it makes sense to make the back button hide it in that case.
Comment 3 Mark Finkle (:mfinkle) (use needinfo?) 2012-03-30 16:47:09 PDT
Comment on attachment 611078 [details] [diff] [review]
patch

>     public boolean onBackPressed() {
>+        // If the soft keyboard is visible in the bookmarks or history tab, the user
>+        // must have explictly brought it up, so we should try hiding it instead of
>+        // exiting the activity or going up a bookmarks folder level.
>+        if (getCurrentTabTag().equals(BOOKMARKS_TAB) ||
>+                getCurrentTabTag().equals(HISTORY_TAB)){

Feel free to let this on a single line. I'd rather have that than the indenting.

>         // also return false if mBookmarksAdapter hasn't been initialized yet.
>         if (!getCurrentTabTag().equals(BOOKMARKS_TAB) ||
>                 mBookmarksAdapter == null)

Can you roll this up on a single line too?

Your logic makes sense to me. Let's see if this handles the current situation well enough.
Comment 5 Ed Morley [:emorley] 2012-03-31 19:26:44 PDT
https://hg.mozilla.org/mozilla-central/rev/8a38635a80bd
Comment 6 :Margaret Leibovic 2012-04-02 08:11:42 PDT
Comment on attachment 611078 [details] [diff] [review]
patch

[Approval Request Comment]
User impact if declined: Inconsistent back button behavior in the awesome screen
Testing completed (on m-c, etc.): Landed on m-c 3/31
Risk to taking this patch (and alternatives if risky): Low risk, mobile-only change
Comment 7 Alex Keybl [:akeybl] 2012-04-02 09:57:36 PDT
Comment on attachment 611078 [details] [diff] [review]
patch

[Triage Comment]
Low risk, mobile-only Fennec fix. Approved for Aurora 13.
Comment 8 Paul Feher 2012-04-04 05:26:44 PDT
Verified/fixed on:
Nightly Fennec 14.0a1 (2012-04-04)
Device: HTC Desire
OS: Android 2.2.2
Comment 9 Mark Finkle (:mfinkle) (use needinfo?) 2012-04-04 10:05:05 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/41600364e780

Note You need to log in before you can comment on or make changes to this bug.